From c27e6d0ce8db5f0fc9873bd3a41574e5bbd34f2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20St=C3=B8vring?= Date: Tue, 7 Nov 2023 09:08:18 +0100 Subject: [PATCH 1/2] Introduces new ILogOutHandler concept # Conflicts: # __test__/common/authHandler/logoutHandler.test.ts # src/app/api/auth/[auth0]/route.ts # src/common/authHandler/logout.ts # src/composition.ts --- __test__/auth/CompositeLogOutHandler.test.ts | 24 +++++++++++ .../auth/ErrorIgnoringLogOutHandler.test.ts | 11 +++++ .../auth/UserDataCleanUpLogOutHandler.test.ts | 16 ++++++++ .../common/authHandler/logoutHandler.test.ts | 41 ------------------- src/app/api/auth/[auth0]/route.ts | 18 ++------ src/common/authHandler/logout.ts | 12 ------ src/composition.ts | 25 +++++++---- .../domain/logOut/CompositeLogOutHandler.ts | 14 +++++++ .../logOut/ErrorIgnoringLogOutHandler.ts | 17 ++++++++ .../auth/domain/logOut/ILogOutHandler.ts | 3 ++ .../logOut/UserDataCleanUpLogOutHandler.ts | 24 +++++++++++ 11 files changed, 129 insertions(+), 76 deletions(-) create mode 100644 __test__/auth/CompositeLogOutHandler.test.ts create mode 100644 __test__/auth/ErrorIgnoringLogOutHandler.test.ts create mode 100644 __test__/auth/UserDataCleanUpLogOutHandler.test.ts delete mode 100644 __test__/common/authHandler/logoutHandler.test.ts delete mode 100644 src/common/authHandler/logout.ts create mode 100644 src/features/auth/domain/logOut/CompositeLogOutHandler.ts create mode 100644 src/features/auth/domain/logOut/ErrorIgnoringLogOutHandler.ts create mode 100644 src/features/auth/domain/logOut/ILogOutHandler.ts create mode 100644 src/features/auth/domain/logOut/UserDataCleanUpLogOutHandler.ts diff --git a/__test__/auth/CompositeLogOutHandler.test.ts b/__test__/auth/CompositeLogOutHandler.test.ts new file mode 100644 index 00000000..9618723c --- /dev/null +++ b/__test__/auth/CompositeLogOutHandler.test.ts @@ -0,0 +1,24 @@ +import CompositeLogOutHandler from "../../src/features/auth/domain/logOut/CompositeLogOutHandler" + +test("It invokes all log out handlers", async () => { + let didCallLogOutHandler1 = false + let didCallLogOutHandler2 = false + let didCallLogOutHandler3 = false + const sut = new CompositeLogOutHandler([{ + async handleLogOut() { + didCallLogOutHandler1 = true + } + }, { + async handleLogOut() { + didCallLogOutHandler2 = true + } + }, { + async handleLogOut() { + didCallLogOutHandler3 = true + } + }]) + await sut.handleLogOut() + expect(didCallLogOutHandler1).toBeTruthy() + expect(didCallLogOutHandler2).toBeTruthy() + expect(didCallLogOutHandler3).toBeTruthy() +}) diff --git a/__test__/auth/ErrorIgnoringLogOutHandler.test.ts b/__test__/auth/ErrorIgnoringLogOutHandler.test.ts new file mode 100644 index 00000000..5feae77c --- /dev/null +++ b/__test__/auth/ErrorIgnoringLogOutHandler.test.ts @@ -0,0 +1,11 @@ +import ErrorIgnoringLogOutHandler from "../../src/features/auth/domain/logOut/ErrorIgnoringLogOutHandler" + +test("It ignores errors", async () => { + const sut = new ErrorIgnoringLogOutHandler({ + async handleLogOut() { + throw new Error("Mock") + } + }) + // Test will fail if the following throws. + await sut.handleLogOut() +}) diff --git a/__test__/auth/UserDataCleanUpLogOutHandler.test.ts b/__test__/auth/UserDataCleanUpLogOutHandler.test.ts new file mode 100644 index 00000000..c8db0461 --- /dev/null +++ b/__test__/auth/UserDataCleanUpLogOutHandler.test.ts @@ -0,0 +1,16 @@ +import UserDataCleanUpLogOutHandler from "../../src/features/auth/domain/logOut/UserDataCleanUpLogOutHandler" + +test("It deletes data for the read user ID", async () => { + let deletedUserId: string | undefined + const sut = new UserDataCleanUpLogOutHandler({ + async getUserId() { + return "foo" + }, + }, { + async delete(userId) { + deletedUserId = userId + } + }) + await sut.handleLogOut() + expect(deletedUserId).toBe("foo") +}) diff --git a/__test__/common/authHandler/logoutHandler.test.ts b/__test__/common/authHandler/logoutHandler.test.ts deleted file mode 100644 index 8060eb45..00000000 --- a/__test__/common/authHandler/logoutHandler.test.ts +++ /dev/null @@ -1,41 +0,0 @@ -import logoutHandler from "../../../src/common/authHandler/logout" - -test("It deletes the user's auth token", async () => { - let didDeleteAuthToken = false - logoutHandler({ - async getOAuthToken() { - throw new Error("Not implemented") - }, - async storeOAuthToken() {}, - async deleteOAuthToken() { - didDeleteAuthToken = true - } - }, { - async get() { - return [] - }, - async set() {}, - async delete() {} - }) - expect(didDeleteAuthToken).toBeTruthy() -}) - -test("It deletes the cached projects", async () => { - let didDeleteProjects = false - logoutHandler({ - async getOAuthToken() { - throw new Error("Not implemented") - }, - async storeOAuthToken() {}, - async deleteOAuthToken() {} - }, { - async get() { - return [] - }, - async set() {}, - async delete() { - didDeleteProjects = true - } - }) - expect(didDeleteProjects).toBeTruthy() -}) diff --git a/src/app/api/auth/[auth0]/route.ts b/src/app/api/auth/[auth0]/route.ts index 072c3a9b..7819d3e1 100644 --- a/src/app/api/auth/[auth0]/route.ts +++ b/src/app/api/auth/[auth0]/route.ts @@ -1,19 +1,13 @@ -import { NextRequest, NextResponse } from "next/server" +import { NextResponse } from "next/server" import { handleAuth, handleCallback, handleLogout, AfterCallbackAppRoute, NextAppRouterHandler, - AppRouteHandlerFnContext, AppRouterOnError } from "@auth0/nextjs-auth0" -import { - initialOAuthTokenService, - sessionOAuthTokenRepository, - projectRepository, - logoutHandler -} from "@/composition" +import { logOutHandler } from "@/composition" const { SHAPE_DOCS_BASE_URL } = process.env @@ -27,12 +21,8 @@ const onError: AppRouterOnError = async () => { return NextResponse.redirect(url) } -const onLogout: NextAppRouterHandler = async (req: NextRequest, ctx: AppRouteHandlerFnContext) => { - await Promise.all([ - sessionOAuthTokenRepository.deleteOAuthToken().catch(() => null), - projectRepository.delete().catch(() => null) - ]) - await logoutHandler() +const onLogout: NextAppRouterHandler = async (req, ctx) => { + await logOutHandler.handleLogOut() return await handleLogout(req, ctx) } diff --git a/src/common/authHandler/logout.ts b/src/common/authHandler/logout.ts deleted file mode 100644 index 34d9a611..00000000 --- a/src/common/authHandler/logout.ts +++ /dev/null @@ -1,12 +0,0 @@ -import ISessionOAuthTokenRepository from "@/features/auth/domain/ISessionOAuthTokenRepository" -import IProjectRepository from "@/features/projects/domain/IProjectRepository" - -export default async function logoutHandler( - sessionOAuthTokenRepository: ISessionOAuthTokenRepository, - projectRepository: IProjectRepository -) { - await Promise.all([ - sessionOAuthTokenRepository.deleteOAuthToken().catch(() => null), - projectRepository.delete().catch(() => null) - ]) -} diff --git a/src/composition.ts b/src/composition.ts index b464ff06..da37527b 100644 --- a/src/composition.ts +++ b/src/composition.ts @@ -2,6 +2,8 @@ import AccessTokenRefreshingGitHubClient from "@/common/github/AccessTokenRefres import Auth0RefreshTokenReader from "@/features/auth/data/Auth0RefreshTokenReader" import Auth0Session from "@/common/session/Auth0Session" import CachingProjectDataSource from "@/features/projects/domain/CachingProjectDataSource" +import CompositeLogOutHandler from "@/features/auth/domain/logOut/CompositeLogOutHandler" +import ErrorIgnoringLogOutHandler from "@/features/auth/domain/logOut/ErrorIgnoringLogOutHandler" import GitHubClient from "@/common/github/GitHubClient" import GitHubOAuthTokenRefresher from "@/features/auth/data/GitHubOAuthTokenRefresher" import GitHubOrganizationSessionValidator from "@/common/session/GitHubOrganizationSessionValidator" @@ -18,7 +20,7 @@ import SessionMutexFactory from "@/common/mutex/SessionMutexFactory" import SessionOAuthTokenRepository from "@/features/auth/domain/SessionOAuthTokenRepository" import SessionValidatingProjectDataSource from "@/features/projects/domain/SessionValidatingProjectDataSource" import OAuthTokenRepository from "@/features/auth/domain/OAuthTokenRepository" -import authLogoutHandler from "@/common/authHandler/logout" +import UserDataCleanUpLogOutHandler from "@/features/auth/domain/logOut/UserDataCleanUpLogOutHandler" const { AUTH0_MANAGEMENT_DOMAIN, @@ -77,12 +79,14 @@ export const sessionValidator = new GitHubOrganizationSessionValidator( GITHUB_ORGANIZATION_NAME ) +const projectUserDataRepository = new KeyValueUserDataRepository( + new RedisKeyValueStore(REDIS_URL), + "projects" +) + export const projectRepository = new ProjectRepository( - new Auth0Session(), - new KeyValueUserDataRepository( - new RedisKeyValueStore(REDIS_URL), - "projects" - ) + session, + projectUserDataRepository ) export const projectDataSource = new CachingProjectDataSource( @@ -107,6 +111,9 @@ export const initialOAuthTokenService = new InitialOAuthTokenService({ oAuthTokenRepository: new OAuthTokenRepository(oAuthTokenRepository) }) -export const logoutHandler = async () => { - await authLogoutHandler(sessionOAuthTokenRepository, projectRepository) -} +export const logOutHandler = new ErrorIgnoringLogOutHandler( + new CompositeLogOutHandler([ + new UserDataCleanUpLogOutHandler(session, projectUserDataRepository), + new UserDataCleanUpLogOutHandler(session, oAuthTokenRepository) + ]) +) diff --git a/src/features/auth/domain/logOut/CompositeLogOutHandler.ts b/src/features/auth/domain/logOut/CompositeLogOutHandler.ts new file mode 100644 index 00000000..ab69b11a --- /dev/null +++ b/src/features/auth/domain/logOut/CompositeLogOutHandler.ts @@ -0,0 +1,14 @@ +import ILogOutHandler from "./ILogOutHandler" + +export default class CompositeLogOutHandler implements ILogOutHandler { + private readonly handlers: ILogOutHandler[] + + constructor(handlers: ILogOutHandler[]) { + this.handlers = handlers + } + + async handleLogOut(): Promise { + const promises = this.handlers.map(e => e.handleLogOut()) + await Promise.all(promises) + } +} diff --git a/src/features/auth/domain/logOut/ErrorIgnoringLogOutHandler.ts b/src/features/auth/domain/logOut/ErrorIgnoringLogOutHandler.ts new file mode 100644 index 00000000..bf33710e --- /dev/null +++ b/src/features/auth/domain/logOut/ErrorIgnoringLogOutHandler.ts @@ -0,0 +1,17 @@ +import ILogOutHandler from "./ILogOutHandler" + +export default class ErrorIgnoringLogOutHandler implements ILogOutHandler { + private readonly handler: ILogOutHandler + + constructor(handler: ILogOutHandler) { + this.handler = handler + } + + async handleLogOut(): Promise { + try { + await this.handler.handleLogOut() + } catch { + // We intentionally do not handle errors. + } + } +} diff --git a/src/features/auth/domain/logOut/ILogOutHandler.ts b/src/features/auth/domain/logOut/ILogOutHandler.ts new file mode 100644 index 00000000..88cab6ac --- /dev/null +++ b/src/features/auth/domain/logOut/ILogOutHandler.ts @@ -0,0 +1,3 @@ +export default interface ILogOutHandler { + handleLogOut(): Promise +} diff --git a/src/features/auth/domain/logOut/UserDataCleanUpLogOutHandler.ts b/src/features/auth/domain/logOut/UserDataCleanUpLogOutHandler.ts new file mode 100644 index 00000000..08f724e2 --- /dev/null +++ b/src/features/auth/domain/logOut/UserDataCleanUpLogOutHandler.ts @@ -0,0 +1,24 @@ +import ILogOutHandler from "./ILogOutHandler" + +interface IUserIDReader { + getUserId(): Promise +} + +interface Repository { + delete(userId: string): Promise +} + +export default class UserDataCleanUpLogOutHandler implements ILogOutHandler { + private readonly userIdReader: IUserIDReader + private readonly repository: Repository + + constructor(userIdReader: IUserIDReader, repository: Repository) { + this.userIdReader = userIdReader + this.repository = repository + } + + async handleLogOut(): Promise { + const userId = await this.userIdReader.getUserId() + return await this.repository.delete(userId) + } +} From bc3a5f100e28532d9c6b841d3fe81a99ac539e99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20St=C3=B8vring?= Date: Tue, 7 Nov 2023 09:24:49 +0100 Subject: [PATCH 2/2] Fixes compile errors after merge --- src/app/api/auth/[auth0]/route.ts | 2 +- src/composition.ts | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/app/api/auth/[auth0]/route.ts b/src/app/api/auth/[auth0]/route.ts index 7819d3e1..b8e11a5e 100644 --- a/src/app/api/auth/[auth0]/route.ts +++ b/src/app/api/auth/[auth0]/route.ts @@ -7,7 +7,7 @@ import { NextAppRouterHandler, AppRouterOnError } from "@auth0/nextjs-auth0" -import { logOutHandler } from "@/composition" +import { initialOAuthTokenService, logOutHandler } from "@/composition" const { SHAPE_DOCS_BASE_URL } = process.env diff --git a/src/composition.ts b/src/composition.ts index da37527b..af57ee2f 100644 --- a/src/composition.ts +++ b/src/composition.ts @@ -36,13 +36,15 @@ const { const gitHubPrivateKey = Buffer.from(GITHUB_PRIVATE_KEY_BASE_64, "base64").toString("utf-8") +const session = new Auth0Session() + const oAuthTokenRepository = new KeyValueUserDataRepository( new RedisKeyValueStore(REDIS_URL), "authToken" ) export const sessionOAuthTokenRepository = new SessionOAuthTokenRepository( - new SessionDataRepository(new Auth0Session(), oAuthTokenRepository) + new SessionDataRepository(session, oAuthTokenRepository) ) const gitHubOAuthTokenRefresher = new GitHubOAuthTokenRefresher({ @@ -57,7 +59,7 @@ export const gitHubClient = new AccessTokenRefreshingGitHubClient( new LockingAccessTokenRefresher( new SessionMutexFactory( new RedisKeyedMutexFactory(REDIS_URL), - new Auth0Session(), + session, "mutexAccessToken" ), sessionOAuthTokenRepository,