diff --git a/api/src/app.ts b/api/src/app.ts index 2dc8f06e16b..feb6dcd5c73 100644 --- a/api/src/app.ts +++ b/api/src/app.ts @@ -186,10 +186,10 @@ export const build = async ( void fastify.register(auth); void fastify.register(notFound); void fastify.register(prismaPlugin); + void fastify.register(bouncer); // Routes requiring authentication: void fastify.register(async function (fastify, _opts) { - await fastify.register(bouncer); fastify.addHook('onRequest', fastify.authorize); // CSRF protection enabled: await fastify.register(async function (fastify, _opts) { @@ -222,14 +222,19 @@ export const build = async ( await fastify.register(settingRedirectRoutes); }); }); - // Routes not requiring authentication: - void fastify.register(mobileAuth0Routes); - // TODO: consolidate with LOCAL_MOCK_AUTH - if (FCC_ENABLE_DEV_LOGIN_MODE) { - void fastify.register(devAuthRoutes); - } else { - void fastify.register(authRoutes); - } + // Routes for signed out users: + void fastify.register(async function (fastify) { + fastify.addHook('onRequest', fastify.authorize); + // TODO(Post-MVP): add the redirectIfSignedIn hook here, rather than in the + // mobileAuth0Routes and authRoutes plugins. + await fastify.register(mobileAuth0Routes); + // TODO: consolidate with LOCAL_MOCK_AUTH + if (FCC_ENABLE_DEV_LOGIN_MODE) { + await fastify.register(devAuthRoutes); + } else { + await fastify.register(authRoutes); + } + }); void fastify.register(chargeStripeRoute); void fastify.register(signoutRoute); void fastify.register(emailSubscribtionRoutes); diff --git a/api/src/plugins/auth0.test.ts b/api/src/plugins/auth0.test.ts index d083990f313..a58bc4c0534 100644 --- a/api/src/plugins/auth0.test.ts +++ b/api/src/plugins/auth0.test.ts @@ -8,6 +8,7 @@ import cookies, { sign, unsign } from './cookies'; import { auth0Client } from './auth0'; import redirectWithMessage, { formatMessage } from './redirect-with-message'; import auth from './auth'; +import bouncer from './bouncer'; // eslint-disable-next-line @typescript-eslint/no-unsafe-return jest.mock('../utils/env', () => ({ @@ -24,6 +25,7 @@ describe('auth0 plugin', () => { await fastify.register(cookies); await fastify.register(redirectWithMessage); await fastify.register(auth); + await fastify.register(bouncer); await fastify.register(auth0Client); await fastify.register(prismaPlugin); }); diff --git a/api/src/plugins/auth0.ts b/api/src/plugins/auth0.ts index 916ade7b8a9..82e1b5e5b97 100644 --- a/api/src/plugins/auth0.ts +++ b/api/src/plugins/auth0.ts @@ -53,21 +53,28 @@ export const auth0Client: FastifyPluginCallbackTypebox = fp( } }); - fastify.get('/signin', async function (request, reply) { - const returnTo = request.headers.referer ?? `${HOME_LOCATION}/learn`; - void reply.setCookie('login-returnto', returnTo, { - domain: COOKIE_DOMAIN, - httpOnly: true, - secure: true, - signed: true, - sameSite: 'lax' - }); + void fastify.register(function (fastify, _options, done) { + // TODO(Post-MVP): move this into the app, so that we add this hook once for + // all auth routes. + fastify.addHook('onRequest', fastify.redirectIfSignedIn); - const redirectUrl = await this.auth0OAuth.generateAuthorizationUri( - request, - reply - ); - void reply.redirect(redirectUrl); + fastify.get('/signin', async function (request, reply) { + const returnTo = request.headers.referer ?? `${HOME_LOCATION}/learn`; + void reply.setCookie('login-returnto', returnTo, { + domain: COOKIE_DOMAIN, + httpOnly: true, + secure: true, + signed: true, + sameSite: 'lax' + }); + + const redirectUrl = await this.auth0OAuth.generateAuthorizationUri( + request, + reply + ); + void reply.redirect(redirectUrl); + }); + done(); }); // TODO: use a schema to validate the query params. @@ -151,5 +158,7 @@ export const auth0Client: FastifyPluginCallbackTypebox = fp( done(); }, - { dependencies: ['redirect-with-message'] } + // TODO(Post-MVP): remove bouncer dependency when moving redirectIfSignedIn + // out of this plugin. + { dependencies: ['redirect-with-message', 'bouncer'] } ); diff --git a/api/src/plugins/bouncer.test.ts b/api/src/plugins/bouncer.test.ts index 8618d3a32f1..6f9d6a363b7 100644 --- a/api/src/plugins/bouncer.test.ts +++ b/api/src/plugins/bouncer.test.ts @@ -40,7 +40,7 @@ describe('bouncer', () => { fastify.addHook('onRequest', fastify.send401IfNoUser); }); - it('should return 401 if no user is present', async () => { + it('should return 401 if NO user is present', async () => { const message = { type: 'danger', content: 'Something undesirable occurred' @@ -83,7 +83,10 @@ describe('bouncer', () => { }); const redirectLocation = `${HOME_LOCATION}?${formatMessage({ type: 'info', content: 'Only authenticated users can access this route. Please sign in and try again.' })}`; - it('should redirect to HOME_LOCATION if no user is present', async () => { + // TODO(Post-MVP): make the redirects consistent between redirectIfNoUser + // and redirectIfSignedIn. Either both should redirect to the referer or + // both should redirect to HOME_LOCATION. + it('should redirect to HOME_LOCATION if NO user is present', async () => { const message = { type: 'danger', content: 'At the moment, content is ignored' @@ -117,36 +120,35 @@ describe('bouncer', () => { }); }); - describe('fallback hook', () => { - it('should reject unauthed requests when no other reject hooks are added', async () => { - const message = { - type: 'danger', - content: 'Something undesirable occurred' - }; - authorizeSpy.mockImplementationOnce((req, _reply, done) => { - req.accessDeniedMessage = message; - done(); - }); - const res = await fastify.inject({ - method: 'GET', - url: '/' - }); - - expect(res.json()).toStrictEqual({ - type: message.type, - message: message.content - }); + describe('redirectIfSignedIn', () => { + beforeEach(() => { + fastify.addHook('onRequest', fastify.redirectIfSignedIn); }); - it('should not be called if another reject hook is added', async () => { - const redirectLocation = `${HOME_LOCATION}?${formatMessage({ type: 'info', content: 'Only authenticated users can access this route. Please sign in and try again.' })}`; + it('should redirect to the referer if a user is present', async () => { + authorizeSpy.mockImplementationOnce((req, _reply, done) => { + req.user = { id: '123' }; + done(); + }); + const res = await fastify.inject({ + method: 'GET', + url: '/', + headers: { + referer: 'https://www.freecodecamp.org/some/other/path' + } + }); + + expect(res.headers.location).toBe( + 'https://www.freecodecamp.org/some/other/path' + ); + expect(res.statusCode).toEqual(302); + }); + + it('should not alter the response if NO user is present', async () => { const message = { type: 'danger', - content: 'Something undesirable occurred' + content: 'At the moment, content is ignored' }; - // using redirectIfNoUser as the reject hook since then it's obvious that - // the fallback hook is not called. - fastify.addHook('onRequest', fastify.redirectIfNoUser); authorizeSpy.mockImplementationOnce((req, _reply, done) => { req.accessDeniedMessage = message; done(); @@ -156,8 +158,8 @@ describe('bouncer', () => { url: '/' }); - expect(res.headers.location).toBe(redirectLocation); - expect(res.statusCode).toEqual(302); + expect(res.json()).toEqual({ foo: 'bar' }); + expect(res.statusCode).toEqual(200); }); }); }); diff --git a/api/src/plugins/bouncer.ts b/api/src/plugins/bouncer.ts index 61d6f71121d..e7181e28ede 100644 --- a/api/src/plugins/bouncer.ts +++ b/api/src/plugins/bouncer.ts @@ -10,6 +10,7 @@ declare module 'fastify' { interface FastifyInstance { send401IfNoUser: (req: FastifyRequest, reply: FastifyReply) => void; redirectIfNoUser: (req: FastifyRequest, reply: FastifyReply) => void; + redirectIfSignedIn: (req: FastifyRequest, reply: FastifyReply) => void; } } @@ -40,9 +41,20 @@ const plugin: FastifyPluginCallback = (fastify, _options, done) => { } ); - fastify.addHook('preParsing', fastify.send401IfNoUser); + fastify.decorate( + 'redirectIfSignedIn', + async function (req: FastifyRequest, reply: FastifyReply) { + if (req.user) { + const { returnTo } = getRedirectParams(req); + await reply.redirect(returnTo); + } + } + ); done(); }; -export default fp(plugin, { dependencies: ['auth', 'redirect-with-message'] }); +export default fp(plugin, { + dependencies: ['auth', 'redirect-with-message'], + name: 'bouncer' +}); diff --git a/api/src/routes/auth.ts b/api/src/routes/auth.ts index 9da7782cd5f..2fcb4a53153 100644 --- a/api/src/routes/auth.ts +++ b/api/src/routes/auth.ts @@ -56,6 +56,10 @@ export const mobileAuth0Routes: FastifyPluginCallback = ( }) ); + // TODO(Post-MVP): move this into the app, so that we add this hook once for + // all auth routes. + fastify.addHook('onRequest', fastify.redirectIfSignedIn); + fastify.get('/mobile-login', async req => { const email = await getEmailFromAuth0(req);