Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

yarn install shouldn't require setting cookies.keys #444

Closed
Tracked by #265
samos123 opened this issue Feb 24, 2022 · 5 comments
Closed
Tracked by #265

yarn install shouldn't require setting cookies.keys #444

samos123 opened this issue Feb 24, 2022 · 5 comments
Labels
breaking The issue or PR will introduce a breaking change enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed v1 Change intended to land in v1.x

Comments

@samos123
Copy link

Describe the bug
I do a yarn install as part of my Dockerfile image building steps and cookies.keys seem to be required.

Error observed:

ode:internal/process/promises:245                                                                                                  
          triggerUncaughtException(err, true /* fromPromise */);                                                                    
          ^                                                                                                                         
                                                                                                                                    
Error: Invalid next-firebase-auth options: The "cookies.keys" setting must be set if "cookies.signed" is true.                
    at b (/build/node_modules/next-firebase-auth/build/index.node.js:2:9873)                                                        
    at Object.init (/build/node_modules/next-firebase-auth/build/index.node.js:2:17601)                                             
    at init (/build/node_modules/next-firebase-auth/build/index.node.js:2:27068)                                            
    at initAuth (/build/.next/server/pages/_app.js:60:42)                                                                           
    at Object.8402 (/build/.next/server/pages/_app.js:330:1)                                                                        
    at __webpack_require__ (/build/.next/server/webpack-runtime.js:25:42)                                                           
    at __webpack_exec__ (/build/.next/server/pages/_app.js:536:39)                                                                  
    at /build/.next/server/pages/_app.js:537:74                                                                                     
    at Function.__webpack_require__.X (/build/.next/server/webpack-runtime.js:108:21)                                               
    at /build/.next/server/pages/_app.js:537:47 {                                                                                   
  type: 'Error'                                                                                                                     
}                                                                                                                                   
error Command failed with exit code 1.                                 

Versions

next-firebase-auth version:
Firebase JS SDK: 8
Next.js: 0.14.3-alpha.0

Expected behavior
yarn install to not require cookies.keys

Full Dockerfile:

###
# Build stage
###
FROM node:15-alpine AS builder
WORKDIR /build

COPY package.json yarn.lock ./
RUN yarn install --frozen-lockfile

COPY . .
RUN yarn build && \
    rm -rf .next/cache

###
# Exec Stage
###
FROM node:15-alpine
WORKDIR /app

COPY package.json yarn.lock ./
RUN yarn install --frozen-lockfile --production

COPY --from=builder /build/.next .next
COPY . .

CMD ["yarn", "start:production"]
@samos123
Copy link
Author

samos123 commented Feb 24, 2022

The workaround is to add a temporary value for the cookie during build but unsure if that might have unintended security consequences. This is the additional step I added in the build phase of the docker image:

ENV COOKIE_SECRET_CURRENT=willbereplacedlater

I'm afraid that somehow this secret might persist somewhere in the docker image?

For context the reason this works for me is due to initAuth.js looking like this:

    cookies: {
      keys: [process.env.COOKIE_SECRET_CURRENT, process.env.COOKIE_SECRET_PREVIOUS],
      httpOnly: true,
      maxAge: 12 * 60 * 60 * 24 * 1000, // twelve days
      overwrite: true,
      path: '/',
      sameSite: 'strict',
      signed: true,
    },

@DanielBailey-web
Copy link

@samos123 Had this same problem. I got around it by copying my env into the working dir
(I am using the example where the build happens in app and is run from root)

COPY --from=builder /app/.env* ./

I think yours would be closer to this:

COPY --from=builder ../build/.env* ./

(run this command AFTER)
WORKDIR /app

@kmjennison
Copy link
Contributor

Thanks for the issue! @samos123 I don't see any security risks in your current workaround.

Related: #70

We can remove the build-time check here and rely on the already-existing runtime check here.

This change should merge into the v1.x branch, because the behavior is a (small) breaking change.

@kmjennison kmjennison added breaking The issue or PR will introduce a breaking change enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed labels Mar 4, 2022
@nanduri1998
Copy link

This workaround is not working for me. Am not sure if there is any issue with my code. Also is there any other workaround?

COPY . .
COPY --from=deps /app/node_modules ./node_modules
ENV NODE_ENV production
ENV COOKIES_SECRET_CURRENT $GITHUB_SHA
ENV COOKIES_SECRET_PREVIOUS $GITHUB_SHA
RUN npm run build
      keys: [process.env.COOKIE_SECRET_CURRENT,process.env.COOKIE_SECRET_PREVIOUS],
      httpOnly: true,
      maxAge: 12 * 60 * 60 * 24 * 1000, // twelve days
      overwrite: true,
      path: '/',
      sameSite: 'strict',
      secure: process.env.NODE_ENV === 'production', // set this to false in local (non-HTTPS) development
      signed: true,

@kmjennison
Copy link
Contributor

Closed in #546.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking The issue or PR will introduce a breaking change enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed v1 Change intended to land in v1.x
Projects
None yet
Development

No branches or pull requests

4 participants