[OLH-1727] Add logging and testing for session state during authentication flow

src/components/oidc-callback/call-back-utils.ts

@@ -48,6 +48,11 @@ export async function generateTokenSet(
   queryParams: CallbackParamsType,
   clientAssertion: string
 ) {
+  logger.info(
+    { trace: req.res?.locals.trace },
+    `request session state: ${req.session.state}`
+  );
+
   const tokenSet: TokenSet = await req.oidc.callback(
     req.oidc.metadata.redirect_uris[0],
     queryParams,
@@ -60,6 +65,21 @@ export async function generateTokenSet(
       },
     }
   );
+  logger.info(
+    { trace: req.res?.locals.trace },
+    `Generated token session state: ${tokenSet.session_state}`
+  );
+  if (tokenSet.session_state !== req.session.state) {
+    this.handleOidcCallbackError(
+      req,
+      req.res!,
+      {
+        error: "session_state_mismatch",
+        description: "Session state mismatch after OICD callback",
+      },
+      false
+    );
+  }
   return tokenSet;
 }
 

src/components/oidc-callback/tests/callback-utils.test.ts

@@ -27,6 +27,7 @@ describe("callback-utils", () => {
       const mockTokenSet = {
         access_token: "fake-access-token",
         id_token: "fake-id-token",
+        session_state: "mock-state",
       } as TokenSet;
 
       callbackStub = sinon.stub().resolves(mockTokenSet);
@@ -83,6 +84,24 @@ describe("callback-utils", () => {
       expect(tokenSet).to.have.property("access_token", "fake-access-token");
       expect(tokenSet).to.have.property("id_token", "fake-id-token");
     });
+
+    it("should verify that the session state returned by the the oicd.callback call matches what passed in", async () => {
+      const queryParams = {
+        code: "fake-code",
+        state: "mock-state-1",
+      };
+
+      req.session.state = "mock-state-2";
+
+      const clientAssertion = "mock-client-assertion";
+
+      try {
+        await generateTokenSet(req, queryParams, clientAssertion);
+        expect.fail("Expected error was not thrown");
+      } catch (error) {
+        expect(error).to.exist;
+      }
+    });
   });
 
   describe("determineRedirectUri", () => {
@@ -165,28 +184,30 @@ describe("callback-utils", () => {
     });
 
     it("clears session and redirects to UNAVAILABLE_TEMPORARY", async () => {
-        const deleteExpressSessionStub = sinon.stub(
-            sessionStore,
-            "deleteExpressSession"
-        );
-        const loggerWarnStub = sinon.stub(logger, "warn");
-
-        const req: any = { session: {}, oidc: {}, cookies: {} };
-        const res: any = {
-            locals: { trace: "trace-id" },
-            redirect: sinon.fake(),
-        };
-
-        const queryParams = {
-            error: "temporarily_unavailable",
-            error_description: "The authorization server is temporarily unavailable",
-        };
-
-        await handleOidcCallbackError(req, res, queryParams);
-
-        expect(loggerWarnStub.calledOnce).to.be.true;
-        expect(deleteExpressSessionStub.calledWith(req)).to.be.true;
-        expect(res.redirect.calledWith(PATH_DATA.UNAVAILABLE_TEMPORARY.url)).to.be.true;
+      const deleteExpressSessionStub = sinon.stub(
+        sessionStore,
+        "deleteExpressSession"
+      );
+      const loggerWarnStub = sinon.stub(logger, "warn");
+
+      const req: any = { session: {}, oidc: {}, cookies: {} };
+      const res: any = {
+        locals: { trace: "trace-id" },
+        redirect: sinon.fake(),
+      };
+
+      const queryParams = {
+        error: "temporarily_unavailable",
+        error_description:
+          "The authorization server is temporarily unavailable",
+      };
+
+      await handleOidcCallbackError(req, res, queryParams);
+
+      expect(loggerWarnStub.calledOnce).to.be.true;
+      expect(deleteExpressSessionStub.calledWith(req)).to.be.true;
+      expect(res.redirect.calledWith(PATH_DATA.UNAVAILABLE_TEMPORARY.url)).to.be
+        .true;
     });
   });