Skip to content

fix: don't override user defined NEXTAUTH_URL #1360

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
May 25, 2022
Merged

fix: don't override user defined NEXTAUTH_URL #1360

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1a2930c
fix: don't override user defined NEXTAUTH_URL
ericapisani May 25, 2022
e10f006
test: small test cleanup
ericapisani May 25, 2022
f9f8a47
refactor: address cr comments
ericapisani May 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
"@types/node": "^17.0.10",
"@types/react": "^17.0.38",
"babel-jest": "^27.2.5",
"chance": "^1.1.8",
"cpy": "^8.1.2",
"cypress": "^9.0.0",
"eslint-config-next": "^12.0.0",
Expand Down
19 changes: 14 additions & 5 deletions plugin/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,22 @@ const plugin: NetlifyPlugin = {
})

if (isNextAuthInstalled()) {
console.log(`NextAuth package detected, setting NEXTAUTH_URL environment variable to ${process.env.URL}`)

const config = await getRequiredServerFiles(publish)
const nextAuthUrl = `${process.env.URL}${basePath}`
config.config.env.NEXTAUTH_URL = nextAuthUrl

await updateRequiredServerFiles(publish, config)
const userDefinedNextAuthUrl = config.config.env.NEXTAUTH_URL

if (userDefinedNextAuthUrl) {
console.log(
`NextAuth package detected, NEXTAUTH_URL environment variable set by user to ${userDefinedNextAuthUrl}`,
)
} else {
const nextAuthUrl = `${process.env.URL}${basePath}`

console.log(`NextAuth package detected, setting NEXTAUTH_URL environment variable to ${nextAuthUrl}`)
config.config.env.NEXTAUTH_URL = nextAuthUrl

await updateRequiredServerFiles(publish, config)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what's already in these files - if the user sets their own NEXTAUTH_URL does it already exist in the required-server-files.json? Just wondering why we're only doing this in the else

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... if the user sets their own NEXTAUTH_URL does it already exist in the required-server-files.json

That's correct. required-server-files.json contains configuration related to the site after running the build command (next build in this case) so any environment variables defined by the user in next.config.js will appear within this file under the config.env object.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

}
}

const buildId = readFileSync(join(publish, 'BUILD_ID'), 'utf8').trim()
Expand Down
33 changes: 27 additions & 6 deletions test/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ jest.mock('../plugin/src/helpers/utils', () => {
}
})

const Chance = require('chance')
const { writeJSON, unlink, existsSync, readFileSync, copy, ensureDir, readJson } = require('fs-extra')
const path = require('path')
const process = require('process')
Expand All @@ -29,6 +30,7 @@ const { getRequiredServerFiles, updateRequiredServerFiles } = require('../plugin
const { dirname } = require('path')
const { getProblematicUserRewrites } = require('../plugin/src/helpers/verification')

const chance = new Chance()
const FIXTURES_DIR = `${__dirname}/fixtures`
const SAMPLE_PROJECT_DIR = `${__dirname}/../demos/default`
const constants = {
Expand Down Expand Up @@ -221,8 +223,31 @@ describe('onBuild()', () => {
})
})

afterEach(() => {
delete process.env.URL
})

test('does not set NEXTAUTH_URL if value is already set', async () => {
const mockUserDefinedSiteUrl = chance.url()
process.env.URL = chance.url()

await moveNextDist()

const initialConfig = await getRequiredServerFiles(netlifyConfig.build.publish)

initialConfig.config.env.NEXTAUTH_URL = mockUserDefinedSiteUrl
await updateRequiredServerFiles(netlifyConfig.build.publish, initialConfig)

await plugin.onBuild(defaultArgs)

expect(onBuildHasRun(netlifyConfig)).toBe(true)
const config = await getRequiredServerFiles(netlifyConfig.build.publish)

expect(config.config.env.NEXTAUTH_URL).toEqual(mockUserDefinedSiteUrl)
})

test('sets NEXTAUTH_URL when next-auth package is detected', async () => {
const mockSiteUrl = 'https://my-netlify-site.app'
const mockSiteUrl = chance.url()

// Value represents the main address to the site and is either
// a Netlify subdomain or custom domain set by the user.
Expand All @@ -237,12 +262,10 @@ describe('onBuild()', () => {
const config = await getRequiredServerFiles(netlifyConfig.build.publish)

expect(config.config.env.NEXTAUTH_URL).toEqual(mockSiteUrl)

delete process.env.URL
})

test('includes the basePath on NEXTAUTH_URL when present', async () => {
const mockSiteUrl = 'https://my-netlify-site.app'
const mockSiteUrl = chance.url()
process.env.URL = mockSiteUrl

await moveNextDist()
Expand All @@ -257,8 +280,6 @@ describe('onBuild()', () => {
const config = await getRequiredServerFiles(netlifyConfig.build.publish)

expect(config.config.env.NEXTAUTH_URL).toEqual(`${mockSiteUrl}/foo`)

delete process.env.URL
})

test('skips setting NEXTAUTH_URL when next-auth package is not found', async () => {
Expand Down