Adjust authorization logic for tokens with tenant id claim

47chapters · Feb 6, 2024 · 23cbc78 · 23cbc78
1 parent 4567cf8
commit 23cbc78
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 32 deletions.
diff --git a/apps/api/src/__tests__/server.test.ts b/apps/api/src/__tests__/server.test.ts
@@ -30,19 +30,14 @@ const createTestServer = () => {
 
   const handler: RequestHandler = (request, response) =>
     response.json({
-      params: request.params,
+      locals: response.locals,
       token: (request as AuthenticatedRequest).user.decodedJwt,
     });
 
   app.get("/1", authenticate(), handler);
-  app.get("/2", authenticate({ useTenantIdFromToken: true }), handler);
+  app.get("/2", authenticate(), handler);
   app.get("/3/:tenantId", authenticate(), authorizeTenant(), handler);
-  app.get(
-    "/3",
-    authenticate({ useTenantIdFromToken: true }),
-    authorizeTenant(),
-    handler
-  );
+  app.get("/3", authenticate(), authorizeTenant(), handler);
 
   return app;
 };
@@ -126,7 +121,7 @@ describe("api", () => {
       sub: "letsgo:test",
       [TenantIdClaim]: testTenantId,
     });
-    expect(res.body.params).toMatchObject({
+    expect(res.body.locals).toMatchObject({
       tenantId: testTenantId,
     });
   });
@@ -167,8 +162,16 @@ describe("api", () => {
       sub: "letsgo:test",
       [TenantIdClaim]: testTenantId,
     });
-    expect(res.body.params).toMatchObject({
+    expect(res.body.locals).toMatchObject({
       tenantId: testTenantId,
     });
   });
+
+  it("GET /3 with access token without tenantId returns 403", async () => {
+    const accessToken = await getAccessToken();
+    const res = await supertest(createTestServer())
+      .get("/3")
+      .set("Authorization", `Bearer ${accessToken}`)
+      .expect(403);
+  });
 });
diff --git a/apps/api/src/middleware/authenticate.ts b/apps/api/src/middleware/authenticate.ts
@@ -41,16 +41,7 @@ export function isAuthenticatedRequest(
   return (request as AuthenticatedRequest).user !== undefined;
 }
 
-export interface AuthenticationOptions {
-  /**
-   * If the tenantId slug is absent in the request path, use the tenantId claim from the token to
-     synthetically populate it. This allows for shorter URLs to be used with access tokens
-     that specify the tenantId.
-   */
-  useTenantIdFromToken?: boolean;
-}
-
-export function authenticate(options?: AuthenticationOptions): RequestHandler {
+export function authenticate(): RequestHandler {
   const authenticationHandler: RequestHandler = async (
     request,
     response,
@@ -84,16 +75,10 @@ export function authenticate(options?: AuthenticationOptions): RequestHandler {
           jwt: token,
           decodedJwt,
         };
-        if (options?.useTenantIdFromToken) {
-          // If the tenantId slug is absent in the request path, use the tenantId claim from the token to
-          // synthetically populate it. This allows for shorter URLs to be used with access tokens
-          // that specify the tenantId.
-          const tenantIdClaim = (decodedJwt.payload as JwtPayload)[
-            TenantIdClaim
-          ];
-          if (tenantIdClaim && !request.params.tenantId) {
-            request.params.tenantId = tenantIdClaim as string;
-          }
+        const tenantIdClaim = (decodedJwt.payload as JwtPayload)[TenantIdClaim];
+        if (tenantIdClaim) {
+          // Promote tenantId claim from the token to the request context
+          response.locals.tenantId = tenantIdClaim;
         }
         next();
         return;

diff --git a/apps/api/src/middleware/authorizeTenant.ts b/apps/api/src/middleware/authorizeTenant.ts
@@ -36,12 +36,13 @@ export function authorizeTenant(
       return;
     }
 
-    // If the access token specifies the tenantId claim, check if it matches the tenantId slug
+    // If the access token specifies the tenantId claim, ensure it either matches the tenantId slug
+    // or the tenantId slug is not defined
     const tenantIdClaim = (
       authenticatedRequest.user?.decodedJwt?.payload as JwtPayload
     )[TenantIdClaim];
     if (tenantIdClaim) {
-      if (tenantId === tenantIdClaim) {
+      if (tenantId === tenantIdClaim || tenantId === undefined) {
         next();
         return;
       }