From 83313709eab651f8c29da10a4e313ae85dee448f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Orb=C3=A1n?= Date: Fri, 11 Dec 2020 22:43:26 +0100 Subject: [PATCH 01/11] chore: fix casing of OAuth --- src/lib/errors.js | 2 +- src/server/index.js | 2 +- src/server/lib/callback-handler.js | 22 +++++++++++----------- src/server/lib/oauth/callback.js | 6 +++--- src/server/lib/oauth/client.js | 4 ++-- src/server/lib/signin/oauth.js | 4 ++-- src/server/routes/callback.js | 2 +- 7 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/lib/errors.js b/src/lib/errors.js index 147d535101..f73e977826 100644 --- a/src/lib/errors.js +++ b/src/lib/errors.js @@ -25,7 +25,7 @@ class CreateUserError extends UnknownError { } // Thrown when an Email address is already associated with an account -// but the user is trying an oAuth account that is not linked to it. +// but the user is trying an OAuth account that is not linked to it. class AccountNotLinkedError extends UnknownError { constructor (message) { super(message) diff --git a/src/server/index.js b/src/server/index.js index 2b47a2e635..2ea1085054 100644 --- a/src/server/index.js +++ b/src/server/index.js @@ -62,7 +62,7 @@ async function NextAuth (req, res, userSuppliedOptions) { // Secret used salt cookies and tokens (e.g. for CSRF protection). // If no secret option is specified then it creates one on the fly // based on options passed here. A options contains unique data, such as - // oAuth provider secrets and database credentials it should be sufficent. + // OAuth provider secrets and database credentials it should be sufficent. const secret = userSuppliedOptions.secret || createHash('sha256').update(JSON.stringify({ baseUrl, basePath, ...userSuppliedOptions })).digest('hex') // Use secure cookies if the site uses HTTPS diff --git a/src/server/lib/callback-handler.js b/src/server/lib/callback-handler.js index 6fdd052456..aea6d48b68 100644 --- a/src/server/lib/callback-handler.js +++ b/src/server/lib/callback-handler.js @@ -2,10 +2,10 @@ // linking (or not linking) accounts depending on if the user is currently logged // in, if they have account already and the authentication mechanism they are using. // -// It prevents insecure behaviour, such as linking oAuth accounts unless a user is +// It prevents insecure behaviour, such as linking OAuth accounts unless a user is // signed in and authenticated with an existing valid account. // -// All verification (e.g. oAuth flows or email address verificaiton flows) are +// All verification (e.g. OAuth flows or email address verificaiton flows) are // done prior to this handler being called to avoid additonal complexity in this // handler. import { AccountNotLinkedError } from '../../lib/errors' @@ -136,7 +136,7 @@ export default async (sessionToken, profile, providerAccount, options) => { } } else { if (isSignedIn) { - // If the user is already signed in and the oAuth account isn't already associated + // If the user is already signed in and the OAuth account isn't already associated // with another user account then we can go ahead and link the accounts safely. await linkAccount( user.id, @@ -157,28 +157,28 @@ export default async (sessionToken, profile, providerAccount, options) => { } } - // If the user is not signed in and it looks like a new oAuth account then we + // If the user is not signed in and it looks like a new OAuth account then we // check there also isn't an user account already associated with the same - // email address as the one in the oAuth profile. + // email address as the one in the OAuth profile. // - // This step is often overlooked in oAuth implementations, but covers the following cases: + // This step is often overlooked in OAuth implementations, but covers the following cases: // // 1. It makes it harder for someone to accidentally create two accounts. // e.g. by signin in with email, then again with an oauth account connected to the same email. - // 2. It makes it harder to hijack a user account using a 3rd party oAuth account. + // 2. It makes it harder to hijack a user account using a 3rd party OAuth account. // e.g. by creating an oauth account then changing the email address associated with it. // // It's quite common for services to automatically link accounts in this case, but it's // better practice to require the user to sign in *then* link accounts to be sure - // someone is not exploiting a problem with a third party oAuth service. + // someone is not exploiting a problem with a third party OAuth service. // - // oAuth providers should require email address verification to prevent this, but in + // OAuth providers should require email address verification to prevent this, but in // practice that is not always the case; this helps protect against that. const userByEmail = profile.email ? await getUserByEmail(profile.email) : null if (userByEmail) { // We end up here when we don't have an account with the same [provider].id *BUT* // we do already have an account with the same email address as the one in the - // oAuth profile the user has just tried to sign in with. + // OAuth profile the user has just tried to sign in with. // // We don't want to have two accounts with the same email address, and we don't // want to link them in case it's not safe to do so, so instead we prompt the user @@ -189,7 +189,7 @@ export default async (sessionToken, profile, providerAccount, options) => { // accounts (by email or provider account id)... // // If no account matching the same [provider].id or .email exists, we can - // create a new account for the user, link it to the oAuth acccount and + // create a new account for the user, link it to the OAuth acccount and // create a new session for them so they are signed in with it. user = await createUser(profile) await dispatchEvent(events.createUser, user) diff --git a/src/server/lib/oauth/callback.js b/src/server/lib/oauth/callback.js index e19020dfc1..16d6eaeae7 100644 --- a/src/server/lib/oauth/callback.js +++ b/src/server/lib/oauth/callback.js @@ -28,7 +28,7 @@ export default async (req, provider, csrfToken, callback) => { if (!Object.prototype.hasOwnProperty.call(provider, 'state') || provider.state === true) { const expectedState = createHash('sha256').update(csrfToken).digest('hex') if (state !== expectedState) { - return callback(new Error('Invalid state returned from oAuth provider')) + return callback(new Error('Invalid state returned from OAuth provider')) } } @@ -103,7 +103,7 @@ export default async (req, provider, csrfToken, callback) => { } ) } else { - // Handle oAuth v1.x + // Handle OAuth v1.x await client.getOAuthAccessToken( oauth_token, null, @@ -211,7 +211,7 @@ async function _getOAuthAccessToken (code, provider, callback) { if (!params.redirect_uri) { params.redirect_uri = provider.callbackUrl } if (!headers['Content-Type']) { headers['Content-Type'] = 'application/x-www-form-urlencoded' } - // Added as a fix to accomodate change in Twitch oAuth API + // Added as a fix to accomodate change in Twitch OAuth API if (!headers['Client-ID']) { headers['Client-ID'] = provider.clientId } // Added as a fix for Reddit Authentication if (provider.id === 'reddit') { diff --git a/src/server/lib/oauth/client.js b/src/server/lib/oauth/client.js index dd2a9f0481..18bdfff581 100644 --- a/src/server/lib/oauth/client.js +++ b/src/server/lib/oauth/client.js @@ -5,7 +5,7 @@ import { OAuth, OAuth2 } from 'oauth' export default (provider) => { if (provider.version && provider.version.startsWith('2.')) { - // Handle oAuth v2.x + // Handle OAuth v2.x const basePath = new URL(provider.authorizationUrl).origin const authorizePath = new URL(provider.authorizationUrl).pathname const accessTokenPath = new URL(provider.accessTokenUrl).pathname @@ -17,7 +17,7 @@ export default (provider) => { accessTokenPath, provider.headers) } else { - // Handle oAuth v1.x + // Handle OAuth v1.x return new OAuth( provider.requestTokenUrl, provider.accessTokenUrl, diff --git a/src/server/lib/signin/oauth.js b/src/server/lib/signin/oauth.js index 3584efe564..13eada068c 100644 --- a/src/server/lib/signin/oauth.js +++ b/src/server/lib/signin/oauth.js @@ -6,7 +6,7 @@ export default (provider, csrfToken, callback, authParams) => { const { callbackUrl } = provider const client = oAuthClient(provider) if (provider.version && provider.version.startsWith('2.')) { - // Handle oAuth v2.x + // Handle OAuth v2.x let url = client.getAuthorizeUrl({ ...authParams, redirect_uri: provider.callbackUrl, @@ -31,7 +31,7 @@ export default (provider, csrfToken, callback, authParams) => { callback(null, url) } else { - // Handle oAuth v1.x + // Handle OAuth v1.x client.getOAuthRequestToken((error, oAuthToken) => { if (error) { logger.error('GET_AUTHORISATION_URL_ERROR', error) diff --git a/src/server/routes/callback.js b/src/server/routes/callback.js index 138861e9c5..cd9ee488ca 100644 --- a/src/server/routes/callback.js +++ b/src/server/routes/callback.js @@ -118,7 +118,7 @@ export default async (req, res, options, done) => { return redirect(callbackUrl || baseUrl) } catch (error) { if (error.name === 'AccountNotLinkedError') { - // If the email on the account is already linked, but nto with this oAuth account + // If the email on the account is already linked, but nto with this OAuth account return redirect(`${baseUrl}${basePath}/error?error=OAuthAccountNotLinked`) } else if (error.name === 'CreateUserError') { return redirect(`${baseUrl}${basePath}/error?error=OAuthCreateAccount`) From d04185cb5722e2b7698073783a88c27ca551cc24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Orb=C3=A1n?= Date: Fri, 11 Dec 2020 22:50:58 +0100 Subject: [PATCH 02/11] refacotr: simplify default callbacks lib file --- src/server/index.js | 12 ++--- .../lib/{callbacks.js => defaultCallbacks.js} | 51 ++++++++----------- 2 files changed, 26 insertions(+), 37 deletions(-) rename src/server/lib/{callbacks.js => defaultCallbacks.js} (58%) diff --git a/src/server/index.js b/src/server/index.js index 2ea1085054..8749c4259a 100644 --- a/src/server/index.js +++ b/src/server/index.js @@ -5,7 +5,7 @@ import cookie from './lib/cookie' import callbackUrlHandler from './lib/callback-url-handler' import parseProviders from './lib/providers' import events from './lib/events' -import callbacks from './lib/callbacks' +import * as defaultCallbacks from './lib/defaultCallbacks' import providers from './routes/providers' import signin from './routes/signin' import signout from './routes/signout' @@ -21,7 +21,7 @@ if (!process.env.NEXTAUTH_URL) { logger.warn('NEXTAUTH_URL', 'NEXTAUTH_URL environment variable not set') } -async function NextAuth (req, res, userSuppliedOptions) { +async function NextAuthHandler (req, res, userSuppliedOptions) { // To the best of my knowledge, we need to return a promise here // to avoid early termination of calls to the serverless function // (and then return that promise when we are done) - eslint @@ -139,7 +139,7 @@ async function NextAuth (req, res, userSuppliedOptions) { // Callback functions const callbacksOptions = { - ...callbacks, + ...defaultCallbacks, ...userSuppliedOptions.callbacks } @@ -314,10 +314,10 @@ async function NextAuth (req, res, userSuppliedOptions) { }) } -export default async (...args) => { +export default async function NextAuth (...args) { if (args.length === 1) { - return (req, res) => NextAuth(req, res, args[0]) + return (req, res) => NextAuthHandler(req, res, args[0]) } - return NextAuth(...args) + return NextAuthHandler(...args) } diff --git a/src/server/lib/callbacks.js b/src/server/lib/defaultCallbacks.js similarity index 58% rename from src/server/lib/callbacks.js rename to src/server/lib/defaultCallbacks.js index 4c760c20b6..a32437980a 100644 --- a/src/server/lib/callbacks.js +++ b/src/server/lib/defaultCallbacks.js @@ -9,19 +9,14 @@ * requests to sign in and again when they activate the link in the sign in * email. * - * @param {object} profile User profile (e.g. user id, name, email) - * @param {object} account Account used to sign in (e.g. OAuth account) - * @param {object} metadata Provider specific metadata (e.g. OAuth Profile) - * @return {boolean|object} Return `true` (or a modified JWT) to allow sign in - * Return `false` to deny access + * @param {object} profile User profile (e.g. user id, name, email) + * @param {object} account Account used to sign in (e.g. OAuth account) + * @param {object} metadata Provider specific metadata (e.g. OAuth Profile) + * @return {Promise} Return `true` (or a modified JWT) to allow sign in + * Return `false` to deny access */ -const signIn = async (profile, account, metadata) => { - const isAllowedToSignIn = true - if (isAllowedToSignIn) { - return Promise.resolve(true) - } else { - return Promise.resolve(false) - } +export async function signIn () { + return true } /** @@ -31,12 +26,13 @@ const signIn = async (profile, account, metadata) => { * * @param {string} url URL provided as callback URL by the client * @param {string} baseUrl Default base URL of site (can be used as fallback) - * @return {string} URL the client will be redirect to + * @return {Promise} URL the client will be redirect to */ -const redirect = async (url, baseUrl) => { - return url.startsWith(baseUrl) - ? Promise.resolve(url) - : Promise.resolve(baseUrl) +export async function redirect (url, baseUrl) { + if (url.startsWith(baseUrl)) { + return url + } + return baseUrl } /** @@ -45,31 +41,24 @@ const redirect = async (url, baseUrl) => { * * @param {object} session Session object * @param {object} token JSON Web Token (if enabled) - * @return {object} Session that will be returned to the client + * @return {Promise} Session that will be returned to the client */ -const session = async (session, token) => { - return Promise.resolve(session) +export async function session (session) { + return session } /** * This callback is called whenever a JSON Web Token is created / updated. * e.g. On sign in, `getSession()`, `useSession()`, `/api/auth/session` (etc) * - * On initial sign in, the raw oAuthProfile is passed if the user is signing in + * On initial sign in, the raw OAuthProfile is passed if the user is signing in * with an OAuth provider. It is not avalible on subsequent calls. You can * take advantage of this to persist additional data you need to in the JWT. * * @param {object} token Decrypted JSON Web Token * @param {object} oAuthProfile OAuth profile - only available on sign in - * @return {object} JSON Web Token that will be saved + * @return {Promise} JSON Web Token that will be saved */ -const jwt = async (token, oAuthProfile) => { - return Promise.resolve(token) -} - -export default { - signIn, - redirect, - session, - jwt +export async function jwt (token) { + return token } From 690c55b04089e4f3157424c816d43ee4cecb77a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Orb=C3=A1n?= Date: Tue, 15 Dec 2020 20:19:13 +0100 Subject: [PATCH 03/11] refactor: use native URL instead of string concats --- src/client/index.js | 18 ++++----- src/lib/baseUrl.js | 30 +++++++++++++++ src/lib/parse-url.js | 27 ------------- src/server/index.js | 36 ++++++++--------- src/server/lib/callback-url-handler.js | 11 +++--- src/server/lib/providers.js | 8 ++-- src/server/lib/signin/email.js | 5 ++- src/server/pages/error.js | 7 ++-- src/server/pages/signout.js | 5 ++- src/server/pages/verify-request.js | 5 ++- src/server/routes/callback.js | 53 ++++++++++++-------------- src/server/routes/signin.js | 17 ++++----- 12 files changed, 108 insertions(+), 114 deletions(-) create mode 100644 src/lib/baseUrl.js delete mode 100644 src/lib/parse-url.js diff --git a/src/client/index.js b/src/client/index.js index 6fcaec2e75..e1eec5f85e 100644 --- a/src/client/index.js +++ b/src/client/index.js @@ -13,7 +13,7 @@ /* global fetch:false */ import { useState, useEffect, useContext, createContext, createElement } from 'react' import logger from '../lib/logger' -import parseUrl from '../lib/parse-url' +import baseUrl from '../lib/baseUrl' // This behaviour mirrors the default behaviour for getting the site name that // happens server side in server/index.js @@ -22,8 +22,7 @@ import parseUrl from '../lib/parse-url' // 2. When invoked server side the value is picked up from an environment // variable and defaults to 'http://localhost:3000'. const __NEXTAUTH = { - baseUrl: parseUrl(process.env.NEXTAUTH_URL || process.env.VERCEL_URL).baseUrl, - basePath: parseUrl(process.env.NEXTAUTH_URL).basePath, + baseUrl: baseUrl(), keepAlive: 0, // 0 == disabled (don't send); 60 == send every 60 seconds clientMaxAge: 0, // 0 == disabled (only use cache); 60 == sync if last checked > 60 seconds ago // Properties starting with _ are used for tracking internal app state @@ -77,13 +76,11 @@ if (typeof window !== 'undefined') { // method is being left in as an alternative, that will be helpful if/when we // expose a vanilla JavaScript version that doesn't depend on React. const setOptions = ({ - baseUrl, - basePath, + baseUrl: _baseUrl, clientMaxAge, keepAlive } = {}) => { - if (baseUrl) { __NEXTAUTH.baseUrl = baseUrl } - if (basePath) { __NEXTAUTH.basePath = basePath } + if (baseUrl) { __NEXTAUTH.baseUrl = baseUrl(_baseUrl) } if (clientMaxAge) { __NEXTAUTH.clientMaxAge = clientMaxAge } if (keepAlive) { __NEXTAUTH.keepAlive = keepAlive @@ -306,11 +303,10 @@ const _apiBaseUrl = () => { if (!process.env.NEXTAUTH_URL) { logger.warn('NEXTAUTH_URL', 'NEXTAUTH_URL environment variable not set') } // Return absolute path when called server side - return `${__NEXTAUTH.baseUrl}${__NEXTAUTH.basePath}` - } else { - // Return relative path when called client side - return __NEXTAUTH.basePath + return __NEXTAUTH.baseUrl.href } + // Return relative path when called client side + return __NEXTAUTH.baseUrl.pathname } const _encodedForm = (formData) => { diff --git a/src/lib/baseUrl.js b/src/lib/baseUrl.js new file mode 100644 index 0000000000..cc09a662df --- /dev/null +++ b/src/lib/baseUrl.js @@ -0,0 +1,30 @@ +import logger from './logger' + +/** + * Simple universal (client/server) function to split host and path. + * It can also take a url (either URL or a string) and parses it correctly. + * @returns {URL} + */ +function baseUrl (url) { + let _url = url || process.env.NEXTAUTH_URL || process.env.VERCEL_URL + if (typeof _url !== 'string' && !(_url instanceof URL)) { + throw new Error('baseUrl must be either a valid URL object or a valid string URL') + } + const defaultUrl = 'http://localhost:3000/api/auth' + _url = _url || defaultUrl + try { + const parsedUrl = new URL(_url) + if (parsedUrl.pathname === '/') { + parsedUrl.pathname = '/api/auth' + } + parsedUrl.pathname = parsedUrl.pathname.replace(/\/$/, '') + parsedUrl.href = parsedUrl.href.replace(/\/$/, '') + + return parsedUrl + } catch (error) { + logger.error('INVALID_URL', _url, error) + return new URL(defaultUrl) + } +} + +export default baseUrl diff --git a/src/lib/parse-url.js b/src/lib/parse-url.js deleted file mode 100644 index 0428925fc0..0000000000 --- a/src/lib/parse-url.js +++ /dev/null @@ -1,27 +0,0 @@ -// Simple universal (client/server) function to split host and path -// We use this rather than a library because we need to use the same logic both -// client and server side and we only need to parse out the host and path, while -// supporting a default value, so a simple split is sufficent. -export default (url) => { - // Default values - const defaultHost = 'http://localhost:3000' - const defaultPath = '/api/auth' - - if (!url) { url = `${defaultHost}${defaultPath}` } - - // Default to HTTPS if no protocol explictly specified - const protocol = url.match(/^http?:\/\//) ? 'http' : 'https' - - // Normalize URLs by stripping protocol and no trailing slash - url = url.replace(/^https?:\/\//, '').replace(/\/$/, '') - - // Simple split based on first / - const [_host, ..._path] = url.split('/') - const baseUrl = _host ? `${protocol}://${_host}` : defaultHost - const basePath = _path.length > 0 ? `/${_path.join('/')}` : defaultPath - - return { - baseUrl, - basePath - } -} diff --git a/src/server/index.js b/src/server/index.js index 8749c4259a..9c03aede27 100644 --- a/src/server/index.js +++ b/src/server/index.js @@ -1,6 +1,6 @@ import { createHash, randomBytes } from 'crypto' import jwt from '../lib/jwt' -import parseUrl from '../lib/parse-url' +import baseUrl from '../lib/baseUrl' import cookie from './lib/cookie' import callbackUrlHandler from './lib/callback-url-handler' import parseProviders from './lib/providers' @@ -44,11 +44,6 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { csrfToken: csrfTokenFromPost } = body - // @todo refactor all existing references to site, baseUrl and basePath - const parsedUrl = parseUrl(process.env.NEXTAUTH_URL || process.env.VERCEL_URL) - const baseUrl = parsedUrl.baseUrl - const basePath = parsedUrl.basePath - // Parse database / adapter let adapter if (userSuppliedOptions.adapter) { @@ -63,7 +58,9 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { // If no secret option is specified then it creates one on the fly // based on options passed here. A options contains unique data, such as // OAuth provider secrets and database credentials it should be sufficent. - const secret = userSuppliedOptions.secret || createHash('sha256').update(JSON.stringify({ baseUrl, basePath, ...userSuppliedOptions })).digest('hex') + const secret = userSuppliedOptions.secret || createHash('sha256').update(JSON.stringify({ + baseUrl: baseUrl(), ...userSuppliedOptions + })).digest('hex') // Use secure cookies if the site uses HTTPS // This being conditional allows cookies to work non-HTTPS development URLs @@ -71,7 +68,7 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { // prefix, but enable them by default if the site URL is HTTPS; but not for // non-HTTPS URLs like http://localhost which are used in development). // For more on prefixes see https://googlechrome.github.io/samples/cookie-prefixes/ - const useSecureCookies = userSuppliedOptions.useSecureCookies || baseUrl.startsWith('https://') + const useSecureCookies = userSuppliedOptions.useSecureCookies || baseUrl().protocol.startsWith('https://') const cookiePrefix = useSecureCookies ? '__Secure-' : '' // @TODO Review cookie settings (names, options) @@ -201,19 +198,16 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { // These computed settings can values in userSuppliedOptions but override them // and are request-specific. adapter, - baseUrl, - basePath, action, provider, cookies, secret, csrfToken, - providers: parseProviders(userSuppliedOptions.providers, baseUrl, basePath), + providers: parseProviders(userSuppliedOptions.providers), session: sessionOptions, jwt: jwtOptions, events: eventsOptions, callbacks: callbacksOptions, - callbackUrl: baseUrl, redirect } @@ -221,7 +215,7 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { if (options.debug === true) { process.env._NEXTAUTH_DEBUG = true } // Get / Set callback URL based on query param / cookie + validation - options.callbackUrl = await callbackUrlHandler(req, res, options) + const callbackUrl = await callbackUrlHandler(req, res, options) if (req.method === 'GET') { switch (action) { @@ -236,17 +230,17 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { return done() case 'signin': if (options.pages.signIn) { - let redirectUrl = `${options.pages.signIn}${options.pages.signIn.includes('?') ? '&' : '?'}callbackUrl=${options.callbackUrl}` + let redirectUrl = `${options.pages.signIn}${options.pages.signIn.includes('?') ? '&' : '?'}callbackUrl=${callbackUrl}` if (req.query.error) { redirectUrl = `${redirectUrl}&error=${req.query.error}` } return redirect(redirectUrl) } - pages.render(req, res, 'signin', { baseUrl, basePath, providers: Object.values(options.providers), callbackUrl: options.callbackUrl, csrfToken }, done) + pages.render(req, res, 'signin', { providers: Object.values(options.providers), callbackUrl, csrfToken }, done) break case 'signout': if (options.pages.signOut) { return redirect(`${options.pages.signOut}${options.pages.signOut.includes('?') ? '&' : '?'}error=${error}`) } - pages.render(req, res, 'signout', { baseUrl, basePath, csrfToken, callbackUrl: options.callbackUrl }, done) + pages.render(req, res, 'signout', { csrfToken, callbackUrl }, done) break case 'callback': if (provider && options.providers[provider]) { @@ -259,12 +253,12 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { case 'verify-request': if (options.pages.verifyRequest) { return redirect(options.pages.verifyRequest) } - pages.render(req, res, 'verify-request', { baseUrl }, done) + pages.render(req, res, 'verify-request', { }, done) break case 'error': if (options.pages.error) { return redirect(`${options.pages.error}${options.pages.error.includes('?') ? '&' : '?'}error=${error}`) } - pages.render(req, res, 'error', { baseUrl, basePath, error }, done) + pages.render(req, res, 'error', { error }, done) break default: res.status(404).end() @@ -275,7 +269,7 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { case 'signin': // Verified CSRF Token required for all sign in routes if (!csrfTokenVerified) { - return redirect(`${baseUrl}${basePath}/signin?csrf=true`) + return redirect(`${baseUrl()}/signin?csrf=true`) } if (provider && options.providers[provider]) { @@ -285,7 +279,7 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { case 'signout': // Verified CSRF Token required for signout if (!csrfTokenVerified) { - return redirect(`${baseUrl}${basePath}/signout?csrf=true`) + return redirect(`${baseUrl()}/signout?csrf=true`) } signout(req, res, options, done) @@ -294,7 +288,7 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { if (provider && options.providers[provider]) { // Verified CSRF Token required for credentials providers only if (options.providers[provider].type === 'credentials' && !csrfTokenVerified) { - return redirect(`${baseUrl}${basePath}/signin?csrf=true`) + return redirect(`${baseUrl()}/signin?csrf=true`) } callback(req, res, options, done) diff --git a/src/server/lib/callback-url-handler.js b/src/server/lib/callback-url-handler.js index 7bb6c6fddf..5df3e76434 100644 --- a/src/server/lib/callback-url-handler.js +++ b/src/server/lib/callback-url-handler.js @@ -1,23 +1,24 @@ import cookie from '../lib/cookie' +import baseUrl from '../../lib/baseUrl' export default async (req, res, options) => { const { query } = req const { body } = req - const { cookies, baseUrl, defaultCallbackUrl, callbacks } = options + const { cookies, defaultCallbackUrl, callbacks } = options + const homepage = baseUrl().origin // Handle preserving and validating callback URLs // If no defaultCallbackUrl option specified, default to the homepage for the site - let callbackUrl = defaultCallbackUrl || baseUrl - + let callbackUrl = defaultCallbackUrl || homepage // Try reading callbackUrlParamValue from request body (form submission) then from query param (get request) const callbackUrlParamValue = body.callbackUrl || query.callbackUrl || null const callbackUrlCookieValue = req.cookies[cookies.callbackUrl.name] || null if (callbackUrlParamValue) { // If callbackUrl form field or query parameter is passed try to use it if allowed - callbackUrl = await callbacks.redirect(callbackUrlParamValue, baseUrl) + callbackUrl = await callbacks.redirect(callbackUrlParamValue, homepage) } else if (callbackUrlCookieValue) { // If no callbackUrl specified, try using the value from the cookie if allowed - callbackUrl = await callbacks.redirect(callbackUrlCookieValue, baseUrl) + callbackUrl = await callbacks.redirect(callbackUrlCookieValue, homepage) } // Save callback URL in a cookie so that can be used for subsequent requests in signin/signout/callback flow diff --git a/src/server/lib/providers.js b/src/server/lib/providers.js index 20b28aa96d..722f9c3024 100644 --- a/src/server/lib/providers.js +++ b/src/server/lib/providers.js @@ -1,12 +1,14 @@ -export default (_providers, baseUrl, basePath) => { +import baseUrl from '../../lib/baseUrl' + +export default function parseProviders (_providers) { const providers = {} _providers.forEach(provider => { const providerId = provider.id providers[providerId] = { ...provider, - signinUrl: `${baseUrl}${basePath}/signin/${providerId}`, - callbackUrl: `${baseUrl}${basePath}/callback/${providerId}` + signinUrl: `${baseUrl()}/signin/${providerId}`, + callbackUrl: `${baseUrl()}/callback/${providerId}` } }) diff --git a/src/server/lib/signin/email.js b/src/server/lib/signin/email.js index da4563c8f7..403e20196d 100644 --- a/src/server/lib/signin/email.js +++ b/src/server/lib/signin/email.js @@ -1,8 +1,9 @@ import { randomBytes } from 'crypto' +import baseUrl from '../../../lib/baseUrl' export default async (email, provider, options) => { try { - const { baseUrl, basePath, adapter } = options + const { adapter } = options const { createVerificationRequest } = await adapter.getAdapter(options) @@ -13,7 +14,7 @@ export default async (email, provider, options) => { const token = randomBytes(32).toString('hex') // Send email with link containing token (the unhashed version) - const url = `${baseUrl}${basePath}/callback/${encodeURIComponent(provider.id)}?email=${encodeURIComponent(email)}&token=${encodeURIComponent(token)}` + const url = `${baseUrl()}/callback/${encodeURIComponent(provider.id)}?email=${encodeURIComponent(email)}&token=${encodeURIComponent(token)}` // @TODO Create invite (send secret so can be hashed) await createVerificationRequest(email, url, token, secret, provider, options) diff --git a/src/server/pages/error.js b/src/server/pages/error.js index eea7c906c2..870fc65d45 100644 --- a/src/server/pages/error.js +++ b/src/server/pages/error.js @@ -1,12 +1,13 @@ import { h } from 'preact' // eslint-disable-line no-unused-vars import render from 'preact-render-to-string' +import baseUrl from '../../lib/baseUrl' -export default ({ baseUrl, basePath, error, res }) => { - const signinPageUrl = `${baseUrl}${basePath}/signin` +export default ({ error, res }) => { + const signinPageUrl = `${baseUrl()}/signin` let statusCode = 200 let heading =

Error

- let message =

{baseUrl.replace(/^https?:\/\//, '')}

+ let message =

{baseUrl().host}

switch (error) { case 'Signin': diff --git a/src/server/pages/signout.js b/src/server/pages/signout.js index 5047b0587c..97bbcdad78 100644 --- a/src/server/pages/signout.js +++ b/src/server/pages/signout.js @@ -1,11 +1,12 @@ import { h } from 'preact' // eslint-disable-line no-unused-vars import render from 'preact-render-to-string' +import baseUrl from '../../lib/baseUrl' -export default ({ baseUrl, basePath, csrfToken }) => { +export default ({ csrfToken }) => { return render(

Are you sure you want to sign out?

-
+
diff --git a/src/server/pages/verify-request.js b/src/server/pages/verify-request.js index 6c9f50e776..259c152c29 100644 --- a/src/server/pages/verify-request.js +++ b/src/server/pages/verify-request.js @@ -1,12 +1,13 @@ import { h } from 'preact' // eslint-disable-line no-unused-vars import render from 'preact-render-to-string' +import baseUrl from '../../lib/baseUrl' -export default ({ baseUrl }) => { +export default function verifyRequest () { return render(

Check your email

A sign in link has been sent to your email address.

-

{baseUrl.replace(/^https?:\/\//, '')}

+

{baseUrl().host}

) } diff --git a/src/server/routes/callback.js b/src/server/routes/callback.js index cd9ee488ca..e6a73a85f9 100644 --- a/src/server/routes/callback.js +++ b/src/server/routes/callback.js @@ -4,14 +4,13 @@ import callbackHandler from '../lib/callback-handler' import cookie from '../lib/cookie' import logger from '../../lib/logger' import dispatchEvent from '../lib/dispatch-event' +import baseUrl from '../../lib/baseUrl' export default async (req, res, options, done) => { const { provider: providerName, providers, adapter, - baseUrl, - basePath, secret, cookies, callbackUrl, @@ -36,7 +35,7 @@ export default async (req, res, options, done) => { try { if (error) { logger.error('CALLBACK_OAUTH_ERROR', error) - return redirect(`${baseUrl}${basePath}/error?error=OAuthCallback`) + return redirect(`${baseUrl()}/error?error=OAuthCallback`) } // Make it easier to debug when adding a new provider @@ -51,7 +50,7 @@ export default async (req, res, options, done) => { // should at least be visible to developers what happened if it is an // error with the provider. if (!profile) { - return redirect(`${baseUrl}${basePath}/signin`) + return redirect(`${baseUrl()}/signin`) } // Check if user is allowed to sign in @@ -70,11 +69,11 @@ export default async (req, res, options, done) => { try { const signInCallbackResponse = await callbacks.signIn(userOrProfile, account, OAuthProfile) if (signInCallbackResponse === false) { - return redirect(`${baseUrl}${basePath}/error?error=AccessDenied`) + return redirect(`${baseUrl()}/error?error=AccessDenied`) } } catch (error) { if (error instanceof Error) { - return redirect(`${baseUrl}${basePath}/error?error=${encodeURIComponent(error)}`) + return redirect(`${baseUrl()}/error?error=${encodeURIComponent(error)}`) } else { return redirect(error) } @@ -115,28 +114,28 @@ export default async (req, res, options, done) => { } // Callback URL is already verified at this point, so safe to use if specified - return redirect(callbackUrl || baseUrl) + return redirect(callbackUrl || baseUrl().origin) } catch (error) { if (error.name === 'AccountNotLinkedError') { // If the email on the account is already linked, but nto with this OAuth account - return redirect(`${baseUrl}${basePath}/error?error=OAuthAccountNotLinked`) + return redirect(`${baseUrl()}/error?error=OAuthAccountNotLinked`) } else if (error.name === 'CreateUserError') { - return redirect(`${baseUrl}${basePath}/error?error=OAuthCreateAccount`) + return redirect(`${baseUrl()}/error?error=OAuthCreateAccount`) } else { logger.error('OAUTH_CALLBACK_HANDLER_ERROR', error) - return redirect(`${baseUrl}${basePath}/error?error=Callback`) + return redirect(`${baseUrl()}/error?error=Callback`) } } }) } catch (error) { logger.error('OAUTH_CALLBACK_ERROR', error) - return redirect(`${baseUrl}${basePath}/error?error=Callback`) + return redirect(`${baseUrl()}/error?error=Callback`) } } else if (type === 'email') { try { if (!adapter) { logger.error('EMAIL_REQUIRES_ADAPTER_ERROR') - return redirect(`${baseUrl}${basePath}/error?error=Configuration`) + return redirect(`${baseUrl()}/error?error=Configuration`) } const { getVerificationRequest, deleteVerificationRequest, getUserByEmail } = await adapter.getAdapter(options) @@ -146,7 +145,7 @@ export default async (req, res, options, done) => { // Verify email and verification token exist in database const invite = await getVerificationRequest(email, verificationToken, secret, provider) if (!invite) { - return redirect(`${baseUrl}${basePath}/error?error=Verification`) + return redirect(`${baseUrl()}/error?error=Verification`) } // If verification token is valid, delete verification request token from @@ -161,11 +160,11 @@ export default async (req, res, options, done) => { try { const signInCallbackResponse = await callbacks.signIn(profile, account, { email }) if (signInCallbackResponse === false) { - return redirect(`${baseUrl}${basePath}/error?error=AccessDenied`) + return redirect(`${baseUrl()}/error?error=AccessDenied`) } } catch (error) { if (error instanceof Error) { - return redirect(`${baseUrl}${basePath}/error?error=${encodeURIComponent(error)}`) + return redirect(`${baseUrl()}/error?error=${encodeURIComponent(error)}`) } else { return redirect(error) } @@ -206,28 +205,24 @@ export default async (req, res, options, done) => { } // Callback URL is already verified at this point, so safe to use if specified - if (callbackUrl) { - return redirect(callbackUrl) - } else { - return redirect(baseUrl) - } + return redirect(callbackUrl || baseUrl().origin) } catch (error) { if (error.name === 'CreateUserError') { - return redirect(`${baseUrl}${basePath}/error?error=EmailCreateAccount`) + return redirect(`${baseUrl()}/error?error=EmailCreateAccount`) } else { logger.error('CALLBACK_EMAIL_ERROR', error) - return redirect(`${baseUrl}${basePath}/error?error=Callback`) + return redirect(`${baseUrl()}/error?error=Callback`) } } } else if (type === 'credentials' && req.method === 'POST') { if (!useJwtSession) { logger.error('CALLBACK_CREDENTIALS_JWT_ERROR', 'Signin in with credentials is only supported if JSON Web Tokens are enabled') - return redirect(`${baseUrl}${basePath}/error?error=Configuration`) + return redirect(`${baseUrl()}/error?error=Configuration`) } if (!provider.authorize) { logger.error('CALLBACK_CREDENTIALS_HANDLER_ERROR', 'Must define an authorize() handler to use credentials authentication provider') - return redirect(`${baseUrl}${basePath}/error?error=Configuration`) + return redirect(`${baseUrl()}/error?error=Configuration`) } const credentials = req.body @@ -236,11 +231,11 @@ export default async (req, res, options, done) => { try { userObjectReturnedFromAuthorizeHandler = await provider.authorize(credentials) if (!userObjectReturnedFromAuthorizeHandler) { - return redirect(`${baseUrl}${basePath}/error?error=CredentialsSignin&provider=${encodeURIComponent(provider.id)}`) + return redirect(`${baseUrl()}/error?error=CredentialsSignin&provider=${encodeURIComponent(provider.id)}`) } } catch (error) { if (error instanceof Error) { - return redirect(`${baseUrl}${basePath}/error?error=${encodeURIComponent(error)}`) + return redirect(`${baseUrl()}/error?error=${encodeURIComponent(error)}`) } else { return redirect(error) } @@ -252,11 +247,11 @@ export default async (req, res, options, done) => { try { const signInCallbackResponse = await callbacks.signIn(user, account, credentials) if (signInCallbackResponse === false) { - return redirect(`${baseUrl}${basePath}/error?error=AccessDenied`) + return redirect(`${baseUrl()}/error?error=AccessDenied`) } } catch (error) { if (error instanceof Error) { - return redirect(`${baseUrl}${basePath}/error?error=${encodeURIComponent(error)}`) + return redirect(`${baseUrl()}/error?error=${encodeURIComponent(error)}`) } else { return redirect(error) } @@ -280,7 +275,7 @@ export default async (req, res, options, done) => { await dispatchEvent(events.signIn, { user, account }) - return redirect(callbackUrl || baseUrl) + return redirect(callbackUrl || baseUrl().origin) } else { res.status(500).end(`Error: Callback for provider type ${type} not supported`) return done() diff --git a/src/server/routes/signin.js b/src/server/routes/signin.js index 68fb3de610..f1a02e2033 100644 --- a/src/server/routes/signin.js +++ b/src/server/routes/signin.js @@ -2,13 +2,12 @@ import oAuthSignin from '../lib/signin/oauth' import emailSignin from '../lib/signin/email' import logger from '../../lib/logger' +import baseUrl from '../../lib/baseUrl' export default async (req, res, options, done) => { const { provider: providerName, providers, - baseUrl, - basePath, adapter, callbacks, csrfToken, @@ -29,7 +28,7 @@ export default async (req, res, options, done) => { oAuthSignin(provider, csrfToken, (error, oAuthSigninUrl) => { if (error) { logger.error('SIGNIN_OAUTH_ERROR', error) - return redirect(`${baseUrl}${basePath}/error?error=OAuthSignin`) + return redirect(`${baseUrl()}/error?error=OAuthSignin`) } return redirect(oAuthSigninUrl) @@ -37,7 +36,7 @@ export default async (req, res, options, done) => { } else if (type === 'email' && req.method === 'POST') { if (!adapter) { logger.error('EMAIL_REQUIRES_ADAPTER_ERROR') - return redirect(`${baseUrl}${basePath}/error?error=Configuration`) + return redirect(`${baseUrl()}/error?error=Configuration`) } const { getUserByEmail } = await adapter.getAdapter(options) @@ -56,11 +55,11 @@ export default async (req, res, options, done) => { try { const signinCallbackResponse = await callbacks.signIn(profile, account, { email, verificationRequest: true }) if (signinCallbackResponse === false) { - return redirect(`${baseUrl}${basePath}/error?error=AccessDenied`) + return redirect(`${baseUrl()}/error?error=AccessDenied`) } } catch (error) { if (error instanceof Error) { - return redirect(`${baseUrl}${basePath}/error?error=${encodeURIComponent(error)}`) + return redirect(`${baseUrl()}/error?error=${encodeURIComponent(error)}`) } else { return redirect(error) } @@ -70,13 +69,13 @@ export default async (req, res, options, done) => { await emailSignin(email, provider, options) } catch (error) { logger.error('SIGNIN_EMAIL_ERROR', error) - return redirect(`${baseUrl}${basePath}/error?error=EmailSignin`) + return redirect(`${baseUrl()}/error?error=EmailSignin`) } - return redirect(`${baseUrl}${basePath}/verify-request?provider=${encodeURIComponent( + return redirect(`${baseUrl()}/verify-request?provider=${encodeURIComponent( provider.id )}&type=${encodeURIComponent(provider.type)}`) } else { - return redirect(`${baseUrl}${basePath}/signin`) + return redirect(`${baseUrl()}/signin`) } } From b63857c221f889765914859712e87f108d1d198f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Orb=C3=A1n?= Date: Tue, 15 Dec 2020 20:40:53 +0100 Subject: [PATCH 04/11] refactor: move redirect to res.redirect, done to res.end --- src/server/index.js | 71 +++++++++++++++------------------- src/server/lib/redirect.js | 19 +++++++++ src/server/pages/index.js | 6 +-- src/server/routes/callback.js | 65 +++++++++++++++---------------- src/server/routes/providers.js | 4 +- src/server/routes/session.js | 6 +-- src/server/routes/signin.js | 25 ++++++------ src/server/routes/signout.js | 6 +-- 8 files changed, 105 insertions(+), 97 deletions(-) create mode 100644 src/server/lib/redirect.js diff --git a/src/server/index.js b/src/server/index.js index 9c03aede27..427a777bc2 100644 --- a/src/server/index.js +++ b/src/server/index.js @@ -14,6 +14,7 @@ import session from './routes/session' import pages from './pages' import adapters from '../adapters' import logger from '../lib/logger' +import redirect from './lib/redirect' // To work properly in production with OAuth providers the NEXTAUTH_URL // environment variable must be set. @@ -30,7 +31,12 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { // This is passed to all methods that handle responses, and must be called // when they are complete so that the serverless function knows when it is // safe to return and that no more data will be sent. - const done = resolve + // REVIEW: Why not just call res.end() as is, and remove the Promise wrapper? + res.end = () => { + resolve() + res.end() + } + res.redirect = redirect(req, res) const { url, query, body } = req const { @@ -173,20 +179,6 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { cookie.set(res, cookies.csrfToken.name, newCsrfTokenCookie, cookies.csrfToken.options) } - // Helper method for handling redirects, this is passed to all routes - // @TODO Refactor into a lib instead of passing as an option - // e.g. and call as redirect(req, res, url) - const redirect = (redirectUrl) => { - const reponseAsJson = !!((req.body && req.body.json === 'true')) - if (reponseAsJson) { - res.json({ url: redirectUrl }) - } else { - res.status(302).setHeader('Location', redirectUrl) - res.end() - } - return done() - } - // User provided options are overriden by other options, // except for the options with special handling above const options = { @@ -207,8 +199,7 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { session: sessionOptions, jwt: jwtOptions, events: eventsOptions, - callbacks: callbacksOptions, - redirect + callbacks: callbacksOptions } // If debug enabled, set ENV VAR so that logger logs debug messages @@ -220,90 +211,90 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { if (req.method === 'GET') { switch (action) { case 'providers': - providers(req, res, options, done) + providers(req, res, options) break case 'session': - session(req, res, options, done) + session(req, res, options) break case 'csrf': res.json({ csrfToken }) - return done() + return res.end() case 'signin': if (options.pages.signIn) { let redirectUrl = `${options.pages.signIn}${options.pages.signIn.includes('?') ? '&' : '?'}callbackUrl=${callbackUrl}` if (req.query.error) { redirectUrl = `${redirectUrl}&error=${req.query.error}` } - return redirect(redirectUrl) + return res.redirect(redirectUrl) } - pages.render(req, res, 'signin', { providers: Object.values(options.providers), callbackUrl, csrfToken }, done) + pages.render(req, res, 'signin', { providers: Object.values(options.providers), callbackUrl, csrfToken }) break case 'signout': - if (options.pages.signOut) { return redirect(`${options.pages.signOut}${options.pages.signOut.includes('?') ? '&' : '?'}error=${error}`) } + if (options.pages.signOut) { return res.redirect(`${options.pages.signOut}${options.pages.signOut.includes('?') ? '&' : '?'}error=${error}`) } - pages.render(req, res, 'signout', { csrfToken, callbackUrl }, done) + pages.render(req, res, 'signout', { csrfToken, callbackUrl }) break case 'callback': if (provider && options.providers[provider]) { - callback(req, res, options, done) + callback(req, res, options) } else { res.status(400).end(`Error: HTTP GET is not supported for ${url}`) - return done() + return res.end() } break case 'verify-request': - if (options.pages.verifyRequest) { return redirect(options.pages.verifyRequest) } + if (options.pages.verifyRequest) { return res.redirect(options.pages.verifyRequest) } - pages.render(req, res, 'verify-request', { }, done) + pages.render(req, res, 'verify-request') break case 'error': - if (options.pages.error) { return redirect(`${options.pages.error}${options.pages.error.includes('?') ? '&' : '?'}error=${error}`) } + if (options.pages.error) { return res.redirect(`${options.pages.error}${options.pages.error.includes('?') ? '&' : '?'}error=${error}`) } - pages.render(req, res, 'error', { error }, done) + pages.render(req, res, 'error', { error }) break default: res.status(404).end() - return done() + return res.end() } } else if (req.method === 'POST') { switch (action) { case 'signin': // Verified CSRF Token required for all sign in routes if (!csrfTokenVerified) { - return redirect(`${baseUrl()}/signin?csrf=true`) + return res.redirect(`${baseUrl()}/signin?csrf=true`) } if (provider && options.providers[provider]) { - signin(req, res, options, done) + signin(req, res, options) } break case 'signout': // Verified CSRF Token required for signout if (!csrfTokenVerified) { - return redirect(`${baseUrl()}/signout?csrf=true`) + return res.redirect(`${baseUrl()}/signout?csrf=true`) } - signout(req, res, options, done) + signout(req, res, options) break case 'callback': if (provider && options.providers[provider]) { // Verified CSRF Token required for credentials providers only if (options.providers[provider].type === 'credentials' && !csrfTokenVerified) { - return redirect(`${baseUrl()}/signin?csrf=true`) + return res.redirect(`${baseUrl()}/signin?csrf=true`) } - callback(req, res, options, done) + callback(req, res, options) } else { res.status(400).end(`Error: HTTP POST is not supported for ${url}`) - return done() + return res.end() } break default: res.status(400).end(`Error: HTTP POST is not supported for ${url}`) - return done() + return res.end() } } else { res.status(400).end(`Error: HTTP ${req.method} is not supported for ${url}`) - return done() + return res.end() } }) } diff --git a/src/server/lib/redirect.js b/src/server/lib/redirect.js new file mode 100644 index 0000000000..e8a6fd5ee8 --- /dev/null +++ b/src/server/lib/redirect.js @@ -0,0 +1,19 @@ +export default function redirect (req, res) { + // This is the one you will use. The wrapper is just to set it up in src/server/index. + return function redirect (redirectUrl) { + const reponseAsJson = !!((req.body && req.body.json === 'true')) + if (reponseAsJson) { + res.json({ url: redirectUrl }) + } else { + if (res.redirect) { + // Next.js makes it availeble by default https://nextjs.org/docs/api-routes/response-helpers + res.redirect(redirectUrl) + return + } else { + res.status(302).setHeader('Location', redirectUrl) + return res.end() + } + } + return res.end() + } +} diff --git a/src/server/pages/index.js b/src/server/pages/index.js index 61b54d6f07..26f20ed14e 100644 --- a/src/server/pages/index.js +++ b/src/server/pages/index.js @@ -4,7 +4,7 @@ import verifyRequest from './verify-request' import error from './error' import css from '../../css' -function render (req, res, page, props, done) { +function render (req, res, page, props = {}) { let html = '' switch (page) { case 'signin': @@ -18,7 +18,7 @@ function render (req, res, page, props, done) { break case 'error': html = error({ ...props, res }) - if (html === false) return done() + if (html === false) return res.end() break default: html = error(props) @@ -27,7 +27,7 @@ function render (req, res, page, props, done) { res.setHeader('Content-Type', 'text/html') res.send(`
${html}
`) - done() + res.end() } export default { diff --git a/src/server/routes/callback.js b/src/server/routes/callback.js index e6a73a85f9..1caf4c1c27 100644 --- a/src/server/routes/callback.js +++ b/src/server/routes/callback.js @@ -6,7 +6,7 @@ import logger from '../../lib/logger' import dispatchEvent from '../lib/dispatch-event' import baseUrl from '../../lib/baseUrl' -export default async (req, res, options, done) => { +export default async (req, res, options) => { const { provider: providerName, providers, @@ -18,8 +18,7 @@ export default async (req, res, options, done) => { jwt, events, callbacks, - csrfToken, - redirect + csrfToken } = options const provider = providers[providerName] const { type } = provider @@ -35,7 +34,7 @@ export default async (req, res, options, done) => { try { if (error) { logger.error('CALLBACK_OAUTH_ERROR', error) - return redirect(`${baseUrl()}/error?error=OAuthCallback`) + return res.redirect(`${baseUrl()}/error?error=OAuthCallback`) } // Make it easier to debug when adding a new provider @@ -50,7 +49,7 @@ export default async (req, res, options, done) => { // should at least be visible to developers what happened if it is an // error with the provider. if (!profile) { - return redirect(`${baseUrl()}/signin`) + return res.redirect(`${baseUrl()}/signin`) } // Check if user is allowed to sign in @@ -69,13 +68,13 @@ export default async (req, res, options, done) => { try { const signInCallbackResponse = await callbacks.signIn(userOrProfile, account, OAuthProfile) if (signInCallbackResponse === false) { - return redirect(`${baseUrl()}/error?error=AccessDenied`) + return res.redirect(`${baseUrl()}/error?error=AccessDenied`) } } catch (error) { if (error instanceof Error) { - return redirect(`${baseUrl()}/error?error=${encodeURIComponent(error)}`) + return res.redirect(`${baseUrl()}/error?error=${encodeURIComponent(error)}`) } else { - return redirect(error) + return res.redirect(error) } } @@ -110,32 +109,32 @@ export default async (req, res, options, done) => { // e.g. option to send users to a new account landing page on initial login // Note that the callback URL is preserved, so the journey can still be resumed if (isNewUser && pages.newUser) { - return redirect(`${pages.newUser}${pages.newUser.includes('?') ? '&' : '?'}callbackUrl=${encodeURIComponent(callbackUrl)}`) + return res.redirect(`${pages.newUser}${pages.newUser.includes('?') ? '&' : '?'}callbackUrl=${encodeURIComponent(callbackUrl)}`) } // Callback URL is already verified at this point, so safe to use if specified - return redirect(callbackUrl || baseUrl().origin) + return res.redirect(callbackUrl || baseUrl().origin) } catch (error) { if (error.name === 'AccountNotLinkedError') { // If the email on the account is already linked, but nto with this OAuth account - return redirect(`${baseUrl()}/error?error=OAuthAccountNotLinked`) + return res.redirect(`${baseUrl()}/error?error=OAuthAccountNotLinked`) } else if (error.name === 'CreateUserError') { - return redirect(`${baseUrl()}/error?error=OAuthCreateAccount`) + return res.redirect(`${baseUrl()}/error?error=OAuthCreateAccount`) } else { logger.error('OAUTH_CALLBACK_HANDLER_ERROR', error) - return redirect(`${baseUrl()}/error?error=Callback`) + return res.redirect(`${baseUrl()}/error?error=Callback`) } } }) } catch (error) { logger.error('OAUTH_CALLBACK_ERROR', error) - return redirect(`${baseUrl()}/error?error=Callback`) + return res.redirect(`${baseUrl()}/error?error=Callback`) } } else if (type === 'email') { try { if (!adapter) { logger.error('EMAIL_REQUIRES_ADAPTER_ERROR') - return redirect(`${baseUrl()}/error?error=Configuration`) + return res.redirect(`${baseUrl()}/error?error=Configuration`) } const { getVerificationRequest, deleteVerificationRequest, getUserByEmail } = await adapter.getAdapter(options) @@ -145,7 +144,7 @@ export default async (req, res, options, done) => { // Verify email and verification token exist in database const invite = await getVerificationRequest(email, verificationToken, secret, provider) if (!invite) { - return redirect(`${baseUrl()}/error?error=Verification`) + return res.redirect(`${baseUrl()}/error?error=Verification`) } // If verification token is valid, delete verification request token from @@ -160,13 +159,13 @@ export default async (req, res, options, done) => { try { const signInCallbackResponse = await callbacks.signIn(profile, account, { email }) if (signInCallbackResponse === false) { - return redirect(`${baseUrl()}/error?error=AccessDenied`) + return res.redirect(`${baseUrl()}/error?error=AccessDenied`) } } catch (error) { if (error instanceof Error) { - return redirect(`${baseUrl()}/error?error=${encodeURIComponent(error)}`) + return res.redirect(`${baseUrl()}/error?error=${encodeURIComponent(error)}`) } else { - return redirect(error) + return res.redirect(error) } } @@ -201,28 +200,28 @@ export default async (req, res, options, done) => { // e.g. option to send users to a new account landing page on initial login // Note that the callback URL is preserved, so the journey can still be resumed if (isNewUser && pages.newUser) { - return redirect(`${pages.newUser}${pages.newUser.includes('?') ? '&' : '?'}callbackUrl=${encodeURIComponent(callbackUrl)}`) + return res.redirect(`${pages.newUser}${pages.newUser.includes('?') ? '&' : '?'}callbackUrl=${encodeURIComponent(callbackUrl)}`) } // Callback URL is already verified at this point, so safe to use if specified - return redirect(callbackUrl || baseUrl().origin) + return res.redirect(callbackUrl || baseUrl().origin) } catch (error) { if (error.name === 'CreateUserError') { - return redirect(`${baseUrl()}/error?error=EmailCreateAccount`) + return res.redirect(`${baseUrl()}/error?error=EmailCreateAccount`) } else { logger.error('CALLBACK_EMAIL_ERROR', error) - return redirect(`${baseUrl()}/error?error=Callback`) + return res.redirect(`${baseUrl()}/error?error=Callback`) } } } else if (type === 'credentials' && req.method === 'POST') { if (!useJwtSession) { logger.error('CALLBACK_CREDENTIALS_JWT_ERROR', 'Signin in with credentials is only supported if JSON Web Tokens are enabled') - return redirect(`${baseUrl()}/error?error=Configuration`) + return res.redirect(`${baseUrl()}/error?error=Configuration`) } if (!provider.authorize) { logger.error('CALLBACK_CREDENTIALS_HANDLER_ERROR', 'Must define an authorize() handler to use credentials authentication provider') - return redirect(`${baseUrl()}/error?error=Configuration`) + return res.redirect(`${baseUrl()}/error?error=Configuration`) } const credentials = req.body @@ -231,13 +230,13 @@ export default async (req, res, options, done) => { try { userObjectReturnedFromAuthorizeHandler = await provider.authorize(credentials) if (!userObjectReturnedFromAuthorizeHandler) { - return redirect(`${baseUrl()}/error?error=CredentialsSignin&provider=${encodeURIComponent(provider.id)}`) + return res.redirect(`${baseUrl()}/error?error=CredentialsSignin&provider=${encodeURIComponent(provider.id)}`) } } catch (error) { if (error instanceof Error) { - return redirect(`${baseUrl()}/error?error=${encodeURIComponent(error)}`) + return res.redirect(`${baseUrl()}/error?error=${encodeURIComponent(error)}`) } else { - return redirect(error) + return res.redirect(error) } } @@ -247,13 +246,13 @@ export default async (req, res, options, done) => { try { const signInCallbackResponse = await callbacks.signIn(user, account, credentials) if (signInCallbackResponse === false) { - return redirect(`${baseUrl()}/error?error=AccessDenied`) + return res.redirect(`${baseUrl()}/error?error=AccessDenied`) } } catch (error) { if (error instanceof Error) { - return redirect(`${baseUrl()}/error?error=${encodeURIComponent(error)}`) + return res.redirect(`${baseUrl()}/error?error=${encodeURIComponent(error)}`) } else { - return redirect(error) + return res.redirect(error) } } @@ -275,9 +274,9 @@ export default async (req, res, options, done) => { await dispatchEvent(events.signIn, { user, account }) - return redirect(callbackUrl || baseUrl().origin) + return res.redirect(callbackUrl || baseUrl().origin) } else { res.status(500).end(`Error: Callback for provider type ${type} not supported`) - return done() + return res.end() } } diff --git a/src/server/routes/providers.js b/src/server/routes/providers.js index 09c1a40682..e38986b361 100644 --- a/src/server/routes/providers.js +++ b/src/server/routes/providers.js @@ -1,7 +1,7 @@ // Return a JSON object with a list of all outh providers currently configured // and their signin and callback URLs. This makes it possible to automatically // generate buttons for all providers when rendering client side. -export default (req, res, options, done) => { +export default (req, res, options) => { const { providers } = options const result = {} @@ -17,5 +17,5 @@ export default (req, res, options, done) => { res.setHeader('Content-Type', 'application/json') res.json(result) - return done() + return res.end() } diff --git a/src/server/routes/session.js b/src/server/routes/session.js index deb782c166..d09948639b 100644 --- a/src/server/routes/session.js +++ b/src/server/routes/session.js @@ -3,7 +3,7 @@ import cookie from '../lib/cookie' import logger from '../../lib/logger' import dispatchEvent from '../lib/dispatch-event' -export default async (req, res, options, done) => { +export default async (req, res, options) => { const { cookies, adapter, jwt, events, callbacks } = options const useJwtSession = options.session.jwt const sessionMaxAge = options.session.maxAge @@ -12,7 +12,7 @@ export default async (req, res, options, done) => { if (!sessionToken) { res.setHeader('Content-Type', 'application/json') res.json({}) - return done() + return res.end() } let response = {} @@ -100,5 +100,5 @@ export default async (req, res, options, done) => { res.setHeader('Content-Type', 'application/json') res.json(response) - return done() + return res.end() } diff --git a/src/server/routes/signin.js b/src/server/routes/signin.js index f1a02e2033..4bab7a8b52 100644 --- a/src/server/routes/signin.js +++ b/src/server/routes/signin.js @@ -4,21 +4,20 @@ import emailSignin from '../lib/signin/email' import logger from '../../lib/logger' import baseUrl from '../../lib/baseUrl' -export default async (req, res, options, done) => { +export default async (req, res, options) => { const { provider: providerName, providers, adapter, callbacks, - csrfToken, - redirect + csrfToken } = options const provider = providers[providerName] const { type } = provider if (!type) { res.status(500).end(`Error: Type not specified for ${provider}`) - return done() + return res.end() } if (type === 'oauth' && req.method === 'POST') { @@ -28,15 +27,15 @@ export default async (req, res, options, done) => { oAuthSignin(provider, csrfToken, (error, oAuthSigninUrl) => { if (error) { logger.error('SIGNIN_OAUTH_ERROR', error) - return redirect(`${baseUrl()}/error?error=OAuthSignin`) + return res.redirect(`${baseUrl()}/error?error=OAuthSignin`) } - return redirect(oAuthSigninUrl) + return res.redirect(oAuthSigninUrl) }, authParams) } else if (type === 'email' && req.method === 'POST') { if (!adapter) { logger.error('EMAIL_REQUIRES_ADAPTER_ERROR') - return redirect(`${baseUrl()}/error?error=Configuration`) + return res.redirect(`${baseUrl()}/error?error=Configuration`) } const { getUserByEmail } = await adapter.getAdapter(options) @@ -55,13 +54,13 @@ export default async (req, res, options, done) => { try { const signinCallbackResponse = await callbacks.signIn(profile, account, { email, verificationRequest: true }) if (signinCallbackResponse === false) { - return redirect(`${baseUrl()}/error?error=AccessDenied`) + return res.redirect(`${baseUrl()}/error?error=AccessDenied`) } } catch (error) { if (error instanceof Error) { - return redirect(`${baseUrl()}/error?error=${encodeURIComponent(error)}`) + return res.redirect(`${baseUrl()}/error?error=${encodeURIComponent(error)}`) } else { - return redirect(error) + return res.redirect(error) } } @@ -69,13 +68,13 @@ export default async (req, res, options, done) => { await emailSignin(email, provider, options) } catch (error) { logger.error('SIGNIN_EMAIL_ERROR', error) - return redirect(`${baseUrl()}/error?error=EmailSignin`) + return res.redirect(`${baseUrl()}/error?error=EmailSignin`) } - return redirect(`${baseUrl()}/verify-request?provider=${encodeURIComponent( + return res.redirect(`${baseUrl()}/verify-request?provider=${encodeURIComponent( provider.id )}&type=${encodeURIComponent(provider.type)}`) } else { - return redirect(`${baseUrl()}/signin`) + return res.redirect(`${baseUrl()}/signin`) } } diff --git a/src/server/routes/signout.js b/src/server/routes/signout.js index 84e8a252c7..3f145c9548 100644 --- a/src/server/routes/signout.js +++ b/src/server/routes/signout.js @@ -3,8 +3,8 @@ import cookie from '../lib/cookie' import logger from '../../lib/logger' import dispatchEvent from '../lib/dispatch-event' -export default async (req, res, options, done) => { - const { adapter, cookies, events, jwt, callbackUrl, redirect } = options +export default async (req, res, options) => { + const { adapter, cookies, events, jwt, callbackUrl } = options const useJwtSession = options.session.jwt const sessionToken = req.cookies[cookies.sessionToken.name] @@ -43,5 +43,5 @@ export default async (req, res, options, done) => { maxAge: 0 }) - return redirect(callbackUrl) + return res.redirect(callbackUrl) } From 740b3a8f98d9123528a8dd5df2c78aba207b600d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Orb=C3=A1n?= Date: Tue, 15 Dec 2020 20:45:39 +0100 Subject: [PATCH 05/11] refactor: move options to req --- src/server/index.js | 20 ++++++++++---------- src/server/lib/callback-url-handler.js | 4 ++-- src/server/routes/callback.js | 16 ++++++++-------- src/server/routes/providers.js | 4 ++-- src/server/routes/session.js | 10 +++++----- src/server/routes/signin.js | 8 ++++---- src/server/routes/signout.js | 8 ++++---- 7 files changed, 35 insertions(+), 35 deletions(-) diff --git a/src/server/index.js b/src/server/index.js index 427a777bc2..2fb5e70268 100644 --- a/src/server/index.js +++ b/src/server/index.js @@ -182,9 +182,8 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { // User provided options are overriden by other options, // except for the options with special handling above const options = { - // Defaults options can be overidden - debug: false, // Enable debug messages to be displayed - pages: {}, // Custom pages (e.g. sign in, sign out, errors) + debug: false, + pages: {}, // Custom options override defaults ...userSuppliedOptions, // These computed settings can values in userSuppliedOptions but override them @@ -201,20 +200,21 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { events: eventsOptions, callbacks: callbacksOptions } + req.options = options // If debug enabled, set ENV VAR so that logger logs debug messages if (options.debug === true) { process.env._NEXTAUTH_DEBUG = true } // Get / Set callback URL based on query param / cookie + validation - const callbackUrl = await callbackUrlHandler(req, res, options) + const callbackUrl = await callbackUrlHandler(req, res) if (req.method === 'GET') { switch (action) { case 'providers': - providers(req, res, options) + providers(req, res) break case 'session': - session(req, res, options) + session(req, res) break case 'csrf': res.json({ csrfToken }) @@ -235,7 +235,7 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { break case 'callback': if (provider && options.providers[provider]) { - callback(req, res, options) + callback(req, res) } else { res.status(400).end(`Error: HTTP GET is not supported for ${url}`) return res.end() @@ -264,7 +264,7 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { } if (provider && options.providers[provider]) { - signin(req, res, options) + signin(req, res) } break case 'signout': @@ -273,7 +273,7 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { return res.redirect(`${baseUrl()}/signout?csrf=true`) } - signout(req, res, options) + signout(req, res) break case 'callback': if (provider && options.providers[provider]) { @@ -282,7 +282,7 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { return res.redirect(`${baseUrl()}/signin?csrf=true`) } - callback(req, res, options) + callback(req, res) } else { res.status(400).end(`Error: HTTP POST is not supported for ${url}`) return res.end() diff --git a/src/server/lib/callback-url-handler.js b/src/server/lib/callback-url-handler.js index 5df3e76434..89832f75b9 100644 --- a/src/server/lib/callback-url-handler.js +++ b/src/server/lib/callback-url-handler.js @@ -1,10 +1,10 @@ import cookie from '../lib/cookie' import baseUrl from '../../lib/baseUrl' -export default async (req, res, options) => { +export default async (req, res) => { const { query } = req const { body } = req - const { cookies, defaultCallbackUrl, callbacks } = options + const { cookies, defaultCallbackUrl, callbacks } = req.options const homepage = baseUrl().origin // Handle preserving and validating callback URLs diff --git a/src/server/routes/callback.js b/src/server/routes/callback.js index 1caf4c1c27..e6963cb4b0 100644 --- a/src/server/routes/callback.js +++ b/src/server/routes/callback.js @@ -6,7 +6,7 @@ import logger from '../../lib/logger' import dispatchEvent from '../lib/dispatch-event' import baseUrl from '../../lib/baseUrl' -export default async (req, res, options) => { +export default async (req, res) => { const { provider: providerName, providers, @@ -19,11 +19,11 @@ export default async (req, res, options) => { events, callbacks, csrfToken - } = options + } = req.options const provider = providers[providerName] const { type } = provider - const useJwtSession = options.session.jwt - const sessionMaxAge = options.session.maxAge + const useJwtSession = req.options.session.jwt + const sessionMaxAge = req.options.session.maxAge // Get session ID (if set) const sessionToken = req.cookies ? req.cookies[cookies.sessionToken.name] : null @@ -58,7 +58,7 @@ export default async (req, res, options) => { // (that just means it's a new user signing in for the first time). let userOrProfile = profile if (adapter) { - const { getUserByProviderAccountId } = await adapter.getAdapter(options) + const { getUserByProviderAccountId } = await adapter.getAdapter(req.options) const userFromProviderAccountId = await getUserByProviderAccountId(account.provider, account.id) if (userFromProviderAccountId) { userOrProfile = userFromProviderAccountId @@ -79,7 +79,7 @@ export default async (req, res, options) => { } // Sign user in - const { user, session, isNewUser } = await callbackHandler(sessionToken, profile, account, options) + const { user, session, isNewUser } = await callbackHandler(sessionToken, profile, account, req.options) if (useJwtSession) { const defaultJwtPayload = { @@ -137,7 +137,7 @@ export default async (req, res, options) => { return res.redirect(`${baseUrl()}/error?error=Configuration`) } - const { getVerificationRequest, deleteVerificationRequest, getUserByEmail } = await adapter.getAdapter(options) + const { getVerificationRequest, deleteVerificationRequest, getUserByEmail } = await adapter.getAdapter(req.options) const verificationToken = req.query.token const email = req.query.email @@ -170,7 +170,7 @@ export default async (req, res, options) => { } // Sign user in - const { user, session, isNewUser } = await callbackHandler(sessionToken, profile, account, options) + const { user, session, isNewUser } = await callbackHandler(sessionToken, profile, account, req.options) if (useJwtSession) { const defaultJwtPayload = { diff --git a/src/server/routes/providers.js b/src/server/routes/providers.js index e38986b361..183c3ff583 100644 --- a/src/server/routes/providers.js +++ b/src/server/routes/providers.js @@ -1,8 +1,8 @@ // Return a JSON object with a list of all outh providers currently configured // and their signin and callback URLs. This makes it possible to automatically // generate buttons for all providers when rendering client side. -export default (req, res, options) => { - const { providers } = options +export default (req, res) => { + const { providers } = req.options const result = {} Object.entries(providers).forEach(([provider, providerConfig]) => { diff --git a/src/server/routes/session.js b/src/server/routes/session.js index d09948639b..95be8d2629 100644 --- a/src/server/routes/session.js +++ b/src/server/routes/session.js @@ -3,10 +3,10 @@ import cookie from '../lib/cookie' import logger from '../../lib/logger' import dispatchEvent from '../lib/dispatch-event' -export default async (req, res, options) => { - const { cookies, adapter, jwt, events, callbacks } = options - const useJwtSession = options.session.jwt - const sessionMaxAge = options.session.maxAge +export default async (req, res) => { + const { cookies, adapter, jwt, events, callbacks } = req.options + const useJwtSession = req.options.session.jwt + const sessionMaxAge = req.options.session.maxAge const sessionToken = req.cookies[cookies.sessionToken.name] if (!sessionToken) { @@ -58,7 +58,7 @@ export default async (req, res, options) => { } } else { try { - const { getUser, getSession, updateSession } = await adapter.getAdapter(options) + const { getUser, getSession, updateSession } = await adapter.getAdapter(req.options) const session = await getSession(sessionToken) if (session) { // Trigger update to session object to update session expiry diff --git a/src/server/routes/signin.js b/src/server/routes/signin.js index 4bab7a8b52..0313765784 100644 --- a/src/server/routes/signin.js +++ b/src/server/routes/signin.js @@ -4,14 +4,14 @@ import emailSignin from '../lib/signin/email' import logger from '../../lib/logger' import baseUrl from '../../lib/baseUrl' -export default async (req, res, options) => { +export default async (req, res) => { const { provider: providerName, providers, adapter, callbacks, csrfToken - } = options + } = req.options const provider = providers[providerName] const { type } = provider @@ -37,7 +37,7 @@ export default async (req, res, options) => { logger.error('EMAIL_REQUIRES_ADAPTER_ERROR') return res.redirect(`${baseUrl()}/error?error=Configuration`) } - const { getUserByEmail } = await adapter.getAdapter(options) + const { getUserByEmail } = await adapter.getAdapter(req.options) // Note: Technically the part of the email address local mailbox element // (everything before the @ symbol) should be treated as 'case sensitive' @@ -65,7 +65,7 @@ export default async (req, res, options) => { } try { - await emailSignin(email, provider, options) + await emailSignin(email, provider, req.options) } catch (error) { logger.error('SIGNIN_EMAIL_ERROR', error) return res.redirect(`${baseUrl()}/error?error=EmailSignin`) diff --git a/src/server/routes/signout.js b/src/server/routes/signout.js index 3f145c9548..214fc95808 100644 --- a/src/server/routes/signout.js +++ b/src/server/routes/signout.js @@ -3,9 +3,9 @@ import cookie from '../lib/cookie' import logger from '../../lib/logger' import dispatchEvent from '../lib/dispatch-event' -export default async (req, res, options) => { - const { adapter, cookies, events, jwt, callbackUrl } = options - const useJwtSession = options.session.jwt +export default async (req, res) => { + const { adapter, cookies, events, jwt, callbackUrl } = req.options + const useJwtSession = req.options.session.jwt const sessionToken = req.cookies[cookies.sessionToken.name] if (useJwtSession) { @@ -18,7 +18,7 @@ export default async (req, res, options) => { } } else { // Get session from database - const { getSession, deleteSession } = await adapter.getAdapter(options) + const { getSession, deleteSession } = await adapter.getAdapter(req.options) try { // Dispatch signout event From 24b08dfb01c6ea64ee98d0a179cf04b15a796677 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Orb=C3=A1n?= Date: Tue, 15 Dec 2020 21:07:01 +0100 Subject: [PATCH 06/11] refactor: improve IntelliSense, name all functions --- src/server/index.js | 15 ++++---- src/server/lib/callback-handler.js | 24 +++++++------ src/server/lib/callback-url-handler.js | 4 +-- src/server/lib/cookie.js | 24 ++++++------- src/server/lib/dispatch-event.js | 2 +- src/server/lib/events.js | 49 +++++++++----------------- src/server/lib/oauth/callback.js | 21 +++++------ src/server/lib/oauth/client.js | 10 +++--- src/server/lib/providers.js | 13 +++---- src/server/lib/signin/email.js | 2 +- src/server/lib/signin/oauth.js | 2 +- src/server/pages/error.js | 2 +- src/server/pages/index.js | 6 +--- src/server/pages/signin.js | 6 ++-- src/server/pages/signout.js | 2 +- src/server/routes/callback.js | 6 ++-- src/server/routes/providers.js | 31 ++++++++-------- src/server/routes/session.js | 9 +++-- src/server/routes/signin.js | 4 +-- src/server/routes/signout.js | 6 ++-- 20 files changed, 113 insertions(+), 125 deletions(-) diff --git a/src/server/index.js b/src/server/index.js index 2fb5e70268..ee892c9d34 100644 --- a/src/server/index.js +++ b/src/server/index.js @@ -1,17 +1,17 @@ import { createHash, randomBytes } from 'crypto' import jwt from '../lib/jwt' import baseUrl from '../lib/baseUrl' -import cookie from './lib/cookie' +import * as cookie from './lib/cookie' import callbackUrlHandler from './lib/callback-url-handler' import parseProviders from './lib/providers' -import events from './lib/events' +import * as events from './lib/events' import * as defaultCallbacks from './lib/defaultCallbacks' import providers from './routes/providers' import signin from './routes/signin' import signout from './routes/signout' import callback from './routes/callback' import session from './routes/session' -import pages from './pages' +import renderPage from './pages' import adapters from '../adapters' import logger from '../lib/logger' import redirect from './lib/redirect' @@ -226,12 +226,12 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { return res.redirect(redirectUrl) } - pages.render(req, res, 'signin', { providers: Object.values(options.providers), callbackUrl, csrfToken }) + renderPage(req, res, 'signin', { providers: Object.values(options.providers), callbackUrl, csrfToken }) break case 'signout': if (options.pages.signOut) { return res.redirect(`${options.pages.signOut}${options.pages.signOut.includes('?') ? '&' : '?'}error=${error}`) } - pages.render(req, res, 'signout', { csrfToken, callbackUrl }) + renderPage(req, res, 'signout', { csrfToken, callbackUrl }) break case 'callback': if (provider && options.providers[provider]) { @@ -244,12 +244,12 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { case 'verify-request': if (options.pages.verifyRequest) { return res.redirect(options.pages.verifyRequest) } - pages.render(req, res, 'verify-request') + renderPage(req, res, 'verify-request') break case 'error': if (options.pages.error) { return res.redirect(`${options.pages.error}${options.pages.error.includes('?') ? '&' : '?'}error=${error}`) } - pages.render(req, res, 'error', { error }) + renderPage(req, res, 'error', { error }) break default: res.status(404).end() @@ -299,6 +299,7 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { }) } +/** Tha main entry point to next-auth */ export default async function NextAuth (...args) { if (args.length === 1) { return (req, res) => NextAuthHandler(req, res, args[0]) diff --git a/src/server/lib/callback-handler.js b/src/server/lib/callback-handler.js index aea6d48b68..69292f19af 100644 --- a/src/server/lib/callback-handler.js +++ b/src/server/lib/callback-handler.js @@ -1,17 +1,19 @@ -// This function handles the complex flow of signing users in, and either creating, -// linking (or not linking) accounts depending on if the user is currently logged -// in, if they have account already and the authentication mechanism they are using. -// -// It prevents insecure behaviour, such as linking OAuth accounts unless a user is -// signed in and authenticated with an existing valid account. -// -// All verification (e.g. OAuth flows or email address verificaiton flows) are -// done prior to this handler being called to avoid additonal complexity in this -// handler. import { AccountNotLinkedError } from '../../lib/errors' import dispatchEvent from '../lib/dispatch-event' -export default async (sessionToken, profile, providerAccount, options) => { +/** + * This function handles the complex flow of signing users in, and either creating, + * linking (or not linking) accounts depending on if the user is currently logged + * in, if they have account already and the authentication mechanism they are using. + * + * It prevents insecure behaviour, such as linking OAuth accounts unless a user is + * signed in and authenticated with an existing valid account. + * + * All verification (e.g. OAuth flows or email address verificaiton flows) are + * done prior to this handler being called to avoid additonal complexity in this + * handler. + */ +export default async function callbackHandler (sessionToken, profile, providerAccount, options) { try { // Input validation if (!profile) { throw new Error('Missing profile') } diff --git a/src/server/lib/callback-url-handler.js b/src/server/lib/callback-url-handler.js index 89832f75b9..8bbaac2337 100644 --- a/src/server/lib/callback-url-handler.js +++ b/src/server/lib/callback-url-handler.js @@ -1,7 +1,7 @@ -import cookie from '../lib/cookie' +import * as cookie from '../lib/cookie' import baseUrl from '../../lib/baseUrl' -export default async (req, res) => { +export default async function callbackUrlHandler (req, res) { const { query } = req const { body } = req const { cookies, defaultCallbackUrl, callbacks } = req.options diff --git a/src/server/lib/cookie.js b/src/server/lib/cookie.js index 078aaca3f3..a2f5ba0e6c 100644 --- a/src/server/lib/cookie.js +++ b/src/server/lib/cookie.js @@ -1,12 +1,14 @@ -// Function to set cookies server side -// -// Credit to @huv1k and @jshttp contributors for the code which this is based on (MIT License). -// * https://github.com/jshttp/cookie/blob/master/index.js -// * https://github.com/zeit/next.js/blob/master/examples/api-routes-middleware/utils/cookies.js -// -// As only partial functionlity is required, only the code we need has been incorporated here -// (with fixes for specific issues) to keep dependancy size down. -const set = (res, name, value, options = {}) => { +/** + * Function to set cookies server side + * + * Credit to @huv1k and @jshttp contributors for the code which this is based on (MIT License). + * * https://github.com/jshttp/cookie/blob/master/index.js + * * https://github.com/zeit/next.js/blob/master/examples/api-routes-middleware/utils/cookies.js + * + * As only partial functionlity is required, only the code we need has been incorporated here + * (with fixes for specific issues) to keep dependancy size down. + */ +export function set (res, name, value, options = {}) { const stringValue = typeof value === 'object' ? 'j:' + JSON.stringify(value) : String(value) if ('maxAge' in options) { @@ -98,7 +100,3 @@ function _serialize (name, val, options) { return str } - -export default { - set -} diff --git a/src/server/lib/dispatch-event.js b/src/server/lib/dispatch-event.js index 68dbb35883..0c5740c305 100644 --- a/src/server/lib/dispatch-event.js +++ b/src/server/lib/dispatch-event.js @@ -1,6 +1,6 @@ import logger from '../../lib/logger' -export default async (event, message) => { +export default async function dispatchEvent (event, message) { try { await event(message) } catch (e) { diff --git a/src/server/lib/events.js b/src/server/lib/events.js index 6b53a4a0d3..caa14794f9 100644 --- a/src/server/lib/events.js +++ b/src/server/lib/events.js @@ -1,38 +1,23 @@ -const signIn = async (message) => { - // Event triggered on successful sign in -} +/** Event triggered on successful sign in */ +export async function signIn (message) {} -const signOut = async (message) => { - // Event triggered on sign out -} +/** Event triggered on sign out */ +export async function signOut (message) {} -const createUser = async (message) => { - // Event triggered on user creation -} +/** Event triggered on user creation */ +export async function createUser (message) {} -const updateUser = async (message) => { - // Event triggered when a user object is updated -} +/** Event triggered when a user object is updated */ +export async function updateUser (message) {} -const linkAccount = async (message) => { - // Event triggered when an account is linked to a user -} +/** Event triggered when an account is linked to a user */ +export async function linkAccount (message) {} -const session = async (message) => { - // Event triggered when a session is active -} +/** Event triggered when a session is active */ +export async function session (message) {} -const error = async (message) => { - // @TODO Event triggered when something goes wrong in an authentication flow - // This event may be fired multiple times when an error occurs -} - -export default { - signIn, - signOut, - createUser, - updateUser, - linkAccount, - session, - error -} +/** + * @TODO Event triggered when something goes wrong in an authentication flow + * This event may be fired multiple times when an error occurs + */ +export async function error (message) {} diff --git a/src/server/lib/oauth/callback.js b/src/server/lib/oauth/callback.js index 16d6eaeae7..0a1ce0d80a 100644 --- a/src/server/lib/oauth/callback.js +++ b/src/server/lib/oauth/callback.js @@ -1,19 +1,20 @@ - import { createHash } from 'crypto' import querystring from 'querystring' import jwtDecode from 'jwt-decode' import oAuthClient from './client' import logger from '../../../lib/logger' -// @TODO Refactor monkey patching in _getOAuthAccessToken() and _get() -// These methods have been forked from `node-oauth` to fix bugs; it may make -// sense to migrate all the methods we need from node-oauth to nexth-auth (with -// appropriate credit) to make it easier to maintain and address issues as they -// come up, as the node-oauth package does not seem to be actively maintained. - -// @TODO Refactor to use promises and not callbacks -// @TODO Refactor to use jsonwebtoken instead of jwt-decode & remove dependancy -export default async (req, provider, csrfToken, callback) => { +/** + * @TODO Refactor monkey patching in _getOAuthAccessToken() and _get() + * These methods have been forked from `node-oauth` to fix bugs; it may make + * sense to migrate all the methods we need from node-oauth to nexth-auth (with + * appropriate credit) to make it easier to maintain and address issues as they + * come up, as the node-oauth package does not seem to be actively maintained. + + * @TODO Refactor to use promises and not callbacks + * @TODO Refactor to use jsonwebtoken instead of jwt-decode & remove dependancy + */ +export default async function oAuthCallback (req, provider, csrfToken, callback) { // The "user" object is specific to apple provider and is provided on first sign in // e.g. {"name":{"firstName":"Johnny","lastName":"Appleseed"},"email":"johnny.appleseed@nextauth.com"} let { oauth_token, oauth_verifier, code, user, state } = req.query // eslint-disable-line camelcase diff --git a/src/server/lib/oauth/client.js b/src/server/lib/oauth/client.js index 18bdfff581..67b82748fd 100644 --- a/src/server/lib/oauth/client.js +++ b/src/server/lib/oauth/client.js @@ -1,9 +1,11 @@ -// @TODO Refactor to remove dependancy on 'oauth' package -// It is already quite monkey patched, we don't use all the features and and it -// would be easier to maintain if all the code was native to next-auth. import { OAuth, OAuth2 } from 'oauth' -export default (provider) => { +/** + * @TODO Refactor to remove dependancy on 'oauth' package + * It is already quite monkey patched, we don't use all the features and and it + * would be easier to maintain if all the code was native to next-auth. + */ +export default function oAuthClient (provider) { if (provider.version && provider.version.startsWith('2.')) { // Handle OAuth v2.x const basePath = new URL(provider.authorizationUrl).origin diff --git a/src/server/lib/providers.js b/src/server/lib/providers.js index 722f9c3024..b36180a796 100644 --- a/src/server/lib/providers.js +++ b/src/server/lib/providers.js @@ -1,16 +1,13 @@ import baseUrl from '../../lib/baseUrl' -export default function parseProviders (_providers) { - const providers = {} - - _providers.forEach(provider => { +export default function parseProviders (providers) { + return providers.reduce((acc, provider) => { const providerId = provider.id - providers[providerId] = { + acc[providerId] = { ...provider, signinUrl: `${baseUrl()}/signin/${providerId}`, callbackUrl: `${baseUrl()}/callback/${providerId}` } - }) - - return providers + return acc + }, {}) } diff --git a/src/server/lib/signin/email.js b/src/server/lib/signin/email.js index 403e20196d..a8e7ac0eed 100644 --- a/src/server/lib/signin/email.js +++ b/src/server/lib/signin/email.js @@ -1,7 +1,7 @@ import { randomBytes } from 'crypto' import baseUrl from '../../../lib/baseUrl' -export default async (email, provider, options) => { +export default async function email (email, provider, options) { try { const { adapter } = options diff --git a/src/server/lib/signin/oauth.js b/src/server/lib/signin/oauth.js index 13eada068c..1b968b79fd 100644 --- a/src/server/lib/signin/oauth.js +++ b/src/server/lib/signin/oauth.js @@ -2,7 +2,7 @@ import oAuthClient from '../oauth/client' import { createHash } from 'crypto' import logger from '../../../lib/logger' -export default (provider, csrfToken, callback, authParams) => { +export default function oauth (provider, csrfToken, callback, authParams) { const { callbackUrl } = provider const client = oAuthClient(provider) if (provider.version && provider.version.startsWith('2.')) { diff --git a/src/server/pages/error.js b/src/server/pages/error.js index 870fc65d45..dac6b5dc0c 100644 --- a/src/server/pages/error.js +++ b/src/server/pages/error.js @@ -2,7 +2,7 @@ import { h } from 'preact' // eslint-disable-line no-unused-vars import render from 'preact-render-to-string' import baseUrl from '../../lib/baseUrl' -export default ({ error, res }) => { +export default function error ({ error, res }) { const signinPageUrl = `${baseUrl()}/signin` let statusCode = 200 diff --git a/src/server/pages/index.js b/src/server/pages/index.js index 26f20ed14e..2dd31fe88c 100644 --- a/src/server/pages/index.js +++ b/src/server/pages/index.js @@ -4,7 +4,7 @@ import verifyRequest from './verify-request' import error from './error' import css from '../../css' -function render (req, res, page, props = {}) { +export default function renderPage (req, res, page, props = {}) { let html = '' switch (page) { case 'signin': @@ -29,7 +29,3 @@ function render (req, res, page, props = {}) { res.send(`
${html}
`) res.end() } - -export default { - render -} diff --git a/src/server/pages/signin.js b/src/server/pages/signin.js index 5c2082c607..409de87c6b 100644 --- a/src/server/pages/signin.js +++ b/src/server/pages/signin.js @@ -1,7 +1,7 @@ import { h } from 'preact' // eslint-disable-line no-unused-vars import render from 'preact-render-to-string' -export default ({ req, csrfToken, providers, callbackUrl }) => { +export default function signin ({ req, csrfToken, providers, callbackUrl }) { const { email, error } = req.query // We only want to render providers @@ -59,8 +59,8 @@ export default ({ req, csrfToken, providers, callbackUrl }) => { } {(provider.type === 'email' || provider.type === 'credentials') && (i > 0) && - providersToRender[i - 1].type !== 'email' && providersToRender[i - 1].type !== 'credentials' && -
} + providersToRender[i - 1].type !== 'email' && providersToRender[i - 1].type !== 'credentials' && +
} {provider.type === 'email' &&
diff --git a/src/server/pages/signout.js b/src/server/pages/signout.js index 97bbcdad78..4206f1559d 100644 --- a/src/server/pages/signout.js +++ b/src/server/pages/signout.js @@ -2,7 +2,7 @@ import { h } from 'preact' // eslint-disable-line no-unused-vars import render from 'preact-render-to-string' import baseUrl from '../../lib/baseUrl' -export default ({ csrfToken }) => { +export default function signout ({ csrfToken }) { return render(

Are you sure you want to sign out?

diff --git a/src/server/routes/callback.js b/src/server/routes/callback.js index e6963cb4b0..cc9a9950af 100644 --- a/src/server/routes/callback.js +++ b/src/server/routes/callback.js @@ -1,12 +1,12 @@ -// Handle callbacks from login services import oAuthCallback from '../lib/oauth/callback' import callbackHandler from '../lib/callback-handler' -import cookie from '../lib/cookie' +import * as cookie from '../lib/cookie' import logger from '../../lib/logger' import dispatchEvent from '../lib/dispatch-event' import baseUrl from '../../lib/baseUrl' -export default async (req, res) => { +/** Handle callbacks from login services */ +export default async function callback (req, res) { const { provider: providerName, providers, diff --git a/src/server/routes/providers.js b/src/server/routes/providers.js index 183c3ff583..4f2f0eac0c 100644 --- a/src/server/routes/providers.js +++ b/src/server/routes/providers.js @@ -1,19 +1,22 @@ -// Return a JSON object with a list of all outh providers currently configured -// and their signin and callback URLs. This makes it possible to automatically -// generate buttons for all providers when rendering client side. -export default (req, res) => { +/** + * Return a JSON object with a list of all outh providers currently configured + * and their signin and callback URLs. This makes it possible to automatically + * generate buttons for all providers when rendering client side. + */ +export default function providers (req, res) { const { providers } = req.options - const result = {} - Object.entries(providers).forEach(([provider, providerConfig]) => { - result[provider] = { - id: provider, - name: providerConfig.name, - type: providerConfig.type, - signinUrl: providerConfig.signinUrl, - callbackUrl: providerConfig.callbackUrl - } - }) + const result = Object.entries(providers) + .reduce((acc, [provider, providerConfig]) => ({ + ...acc, + [provider]: { + id: provider, + name: providerConfig.name, + type: providerConfig.type, + signinUrl: providerConfig.signinUrl, + callbackUrl: providerConfig.callbackUrl + } + }), {}) res.setHeader('Content-Type', 'application/json') res.json(result) diff --git a/src/server/routes/session.js b/src/server/routes/session.js index 95be8d2629..d8d3de3a04 100644 --- a/src/server/routes/session.js +++ b/src/server/routes/session.js @@ -1,9 +1,12 @@ -// Return a session object (without any private fields) for Single Page App clients -import cookie from '../lib/cookie' +import * as cookie from '../lib/cookie' import logger from '../../lib/logger' import dispatchEvent from '../lib/dispatch-event' -export default async (req, res) => { +/** + * Return a session object (without any private fields) + * for Single Page App clients + */ +export default async function session (req, res) { const { cookies, adapter, jwt, events, callbacks } = req.options const useJwtSession = req.options.session.jwt const sessionMaxAge = req.options.session.maxAge diff --git a/src/server/routes/signin.js b/src/server/routes/signin.js index 0313765784..24094d9eab 100644 --- a/src/server/routes/signin.js +++ b/src/server/routes/signin.js @@ -1,10 +1,10 @@ -// Handle requests to /api/auth/signin import oAuthSignin from '../lib/signin/oauth' import emailSignin from '../lib/signin/email' import logger from '../../lib/logger' import baseUrl from '../../lib/baseUrl' -export default async (req, res) => { +/** Handle requests to /api/auth/signin */ +export default async function signin (req, res) { const { provider: providerName, providers, diff --git a/src/server/routes/signout.js b/src/server/routes/signout.js index 214fc95808..175cbfb238 100644 --- a/src/server/routes/signout.js +++ b/src/server/routes/signout.js @@ -1,9 +1,9 @@ -// Handle requests to /api/auth/signout -import cookie from '../lib/cookie' +import * as cookie from '../lib/cookie' import logger from '../../lib/logger' import dispatchEvent from '../lib/dispatch-event' -export default async (req, res) => { +/** Handle requests to /api/auth/signout */ +export default async function signout (req, res) { const { adapter, cookies, events, jwt, callbackUrl } = req.options const useJwtSession = req.options.session.jwt const sessionToken = req.cookies[cookies.sessionToken.name] From 0bac6cf0d582f4e7e820741de6f9ae20976acd2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Orb=C3=A1n?= Date: Tue, 15 Dec 2020 21:41:22 +0100 Subject: [PATCH 07/11] fix(lint): fix lint errors --- src/adapters/fauna/index.js | 2 +- src/server/index.js | 8 ++------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/adapters/fauna/index.js b/src/adapters/fauna/index.js index f86b875cfb..f209f44999 100644 --- a/src/adapters/fauna/index.js +++ b/src/adapters/fauna/index.js @@ -270,7 +270,7 @@ const Adapter = (config, options = {}) => { _debug('getSession', sessionToken) try { - var session = await faunaClient.query( + const session = await faunaClient.query( q.Get( q.Match( q.Index('session_by_token'), diff --git a/src/server/index.js b/src/server/index.js index dc2d7e1ac3..05dc23721a 100644 --- a/src/server/index.js +++ b/src/server/index.js @@ -42,12 +42,8 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { const error = 'Cannot find [...nextauth].js in pages/api/auth. Make sure the filename is written correctly.' logger.error('MISSING_NEXTAUTH_API_ROUTE_ERROR', error) - res - .status(500) - .end( - `Error: ${error}` - ) - return done() + res.status(500).end(`Error: ${error}`) + return res.end() } const { url, query, body } = req From a0ebbb25fff49b123bc731fd22ceb78663cfcd36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Orb=C3=A1n?= Date: Tue, 15 Dec 2020 21:57:04 +0100 Subject: [PATCH 08/11] refactor: remove jwt-decode dependency --- package-lock.json | 11 ----------- package.json | 1 - src/server/lib/oauth/callback.js | 3 +-- 3 files changed, 1 insertion(+), 14 deletions(-) diff --git a/package-lock.json b/package-lock.json index fc3049b699..c9298a0ed4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5199,11 +5199,6 @@ "safe-buffer": "^5.0.1" } }, - "jwt-decode": { - "version": "2.2.0", - "resolved": "https://registry.npmjs.org/jwt-decode/-/jwt-decode-2.2.0.tgz", - "integrity": "sha1-fYa9VmefWM5qhHBKZX3TkruoGnk=" - }, "kind-of": { "version": "6.0.3", "resolved": "https://registry.npmjs.org/kind-of/-/kind-of-6.0.3.tgz", @@ -5908,12 +5903,6 @@ "integrity": "sha1-Sr6/7tdUHywnrPspvbvRXI1bpPc=", "dev": true }, - "nice-try": { - "version": "1.0.5", - "resolved": "https://registry.npmjs.org/nice-try/-/nice-try-1.0.5.tgz", - "integrity": "sha512-1nh45deeb5olNY7eX82BkPO7SSxR5SSYJiPTrTdFUVYwAl8CKMA5N9PjTYkHiRjisVcxcQ1HXdLhx2qxxJzLNQ==", - "dev": true - }, "node-fetch": { "version": "2.6.1", "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-2.6.1.tgz", diff --git a/package.json b/package.json index f3d47a8e2c..ebe7ea1056 100644 --- a/package.json +++ b/package.json @@ -47,7 +47,6 @@ "futoin-hkdf": "^1.3.2", "jose": "^1.27.2", "jsonwebtoken": "^8.5.1", - "jwt-decode": "^2.2.0", "nodemailer": "^6.4.16", "oauth": "^0.9.15", "preact": "^10.4.1", diff --git a/src/server/lib/oauth/callback.js b/src/server/lib/oauth/callback.js index 0a1ce0d80a..122652f28a 100644 --- a/src/server/lib/oauth/callback.js +++ b/src/server/lib/oauth/callback.js @@ -1,6 +1,6 @@ import { createHash } from 'crypto' import querystring from 'querystring' -import jwtDecode from 'jwt-decode' +import { decode as jwtDecode } from 'jsonwebtoken' import oAuthClient from './client' import logger from '../../../lib/logger' @@ -12,7 +12,6 @@ import logger from '../../../lib/logger' * come up, as the node-oauth package does not seem to be actively maintained. * @TODO Refactor to use promises and not callbacks - * @TODO Refactor to use jsonwebtoken instead of jwt-decode & remove dependancy */ export default async function oAuthCallback (req, provider, csrfToken, callback) { // The "user" object is specific to apple provider and is provided on first sign in From 27ecee874465d258e3b552340ce5efad320bc8ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Orb=C3=A1n?= Date: Tue, 15 Dec 2020 22:34:31 +0100 Subject: [PATCH 09/11] refactor: refactor some callbacks to Promises --- src/server/lib/oauth/callback.js | 102 +++++++++---------- src/server/routes/callback.js | 162 +++++++++++++++---------------- 2 files changed, 132 insertions(+), 132 deletions(-) diff --git a/src/server/lib/oauth/callback.js b/src/server/lib/oauth/callback.js index 122652f28a..5f73431c2a 100644 --- a/src/server/lib/oauth/callback.js +++ b/src/server/lib/oauth/callback.js @@ -4,6 +4,14 @@ import { decode as jwtDecode } from 'jsonwebtoken' import oAuthClient from './client' import logger from '../../../lib/logger' +class OAuthCallbackError extends Error { + constructor (message) { + super(message) + this.name = 'OAuthCallbackError' + this.message = message + } +} + /** * @TODO Refactor monkey patching in _getOAuthAccessToken() and _get() * These methods have been forked from `node-oauth` to fix bugs; it may make @@ -13,13 +21,13 @@ import logger from '../../../lib/logger' * @TODO Refactor to use promises and not callbacks */ -export default async function oAuthCallback (req, provider, csrfToken, callback) { - // The "user" object is specific to apple provider and is provided on first sign in +export default async function oAuthCallback (req, provider, csrfToken) { + // The "user" object is specific to the Apple provider and is provided on first sign in // e.g. {"name":{"firstName":"Johnny","lastName":"Appleseed"},"email":"johnny.appleseed@nextauth.com"} let { oauth_token, oauth_verifier, code, user, state } = req.query // eslint-disable-line camelcase const client = oAuthClient(provider) - if (provider.version && provider.version.startsWith('2.')) { + if (provider.version?.startsWith('2.')) { // For OAuth 2.0 flows, check state returned and matches expected value // (a hash of the NextAuth.js CSRF token). // @@ -28,7 +36,7 @@ export default async function oAuthCallback (req, provider, csrfToken, callback) if (!Object.prototype.hasOwnProperty.call(provider, 'state') || provider.state === true) { const expectedState = createHash('sha256').update(csrfToken).digest('hex') if (state !== expectedState) { - return callback(new Error('Invalid state returned from OAuth provider')) + throw new OAuthCallbackError('Invalid state returned from OAuth provider') } } @@ -41,7 +49,7 @@ export default async function oAuthCallback (req, provider, csrfToken, callback) user = body.user != null ? JSON.parse(body.user) : null } catch (e) { logger.error('OAUTH_CALLBACK_HANDLER_ERROR', e, req.body, provider.id, code) - return callback() + throw new OAuthCallbackError() } } @@ -58,10 +66,10 @@ export default async function oAuthCallback (req, provider, csrfToken, callback) await client.getOAuthAccessToken( code, provider, - (error, accessToken, refreshToken, results) => { + async (error, accessToken, refreshToken, results) => { if (error || results.error) { logger.error('OAUTH_GET_ACCESS_TOKEN_ERROR', error, results, provider.id, code) - return callback(error || results.error) + throw new OAuthCallbackError(error || results.error) } if (provider.idToken) { @@ -71,34 +79,28 @@ export default async function oAuthCallback (req, provider, csrfToken, callback) // Unfortunately, we can't tell which, so we can't treat it as an // error, so instead we just returning nothing, which will cause the // user to be redirected back to the sign in page. - if (!results || !results.id_token) { - return callback() + if (!results?.id_token) { + throw new OAuthCallbackError() } // Support services that use OpenID ID Tokens to encode profile data - _decodeToken( - provider, - accessToken, - refreshToken, - results.id_token, - async (error, profileData) => { - const { profile, account, OAuthProfile } = await _getProfile(error, profileData, accessToken, refreshToken, provider, user) - callback(error, profile, account, OAuthProfile) - } - ) + const profileData = decodeIdToken(results.id_token) + + return _getProfile(error, profileData, accessToken, refreshToken, provider, user) } else { // Use custom get() method for oAuth2 flows client.get = _get + let result client.get( provider, accessToken, results, async (error, profileData) => { - const { profile, account, OAuthProfile } = await _getProfile(error, profileData, accessToken, refreshToken, provider) - callback(error, profile, account, OAuthProfile) + result = await _getProfile(error, profileData, accessToken, refreshToken, provider) } ) + return result } } ) @@ -114,15 +116,16 @@ export default async function oAuthCallback (req, provider, csrfToken, callback) logger.error('OAUTH_V1_GET_ACCESS_TOKEN_ERROR', error, results) } + let result client.get( provider.profileUrl, accessToken, refreshToken, async (error, profileData) => { - const { profile, account, OAuthProfile } = await _getProfile(error, profileData, accessToken, refreshToken, provider) - callback(error, profile, account, OAuthProfile) + result = await _getProfile(error, profileData, accessToken, refreshToken, provider) } ) + return result } ) } @@ -133,15 +136,16 @@ export default async function oAuthCallback (req, provider, csrfToken, callback) * Returns profile, raw profile and auth provider details */ async function _getProfile (error, profileData, accessToken, refreshToken, provider, userData) { - // @TODO Handle error if (error) { logger.error('OAUTH_GET_PROFILE_ERROR', error) + throw new OAuthCallbackError(error) } - let profile = {} try { // Convert profileData into an object if it's a string - if (typeof profileData === 'string' || profileData instanceof String) { profileData = JSON.parse(profileData) } + if (typeof profileData === 'string' || profileData instanceof String) { + profileData = JSON.parse(profileData) + } // If a user object is supplied (e.g. Apple provider) add it to the profile object if (userData != null) { @@ -150,7 +154,23 @@ async function _getProfile (error, profileData, accessToken, refreshToken, provi logger.debug('PROFILE_DATA', profileData) - profile = await provider.profile(profileData) + const profile = await provider.profile(profileData) + // Return profile, raw profile and auth provider details + return { + profile: { + ...profile, + email: profile.email?.toLowerCase() ?? null + }, + account: { + provider: provider.id, + type: provider.type, + id: profile.id, + refreshToken, + accessToken, + accessTokenExpires: null + }, + OAuthProfile: profileData + } } catch (exception) { // If we didn't get a response either there was a problem with the provider // response *or* the user cancelled the action with the provider. @@ -166,24 +186,6 @@ async function _getProfile (error, profileData, accessToken, refreshToken, provi OAuthProfile: profileData } } - - // Return profile, raw profile and auth provider details - return { - profile: { - name: profile.name, - email: profile.email ? profile.email.toLowerCase() : null, - image: profile.image - }, - account: { - provider: provider.id, - type: provider.type, - id: profile.id, - refreshToken, - accessToken, - accessTokenExpires: null - }, - OAuthProfile: profileData - } } // Ported from https://github.com/ciaranj/node-oauth/blob/a7f8a1e21c362eb4ed2039431fb9ac2ae749f26a/lib/oauth2.js @@ -280,9 +282,9 @@ function _get (provider, accessToken, results, callback) { this._request('GET', url, headers, null, accessToken, callback) } -function _decodeToken (provider, accessToken, refreshToken, idToken, callback) { - if (!idToken) { throw new Error('Missing JWT ID Token', provider, idToken) } - const decodedToken = jwtDecode(idToken) - const profileData = JSON.stringify(decodedToken) - callback(null, profileData, accessToken, refreshToken, provider) +function decodeIdToken (idToken) { + if (!idToken) { + throw new OAuthCallbackError('Missing JWT ID Token') + } + return jwtDecode(idToken, { json: true }) } diff --git a/src/server/routes/callback.js b/src/server/routes/callback.js index cc9a9950af..d59363d55c 100644 --- a/src/server/routes/callback.js +++ b/src/server/routes/callback.js @@ -30,103 +30,101 @@ export default async function callback (req, res) { if (type === 'oauth') { try { - oAuthCallback(req, provider, csrfToken, async (error, profile, account, OAuthProfile) => { - try { - if (error) { - logger.error('CALLBACK_OAUTH_ERROR', error) - return res.redirect(`${baseUrl()}/error?error=OAuthCallback`) - } + const { profile, account, OAuthProfile } = await oAuthCallback(req, provider, csrfToken) + try { + // Make it easier to debug when adding a new provider + logger.debug('OAUTH_CALLBACK_RESPONSE', { profile, account, OAuthProfile }) + + // If we don't have a profile object then either something went wrong + // or the user cancelled signin in. We don't know which, so we just + // direct the user to the signup page for now. We could do something + // else in future. + // + // Note: In oAuthCallback an error is logged with debug info, so it + // should at least be visible to developers what happened if it is an + // error with the provider. + if (!profile) { + return res.redirect(`${baseUrl()}/signin`) + } - // Make it easier to debug when adding a new provider - logger.debug('OAUTH_CALLBACK_RESPONSE', { profile, account, OAuthProfile }) - - // If we don't have a profile object then either something went wrong - // or the user cancelled signin in. We don't know which, so we just - // direct the user to the signup page for now. We could do something - // else in future. - // - // Note: In oAuthCallback an error is logged with debug info, so it - // should at least be visible to developers what happened if it is an - // error with the provider. - if (!profile) { - return res.redirect(`${baseUrl()}/signin`) + // Check if user is allowed to sign in + // Attempt to get Profile from OAuth provider details before invoking + // signIn callback - but if no user object is returned, that is fine + // (that just means it's a new user signing in for the first time). + let userOrProfile = profile + if (adapter) { + const { getUserByProviderAccountId } = await adapter.getAdapter(req.options) + const userFromProviderAccountId = await getUserByProviderAccountId(account.provider, account.id) + if (userFromProviderAccountId) { + userOrProfile = userFromProviderAccountId } + } - // Check if user is allowed to sign in - // Attempt to get Profile from OAuth provider details before invoking - // signIn callback - but if no user object is returned, that is fine - // (that just means it's a new user signing in for the first time). - let userOrProfile = profile - if (adapter) { - const { getUserByProviderAccountId } = await adapter.getAdapter(req.options) - const userFromProviderAccountId = await getUserByProviderAccountId(account.provider, account.id) - if (userFromProviderAccountId) { - userOrProfile = userFromProviderAccountId - } + try { + const signInCallbackResponse = await callbacks.signIn(userOrProfile, account, OAuthProfile) + if (signInCallbackResponse === false) { + return res.redirect(`${baseUrl()}/error?error=AccessDenied`) } - - try { - const signInCallbackResponse = await callbacks.signIn(userOrProfile, account, OAuthProfile) - if (signInCallbackResponse === false) { - return res.redirect(`${baseUrl()}/error?error=AccessDenied`) - } - } catch (error) { - if (error instanceof Error) { - return res.redirect(`${baseUrl()}/error?error=${encodeURIComponent(error)}`) - } else { - return res.redirect(error) - } + } catch (error) { + if (error instanceof Error) { + return res.redirect(`${baseUrl()}/error?error=${encodeURIComponent(error)}`) + } else { + return res.redirect(error) } + } - // Sign user in - const { user, session, isNewUser } = await callbackHandler(sessionToken, profile, account, req.options) + // Sign user in + const { user, session, isNewUser } = await callbackHandler(sessionToken, profile, account, req.options) - if (useJwtSession) { - const defaultJwtPayload = { - name: user.name, - email: user.email, - picture: user.image, - sub: user.id.toString() - } - const jwtPayload = await callbacks.jwt(defaultJwtPayload, user, account, OAuthProfile, isNewUser) + if (useJwtSession) { + const defaultJwtPayload = { + name: user.name, + email: user.email, + picture: user.image, + sub: user.id.toString() + } + const jwtPayload = await callbacks.jwt(defaultJwtPayload, user, account, OAuthProfile, isNewUser) - // Sign and encrypt token - const newEncodedJwt = await jwt.encode({ ...jwt, token: jwtPayload }) + // Sign and encrypt token + const newEncodedJwt = await jwt.encode({ ...jwt, token: jwtPayload }) - // Set cookie expiry date - const cookieExpires = new Date() - cookieExpires.setTime(cookieExpires.getTime() + (sessionMaxAge * 1000)) + // Set cookie expiry date + const cookieExpires = new Date() + cookieExpires.setTime(cookieExpires.getTime() + (sessionMaxAge * 1000)) - cookie.set(res, cookies.sessionToken.name, newEncodedJwt, { expires: cookieExpires.toISOString(), ...cookies.sessionToken.options }) - } else { - // Save Session Token in cookie - cookie.set(res, cookies.sessionToken.name, session.sessionToken, { expires: session.expires || null, ...cookies.sessionToken.options }) - } + cookie.set(res, cookies.sessionToken.name, newEncodedJwt, { expires: cookieExpires.toISOString(), ...cookies.sessionToken.options }) + } else { + // Save Session Token in cookie + cookie.set(res, cookies.sessionToken.name, session.sessionToken, { expires: session.expires || null, ...cookies.sessionToken.options }) + } - await dispatchEvent(events.signIn, { user, account, isNewUser }) + await dispatchEvent(events.signIn, { user, account, isNewUser }) - // Handle first logins on new accounts - // e.g. option to send users to a new account landing page on initial login - // Note that the callback URL is preserved, so the journey can still be resumed - if (isNewUser && pages.newUser) { - return res.redirect(`${pages.newUser}${pages.newUser.includes('?') ? '&' : '?'}callbackUrl=${encodeURIComponent(callbackUrl)}`) - } + // Handle first logins on new accounts + // e.g. option to send users to a new account landing page on initial login + // Note that the callback URL is preserved, so the journey can still be resumed + if (isNewUser && pages.newUser) { + return res.redirect(`${pages.newUser}${pages.newUser.includes('?') ? '&' : '?'}callbackUrl=${encodeURIComponent(callbackUrl)}`) + } - // Callback URL is already verified at this point, so safe to use if specified - return res.redirect(callbackUrl || baseUrl().origin) - } catch (error) { - if (error.name === 'AccountNotLinkedError') { - // If the email on the account is already linked, but nto with this OAuth account - return res.redirect(`${baseUrl()}/error?error=OAuthAccountNotLinked`) - } else if (error.name === 'CreateUserError') { - return res.redirect(`${baseUrl()}/error?error=OAuthCreateAccount`) - } else { - logger.error('OAUTH_CALLBACK_HANDLER_ERROR', error) - return res.redirect(`${baseUrl()}/error?error=Callback`) - } + // Callback URL is already verified at this point, so safe to use if specified + return res.redirect(callbackUrl || baseUrl().origin) + } catch (error) { + if (error.name === 'AccountNotLinkedError') { + // If the email on the account is already linked, but not with this OAuth account + return res.redirect(`${baseUrl()}/error?error=OAuthAccountNotLinked`) + } else if (error.name === 'CreateUserError') { + return res.redirect(`${baseUrl()}/error?error=OAuthCreateAccount`) + } else { + logger.error('OAUTH_CALLBACK_HANDLER_ERROR', error) + return res.redirect(`${baseUrl()}/error?error=Callback`) } - }) + } } catch (error) { + if (error.name === 'OAuthCallbackError') { + logger.error('CALLBACK_OAUTH_ERROR', error) + return res.redirect(`${baseUrl()}/error?error=OAuthCallback`) + } logger.error('OAUTH_CALLBACK_ERROR', error) return res.redirect(`${baseUrl()}/error?error=Callback`) } From 0c9f6f0d2c704e7333f00511f049e0cbcebe382d Mon Sep 17 00:00:00 2001 From: Balazs Orban Date: Fri, 1 Jan 2021 14:34:37 +0100 Subject: [PATCH 10/11] revert: "refactor: use native URL instead of string concats" Refs: 690c55b04089e4f3157424c816d43ee4cecb77a0 --- src/client/index.js | 18 ++++++----- src/lib/baseUrl.js | 30 ------------------ src/lib/parse-url.js | 27 ++++++++++++++++ src/server/index.js | 42 +++++++++++++------------ src/server/lib/callback-url-handler.js | 16 +++++----- src/server/lib/providers.js | 8 ++--- src/server/lib/signin/email.js | 5 ++- src/server/pages/error.js | 7 ++--- src/server/pages/index.js | 4 ++- src/server/pages/signout.js | 5 ++- src/server/pages/verify-request.js | 5 ++- src/server/routes/callback.js | 43 +++++++++++++------------- src/server/routes/signin.js | 14 ++++----- 13 files changed, 113 insertions(+), 111 deletions(-) delete mode 100644 src/lib/baseUrl.js create mode 100644 src/lib/parse-url.js diff --git a/src/client/index.js b/src/client/index.js index e1eec5f85e..6fcaec2e75 100644 --- a/src/client/index.js +++ b/src/client/index.js @@ -13,7 +13,7 @@ /* global fetch:false */ import { useState, useEffect, useContext, createContext, createElement } from 'react' import logger from '../lib/logger' -import baseUrl from '../lib/baseUrl' +import parseUrl from '../lib/parse-url' // This behaviour mirrors the default behaviour for getting the site name that // happens server side in server/index.js @@ -22,7 +22,8 @@ import baseUrl from '../lib/baseUrl' // 2. When invoked server side the value is picked up from an environment // variable and defaults to 'http://localhost:3000'. const __NEXTAUTH = { - baseUrl: baseUrl(), + baseUrl: parseUrl(process.env.NEXTAUTH_URL || process.env.VERCEL_URL).baseUrl, + basePath: parseUrl(process.env.NEXTAUTH_URL).basePath, keepAlive: 0, // 0 == disabled (don't send); 60 == send every 60 seconds clientMaxAge: 0, // 0 == disabled (only use cache); 60 == sync if last checked > 60 seconds ago // Properties starting with _ are used for tracking internal app state @@ -76,11 +77,13 @@ if (typeof window !== 'undefined') { // method is being left in as an alternative, that will be helpful if/when we // expose a vanilla JavaScript version that doesn't depend on React. const setOptions = ({ - baseUrl: _baseUrl, + baseUrl, + basePath, clientMaxAge, keepAlive } = {}) => { - if (baseUrl) { __NEXTAUTH.baseUrl = baseUrl(_baseUrl) } + if (baseUrl) { __NEXTAUTH.baseUrl = baseUrl } + if (basePath) { __NEXTAUTH.basePath = basePath } if (clientMaxAge) { __NEXTAUTH.clientMaxAge = clientMaxAge } if (keepAlive) { __NEXTAUTH.keepAlive = keepAlive @@ -303,10 +306,11 @@ const _apiBaseUrl = () => { if (!process.env.NEXTAUTH_URL) { logger.warn('NEXTAUTH_URL', 'NEXTAUTH_URL environment variable not set') } // Return absolute path when called server side - return __NEXTAUTH.baseUrl.href + return `${__NEXTAUTH.baseUrl}${__NEXTAUTH.basePath}` + } else { + // Return relative path when called client side + return __NEXTAUTH.basePath } - // Return relative path when called client side - return __NEXTAUTH.baseUrl.pathname } const _encodedForm = (formData) => { diff --git a/src/lib/baseUrl.js b/src/lib/baseUrl.js deleted file mode 100644 index cc09a662df..0000000000 --- a/src/lib/baseUrl.js +++ /dev/null @@ -1,30 +0,0 @@ -import logger from './logger' - -/** - * Simple universal (client/server) function to split host and path. - * It can also take a url (either URL or a string) and parses it correctly. - * @returns {URL} - */ -function baseUrl (url) { - let _url = url || process.env.NEXTAUTH_URL || process.env.VERCEL_URL - if (typeof _url !== 'string' && !(_url instanceof URL)) { - throw new Error('baseUrl must be either a valid URL object or a valid string URL') - } - const defaultUrl = 'http://localhost:3000/api/auth' - _url = _url || defaultUrl - try { - const parsedUrl = new URL(_url) - if (parsedUrl.pathname === '/') { - parsedUrl.pathname = '/api/auth' - } - parsedUrl.pathname = parsedUrl.pathname.replace(/\/$/, '') - parsedUrl.href = parsedUrl.href.replace(/\/$/, '') - - return parsedUrl - } catch (error) { - logger.error('INVALID_URL', _url, error) - return new URL(defaultUrl) - } -} - -export default baseUrl diff --git a/src/lib/parse-url.js b/src/lib/parse-url.js new file mode 100644 index 0000000000..d5dd72aba1 --- /dev/null +++ b/src/lib/parse-url.js @@ -0,0 +1,27 @@ +/** + * Simple universal (client/server) function to split host and path + * We use this rather than a library because we need to use the same logic both + * client and server side and we only need to parse out the host and path, while + * supporting a default value, so a simple split is sufficent. + * @param {string} url + */ +export default function parseUrl (url) { + // Default values + const defaultHost = 'http://localhost:3000' + const defaultPath = '/api/auth' + + if (!url) { url = `${defaultHost}${defaultPath}` } + + // Default to HTTPS if no protocol explictly specified + const protocol = url.match(/^http?:\/\//) ? 'http' : 'https' + + // Normalize URLs by stripping protocol and no trailing slash + url = url.replace(/^https?:\/\//, '').replace(/\/$/, '') + + // Simple split based on first / + const [_host, ..._path] = url.split('/') + const baseUrl = _host ? `${protocol}://${_host}` : defaultHost + const basePath = _path.length > 0 ? `/${_path.join('/')}` : defaultPath + + return { baseUrl, basePath } +} diff --git a/src/server/index.js b/src/server/index.js index 05dc23721a..ae56034050 100644 --- a/src/server/index.js +++ b/src/server/index.js @@ -1,6 +1,6 @@ import { createHash, randomBytes } from 'crypto' import jwt from '../lib/jwt' -import baseUrl from '../lib/baseUrl' +import parseUrl from '../lib/parse-url' import * as cookie from './lib/cookie' import callbackUrlHandler from './lib/callback-url-handler' import parseProviders from './lib/providers' @@ -58,6 +58,9 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { csrfToken: csrfTokenFromPost } = body + // @todo refactor all existing references to baseUrl and basePath + const { basePath, baseUrl } = parseUrl(process.env.NEXTAUTH_URL || process.env.VERCEL_URL) + // Parse database / adapter let adapter if (userSuppliedOptions.adapter) { @@ -73,7 +76,7 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { // based on options passed here. A options contains unique data, such as // OAuth provider secrets and database credentials it should be sufficent. const secret = userSuppliedOptions.secret || createHash('sha256').update(JSON.stringify({ - baseUrl: baseUrl(), ...userSuppliedOptions + baseUrl, basePath, ...userSuppliedOptions })).digest('hex') // Use secure cookies if the site uses HTTPS @@ -82,7 +85,7 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { // prefix, but enable them by default if the site URL is HTTPS; but not for // non-HTTPS URLs like http://localhost which are used in development). // For more on prefixes see https://googlechrome.github.io/samples/cookie-prefixes/ - const useSecureCookies = userSuppliedOptions.useSecureCookies || baseUrl().protocol.startsWith('https://') + const useSecureCookies = userSuppliedOptions.useSecureCookies || baseUrl.startsWith('https://') const cookiePrefix = useSecureCookies ? '__Secure-' : '' // @TODO Review cookie settings (names, options) @@ -197,12 +200,14 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { // These computed settings can values in userSuppliedOptions but override them // and are request-specific. adapter, + baseUrl, + basePath, action, provider, cookies, secret, csrfToken, - providers: parseProviders(userSuppliedOptions.providers), + providers: parseProviders({ providers: userSuppliedOptions.providers, basePath, baseUrl }), session: sessionOptions, jwt: jwtOptions, events: eventsOptions, @@ -211,7 +216,9 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { req.options = options // If debug enabled, set ENV VAR so that logger logs debug messages - if (options.debug === true) { process.env._NEXTAUTH_DEBUG = true } + if (options.debug) { + process.env._NEXTAUTH_DEBUG = true + } // Get / Set callback URL based on query param / cookie + validation const callbackUrl = await callbackUrlHandler(req, res) @@ -237,7 +244,9 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { renderPage(req, res, 'signin', { providers: Object.values(options.providers), callbackUrl, csrfToken }) break case 'signout': - if (options.pages.signOut) { return res.redirect(`${options.pages.signOut}${options.pages.signOut.includes('?') ? '&' : '?'}error=${error}`) } + if (options.pages.signOut) { + return res.redirect(`${options.pages.signOut}${options.pages.signOut.includes('?') ? '&' : '?'}error=${error}`) + } renderPage(req, res, 'signout', { csrfToken, callbackUrl }) break @@ -245,8 +254,7 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { if (provider && options.providers[provider]) { callback(req, res) } else { - res.status(400).end(`Error: HTTP GET is not supported for ${url}`) - return res.end() + return res.status(400).end(`Error: HTTP GET is not supported for ${url}`).end() } break case 'verify-request': @@ -260,15 +268,14 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { renderPage(req, res, 'error', { error }) break default: - res.status(404).end() - return res.end() + return res.status(404).end() } } else if (req.method === 'POST') { switch (action) { case 'signin': // Verified CSRF Token required for all sign in routes if (!csrfTokenVerified) { - return res.redirect(`${baseUrl()}/signin?csrf=true`) + return res.redirect(`${baseUrl}${basePath}/signin?csrf=true`) } if (provider && options.providers[provider]) { @@ -278,7 +285,7 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { case 'signout': // Verified CSRF Token required for signout if (!csrfTokenVerified) { - return res.redirect(`${baseUrl()}/signout?csrf=true`) + return res.redirect(`${baseUrl}${basePath}/signout?csrf=true`) } signout(req, res) @@ -287,22 +294,19 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { if (provider && options.providers[provider]) { // Verified CSRF Token required for credentials providers only if (options.providers[provider].type === 'credentials' && !csrfTokenVerified) { - return res.redirect(`${baseUrl()}/signin?csrf=true`) + return res.redirect(`${baseUrl}${basePath}/signin?csrf=true`) } callback(req, res) } else { - res.status(400).end(`Error: HTTP POST is not supported for ${url}`) - return res.end() + return res.status(400).end(`Error: HTTP POST is not supported for ${url}`).end() } break default: - res.status(400).end(`Error: HTTP POST is not supported for ${url}`) - return res.end() + return res.status(400).end(`Error: HTTP POST is not supported for ${url}`).end() } } else { - res.status(400).end(`Error: HTTP ${req.method} is not supported for ${url}`) - return res.end() + return res.status(400).end(`Error: HTTP ${req.method} is not supported for ${url}`).end() } }) } diff --git a/src/server/lib/callback-url-handler.js b/src/server/lib/callback-url-handler.js index 8bbaac2337..73fee30677 100644 --- a/src/server/lib/callback-url-handler.js +++ b/src/server/lib/callback-url-handler.js @@ -1,28 +1,28 @@ import * as cookie from '../lib/cookie' -import baseUrl from '../../lib/baseUrl' export default async function callbackUrlHandler (req, res) { const { query } = req const { body } = req - const { cookies, defaultCallbackUrl, callbacks } = req.options - const homepage = baseUrl().origin + const { cookies, baseUrl, defaultCallbackUrl, callbacks } = req.options // Handle preserving and validating callback URLs // If no defaultCallbackUrl option specified, default to the homepage for the site - let callbackUrl = defaultCallbackUrl || homepage + let callbackUrl = defaultCallbackUrl || baseUrl // Try reading callbackUrlParamValue from request body (form submission) then from query param (get request) const callbackUrlParamValue = body.callbackUrl || query.callbackUrl || null const callbackUrlCookieValue = req.cookies[cookies.callbackUrl.name] || null if (callbackUrlParamValue) { // If callbackUrl form field or query parameter is passed try to use it if allowed - callbackUrl = await callbacks.redirect(callbackUrlParamValue, homepage) + callbackUrl = await callbacks.redirect(callbackUrlParamValue, baseUrl) } else if (callbackUrlCookieValue) { // If no callbackUrl specified, try using the value from the cookie if allowed - callbackUrl = await callbacks.redirect(callbackUrlCookieValue, homepage) + callbackUrl = await callbacks.redirect(callbackUrlCookieValue, baseUrl) } // Save callback URL in a cookie so that can be used for subsequent requests in signin/signout/callback flow - if (callbackUrl && (callbackUrl !== callbackUrlCookieValue)) { cookie.set(res, cookies.callbackUrl.name, callbackUrl, cookies.callbackUrl.options) } + if (callbackUrl && (callbackUrl !== callbackUrlCookieValue)) { + cookie.set(res, cookies.callbackUrl.name, callbackUrl, cookies.callbackUrl.options) + } - return Promise.resolve(callbackUrl) + return callbackUrl } diff --git a/src/server/lib/providers.js b/src/server/lib/providers.js index b36180a796..99a4bc3b09 100644 --- a/src/server/lib/providers.js +++ b/src/server/lib/providers.js @@ -1,12 +1,10 @@ -import baseUrl from '../../lib/baseUrl' - -export default function parseProviders (providers) { +export default function parseProviders ({ providers, baseUrl, basePath }) { return providers.reduce((acc, provider) => { const providerId = provider.id acc[providerId] = { ...provider, - signinUrl: `${baseUrl()}/signin/${providerId}`, - callbackUrl: `${baseUrl()}/callback/${providerId}` + signinUrl: `${baseUrl}${basePath}/signin/${providerId}`, + callbackUrl: `${baseUrl}${basePath}/callback/${providerId}` } return acc }, {}) diff --git a/src/server/lib/signin/email.js b/src/server/lib/signin/email.js index a8e7ac0eed..3b5912ec55 100644 --- a/src/server/lib/signin/email.js +++ b/src/server/lib/signin/email.js @@ -1,9 +1,8 @@ import { randomBytes } from 'crypto' -import baseUrl from '../../../lib/baseUrl' export default async function email (email, provider, options) { try { - const { adapter } = options + const { baseUrl, basePath, adapter } = options const { createVerificationRequest } = await adapter.getAdapter(options) @@ -14,7 +13,7 @@ export default async function email (email, provider, options) { const token = randomBytes(32).toString('hex') // Send email with link containing token (the unhashed version) - const url = `${baseUrl()}/callback/${encodeURIComponent(provider.id)}?email=${encodeURIComponent(email)}&token=${encodeURIComponent(token)}` + const url = `${baseUrl}${basePath}/callback/${encodeURIComponent(provider.id)}?email=${encodeURIComponent(email)}&token=${encodeURIComponent(token)}` // @TODO Create invite (send secret so can be hashed) await createVerificationRequest(email, url, token, secret, provider, options) diff --git a/src/server/pages/error.js b/src/server/pages/error.js index dac6b5dc0c..317b906ac6 100644 --- a/src/server/pages/error.js +++ b/src/server/pages/error.js @@ -1,13 +1,12 @@ import { h } from 'preact' // eslint-disable-line no-unused-vars import render from 'preact-render-to-string' -import baseUrl from '../../lib/baseUrl' -export default function error ({ error, res }) { - const signinPageUrl = `${baseUrl()}/signin` +export default function error ({ baseUrl, basePath, error, res }) { + const signinPageUrl = `${baseUrl}${basePath}/signin` let statusCode = 200 let heading =

Error

- let message =

{baseUrl().host}

+ let message =

{baseUrl.replace(/^https?:\/\//, '')}

switch (error) { case 'Signin': diff --git a/src/server/pages/index.js b/src/server/pages/index.js index 2dd31fe88c..49cbea70c0 100644 --- a/src/server/pages/index.js +++ b/src/server/pages/index.js @@ -4,7 +4,9 @@ import verifyRequest from './verify-request' import error from './error' import css from '../../css' -export default function renderPage (req, res, page, props = {}) { +export default function renderPage ({ req, res, page, props = {} }) { + props.baseUrl = req.options.baseUrl + props.basePath = req.options.basePath let html = '' switch (page) { case 'signin': diff --git a/src/server/pages/signout.js b/src/server/pages/signout.js index 4206f1559d..12d8cf7c00 100644 --- a/src/server/pages/signout.js +++ b/src/server/pages/signout.js @@ -1,12 +1,11 @@ import { h } from 'preact' // eslint-disable-line no-unused-vars import render from 'preact-render-to-string' -import baseUrl from '../../lib/baseUrl' -export default function signout ({ csrfToken }) { +export default function signout ({ baseUrl, basePath, csrfToken }) { return render(

Are you sure you want to sign out?

- + diff --git a/src/server/pages/verify-request.js b/src/server/pages/verify-request.js index 259c152c29..cd336742ff 100644 --- a/src/server/pages/verify-request.js +++ b/src/server/pages/verify-request.js @@ -1,13 +1,12 @@ import { h } from 'preact' // eslint-disable-line no-unused-vars import render from 'preact-render-to-string' -import baseUrl from '../../lib/baseUrl' -export default function verifyRequest () { +export default function verifyRequest ({ baseUrl }) { return render(

Check your email

A sign in link has been sent to your email address.

-

{baseUrl().host}

+

{baseUrl.replace(/^https?:\/\//, '')}

) } diff --git a/src/server/routes/callback.js b/src/server/routes/callback.js index d59363d55c..873e4f901d 100644 --- a/src/server/routes/callback.js +++ b/src/server/routes/callback.js @@ -3,7 +3,6 @@ import callbackHandler from '../lib/callback-handler' import * as cookie from '../lib/cookie' import logger from '../../lib/logger' import dispatchEvent from '../lib/dispatch-event' -import baseUrl from '../../lib/baseUrl' /** Handle callbacks from login services */ export default async function callback (req, res) { @@ -11,6 +10,8 @@ export default async function callback (req, res) { provider: providerName, providers, adapter, + baseUrl, + basePath, secret, cookies, callbackUrl, @@ -44,7 +45,7 @@ export default async function callback (req, res) { // should at least be visible to developers what happened if it is an // error with the provider. if (!profile) { - return res.redirect(`${baseUrl()}/signin`) + return res.redirect(`${baseUrl}${basePath}/signin`) } // Check if user is allowed to sign in @@ -63,11 +64,11 @@ export default async function callback (req, res) { try { const signInCallbackResponse = await callbacks.signIn(userOrProfile, account, OAuthProfile) if (signInCallbackResponse === false) { - return res.redirect(`${baseUrl()}/error?error=AccessDenied`) + return res.redirect(`${baseUrl}${basePath}/error?error=AccessDenied`) } } catch (error) { if (error instanceof Error) { - return res.redirect(`${baseUrl()}/error?error=${encodeURIComponent(error)}`) + return res.redirect(`${baseUrl}${basePath}/error?error=${encodeURIComponent(error)}`) } else { return res.redirect(error) } @@ -112,27 +113,27 @@ export default async function callback (req, res) { } catch (error) { if (error.name === 'AccountNotLinkedError') { // If the email on the account is already linked, but not with this OAuth account - return res.redirect(`${baseUrl()}/error?error=OAuthAccountNotLinked`) + return res.redirect(`${baseUrl}${basePath}/error?error=OAuthAccountNotLinked`) } else if (error.name === 'CreateUserError') { - return res.redirect(`${baseUrl()}/error?error=OAuthCreateAccount`) + return res.redirect(`${baseUrl}${basePath}/error?error=OAuthCreateAccount`) } else { logger.error('OAUTH_CALLBACK_HANDLER_ERROR', error) - return res.redirect(`${baseUrl()}/error?error=Callback`) + return res.redirect(`${baseUrl}${basePath}/error?error=Callback`) } } } catch (error) { if (error.name === 'OAuthCallbackError') { logger.error('CALLBACK_OAUTH_ERROR', error) - return res.redirect(`${baseUrl()}/error?error=OAuthCallback`) + return res.redirect(`${baseUrl}${basePath}/error?error=OAuthCallback`) } logger.error('OAUTH_CALLBACK_ERROR', error) - return res.redirect(`${baseUrl()}/error?error=Callback`) + return res.redirect(`${baseUrl}${basePath}/error?error=Callback`) } } else if (type === 'email') { try { if (!adapter) { logger.error('EMAIL_REQUIRES_ADAPTER_ERROR') - return res.redirect(`${baseUrl()}/error?error=Configuration`) + return res.redirect(`${baseUrl}${basePath}/error?error=Configuration`) } const { getVerificationRequest, deleteVerificationRequest, getUserByEmail } = await adapter.getAdapter(req.options) @@ -142,7 +143,7 @@ export default async function callback (req, res) { // Verify email and verification token exist in database const invite = await getVerificationRequest(email, verificationToken, secret, provider) if (!invite) { - return res.redirect(`${baseUrl()}/error?error=Verification`) + return res.redirect(`${baseUrl}${basePath}/error?error=Verification`) } // If verification token is valid, delete verification request token from @@ -157,11 +158,11 @@ export default async function callback (req, res) { try { const signInCallbackResponse = await callbacks.signIn(profile, account, { email }) if (signInCallbackResponse === false) { - return res.redirect(`${baseUrl()}/error?error=AccessDenied`) + return res.redirect(`${baseUrl}${basePath}/error?error=AccessDenied`) } } catch (error) { if (error instanceof Error) { - return res.redirect(`${baseUrl()}/error?error=${encodeURIComponent(error)}`) + return res.redirect(`${baseUrl}${basePath}/error?error=${encodeURIComponent(error)}`) } else { return res.redirect(error) } @@ -205,21 +206,21 @@ export default async function callback (req, res) { return res.redirect(callbackUrl || baseUrl().origin) } catch (error) { if (error.name === 'CreateUserError') { - return res.redirect(`${baseUrl()}/error?error=EmailCreateAccount`) + return res.redirect(`${baseUrl}${basePath}/error?error=EmailCreateAccount`) } else { logger.error('CALLBACK_EMAIL_ERROR', error) - return res.redirect(`${baseUrl()}/error?error=Callback`) + return res.redirect(`${baseUrl}${basePath}/error?error=Callback`) } } } else if (type === 'credentials' && req.method === 'POST') { if (!useJwtSession) { logger.error('CALLBACK_CREDENTIALS_JWT_ERROR', 'Signin in with credentials is only supported if JSON Web Tokens are enabled') - return res.redirect(`${baseUrl()}/error?error=Configuration`) + return res.redirect(`${baseUrl}${basePath}/error?error=Configuration`) } if (!provider.authorize) { logger.error('CALLBACK_CREDENTIALS_HANDLER_ERROR', 'Must define an authorize() handler to use credentials authentication provider') - return res.redirect(`${baseUrl()}/error?error=Configuration`) + return res.redirect(`${baseUrl}${basePath}/error?error=Configuration`) } const credentials = req.body @@ -228,11 +229,11 @@ export default async function callback (req, res) { try { userObjectReturnedFromAuthorizeHandler = await provider.authorize(credentials) if (!userObjectReturnedFromAuthorizeHandler) { - return res.redirect(`${baseUrl()}/error?error=CredentialsSignin&provider=${encodeURIComponent(provider.id)}`) + return res.redirect(`${baseUrl}${basePath}/error?error=CredentialsSignin&provider=${encodeURIComponent(provider.id)}`) } } catch (error) { if (error instanceof Error) { - return res.redirect(`${baseUrl()}/error?error=${encodeURIComponent(error)}`) + return res.redirect(`${baseUrl}${basePath}/error?error=${encodeURIComponent(error)}`) } else { return res.redirect(error) } @@ -244,11 +245,11 @@ export default async function callback (req, res) { try { const signInCallbackResponse = await callbacks.signIn(user, account, credentials) if (signInCallbackResponse === false) { - return res.redirect(`${baseUrl()}/error?error=AccessDenied`) + return res.redirect(`${baseUrl}${basePath}/error?error=AccessDenied`) } } catch (error) { if (error instanceof Error) { - return res.redirect(`${baseUrl()}/error?error=${encodeURIComponent(error)}`) + return res.redirect(`${baseUrl}${basePath}/error?error=${encodeURIComponent(error)}`) } else { return res.redirect(error) } diff --git a/src/server/routes/signin.js b/src/server/routes/signin.js index 24094d9eab..99d58c3879 100644 --- a/src/server/routes/signin.js +++ b/src/server/routes/signin.js @@ -1,13 +1,14 @@ import oAuthSignin from '../lib/signin/oauth' import emailSignin from '../lib/signin/email' import logger from '../../lib/logger' -import baseUrl from '../../lib/baseUrl' /** Handle requests to /api/auth/signin */ export default async function signin (req, res) { const { provider: providerName, providers, + baseUrl, + basePath, adapter, callbacks, csrfToken @@ -27,7 +28,7 @@ export default async function signin (req, res) { oAuthSignin(provider, csrfToken, (error, oAuthSigninUrl) => { if (error) { logger.error('SIGNIN_OAUTH_ERROR', error) - return res.redirect(`${baseUrl()}/error?error=OAuthSignin`) + return res.redirect(`${baseUrl}${basePath}/error?error=OAuthSignin`) } return res.redirect(oAuthSigninUrl) @@ -35,7 +36,7 @@ export default async function signin (req, res) { } else if (type === 'email' && req.method === 'POST') { if (!adapter) { logger.error('EMAIL_REQUIRES_ADAPTER_ERROR') - return res.redirect(`${baseUrl()}/error?error=Configuration`) + return res.redirect(`${baseUrl}${basePath}/error?error=Configuration`) } const { getUserByEmail } = await adapter.getAdapter(req.options) @@ -54,14 +55,13 @@ export default async function signin (req, res) { try { const signinCallbackResponse = await callbacks.signIn(profile, account, { email, verificationRequest: true }) if (signinCallbackResponse === false) { - return res.redirect(`${baseUrl()}/error?error=AccessDenied`) + return res.redirect(`${baseUrl}${basePath}/error?error=AccessDenied`) } } catch (error) { if (error instanceof Error) { - return res.redirect(`${baseUrl()}/error?error=${encodeURIComponent(error)}`) - } else { - return res.redirect(error) + return res.redirect(`${baseUrl}${basePath}/error?error=${encodeURIComponent(error)}`) } + return res.redirect(error) } try { From 7b4d8c2d990196939d4f5eaa51569345b7032541 Mon Sep 17 00:00:00 2001 From: Balazs Orban Date: Fri, 1 Jan 2021 14:43:00 +0100 Subject: [PATCH 11/11] chore: misc changes --- src/server/index.js | 5 ++--- src/server/lib/providers.js | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/server/index.js b/src/server/index.js index ae56034050..cca4c7df40 100644 --- a/src/server/index.js +++ b/src/server/index.js @@ -42,8 +42,7 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { const error = 'Cannot find [...nextauth].js in pages/api/auth. Make sure the filename is written correctly.' logger.error('MISSING_NEXTAUTH_API_ROUTE_ERROR', error) - res.status(500).end(`Error: ${error}`) - return res.end() + return res.status(500).end(`Error: ${error}`).end() } const { url, query, body } = req @@ -207,7 +206,7 @@ async function NextAuthHandler (req, res, userSuppliedOptions) { cookies, secret, csrfToken, - providers: parseProviders({ providers: userSuppliedOptions.providers, basePath, baseUrl }), + providers: parseProviders(userSuppliedOptions.providers, basePath, baseUrl), session: sessionOptions, jwt: jwtOptions, events: eventsOptions, diff --git a/src/server/lib/providers.js b/src/server/lib/providers.js index 99a4bc3b09..f5f988a01b 100644 --- a/src/server/lib/providers.js +++ b/src/server/lib/providers.js @@ -1,4 +1,4 @@ -export default function parseProviders ({ providers, baseUrl, basePath }) { +export default function parseProviders (providers, baseUrl, basePath) { return providers.reduce((acc, provider) => { const providerId = provider.id acc[providerId] = {