Merge pull request #2797 from akhilmhdh/feat/oauth2-csrf

feat: resolved csrf for oauth2 using state parameter
2026-09-22 13:39:35 +00:00 · 2024-12-05 14:37:47 +05:30
parent 095b26c8c9 e4b149a849
commit ef82c664a6
8 changed files with 116 additions and 52 deletions
--- a/backend/e2e-test/vitest-environment-knex.ts
+++ b/backend/e2e-test/vitest-environment-knex.ts
@@ -59,7 +59,7 @@ export default {
      const hsmModule = initializeHsmModule();
      hsmModule.initialize();

-      const server = await main({ db, smtp, logger, queue, keyStore, hsmModule: hsmModule.getModule() });
+      const server = await main({ db, smtp, logger, queue, keyStore, hsmModule: hsmModule.getModule(), redis });

      // @ts-expect-error type
      globalThis.testServer = server;
--- a/backend/src/@types/fastify.d.ts
+++ b/backend/src/@types/fastify.d.ts
@@ -1,5 +1,7 @@
 import "fastify";

+import { Redis } from "ioredis";
+
 import { TUsers } from "@app/db/schemas";
 import { TAccessApprovalPolicyServiceFactory } from "@app/ee/services/access-approval-policy/access-approval-policy-service";
 import { TAccessApprovalRequestServiceFactory } from "@app/ee/services/access-approval-request/access-approval-request-service";
@@ -87,6 +89,10 @@ import { TWebhookServiceFactory } from "@app/services/webhook/webhook-service";
 import { TWorkflowIntegrationServiceFactory } from "@app/services/workflow-integration/workflow-integration-service";

 declare module "fastify" {
+  interface Session {
+    callbackPort: string;
+  }
+
  interface FastifyRequest {
    realIp: string;
    // used for mfa session authentication
@@ -115,6 +121,7 @@ declare module "fastify" {
  }

  interface FastifyInstance {
+    redis: Redis;
    services: {
      login: TAuthLoginFactory;
      password: TAuthPasswordFactory;
--- a/backend/src/ee/routes/v1/oidc-router.ts
+++ b/backend/src/ee/routes/v1/oidc-router.ts
@@ -9,7 +9,6 @@
 import { Authenticator, Strategy } from "@fastify/passport";
 import fastifySession from "@fastify/session";
 import RedisStore from "connect-redis";
-import { Redis } from "ioredis";
 import { z } from "zod";

 import { OidcConfigsSchema } from "@app/db/schemas/oidc-configs";
@@ -21,7 +20,6 @@ import { AuthMode } from "@app/services/auth/auth-type";

 export const registerOidcRouter = async (server: FastifyZodProvider) => {
  const appCfg = getConfig();
-  const redis = new Redis(appCfg.REDIS_URL);
  const passport = new Authenticator({ key: "oidc", userProperty: "passportUser" });

  /*
@@ -30,7 +28,7 @@ export const registerOidcRouter = async (server: FastifyZodProvider) => {
  - Fastify session <> Redis structure is based on the ff: https://github.com/fastify/session/blob/master/examples/redis.js
  */
  const redisStore = new RedisStore({
-    client: redis,
+    client: server.redis,
    prefix: "oidc-session:",
    ttl: 600 // 10 minutes
  });
--- a/backend/src/main.ts
+++ b/backend/src/main.ts
@@ -1,6 +1,7 @@
 import "./lib/telemetry/instrumentation";

 import dotenv from "dotenv";
+import { Redis } from "ioredis";
 import path from "path";

 import { initializeHsmModule } from "@app/ee/services/hsm/hsm-fns";
@@ -60,11 +61,12 @@ const run = async () => {
  await queue.initialize();

  const keyStore = keyStoreFactory(appCfg.REDIS_URL);
+  const redis = new Redis(appCfg.REDIS_URL);

  const hsmModule = initializeHsmModule();
  hsmModule.initialize();

-  const server = await main({ db, auditLogDb, hsmModule: hsmModule.getModule(), smtp, logger, queue, keyStore });
+  const server = await main({ db, auditLogDb, hsmModule: hsmModule.getModule(), smtp, logger, queue, keyStore, redis });
  const bootstrap = await bootstrapCheck({ db });

  // eslint-disable-next-line
--- a/backend/src/server/app.ts
+++ b/backend/src/server/app.ts
@@ -12,6 +12,7 @@ import type { FastifyRateLimitOptions } from "@fastify/rate-limit";
 import ratelimiter from "@fastify/rate-limit";
 import { fastifyRequestContext } from "@fastify/request-context";
 import fastify from "fastify";
+import { Redis } from "ioredis";
 import { Knex } from "knex";

 import { HsmModule } from "@app/ee/services/hsm/hsm-types";
@@ -41,10 +42,11 @@ type TMain = {
  queue: TQueueServiceFactory;
  keyStore: TKeyStoreFactory;
  hsmModule: HsmModule;
+  redis: Redis;
 };

 // Run the server!
-export const main = async ({ db, hsmModule, auditLogDb, smtp, logger, queue, keyStore }: TMain) => {
+export const main = async ({ db, hsmModule, auditLogDb, smtp, logger, queue, keyStore, redis }: TMain) => {
  const appCfg = getConfig();

  const server = fastify({
@@ -60,6 +62,7 @@ export const main = async ({ db, hsmModule, auditLogDb, smtp, logger, queue, key
  server.setValidatorCompiler(validatorCompiler);
  server.setSerializerCompiler(serializerCompiler);

+  server.decorate("redis", redis);
  server.addContentTypeParser("application/scim+json", { parseAs: "string" }, (_, body, done) => {
    try {
      const strBody = body instanceof Buffer ? body.toString() : body;
--- a/backend/src/server/routes/v1/sso-router.ts
+++ b/backend/src/server/routes/v1/sso-router.ts
@@ -8,6 +8,7 @@

 import { Authenticator } from "@fastify/passport";
 import fastifySession from "@fastify/session";
+import RedisStore from "connect-redis";
 import { Strategy as GitHubStrategy } from "passport-github";
 import { Strategy as GitLabStrategy } from "passport-gitlab2";
 import { Strategy as GoogleStrategy } from "passport-google-oauth20";
@@ -23,8 +24,22 @@ import { OrgAuthMethod } from "@app/services/org/org-types";

 export const registerSsoRouter = async (server: FastifyZodProvider) => {
  const appCfg = getConfig();
+
  const passport = new Authenticator({ key: "sso", userProperty: "passportUser" });
-  await server.register(fastifySession, { secret: appCfg.COOKIE_SECRET_SIGN_KEY });
+  const redisStore = new RedisStore({
+    client: server.redis,
+    prefix: "oauth-session:",
+    ttl: 600 // 10 minutes
+  });
+
+  await server.register(fastifySession, {
+    secret: appCfg.COOKIE_SECRET_SIGN_KEY,
+    store: redisStore,
+    cookie: {
+      secure: appCfg.HTTPS_ENABLED,
+      sameSite: "lax" // we want cookies to be sent to Infisical in redirects originating from IDP server
+    }
+  });
  await server.register(passport.initialize());
  await server.register(passport.secureSession());
  // passport oauth strategy for Google
@@ -37,11 +52,15 @@ export const registerSsoRouter = async (server: FastifyZodProvider) => {
          clientID: appCfg.CLIENT_ID_GOOGLE_LOGIN as string,
          clientSecret: appCfg.CLIENT_SECRET_GOOGLE_LOGIN as string,
          callbackURL: `${appCfg.SITE_URL}/api/v1/sso/google`,
-          scope: ["profile", " email"]
+          scope: ["profile", " email"],
+          state: true
        },
        // eslint-disable-next-line
        async (req, _accessToken, _refreshToken, profile, cb) => {
          try {
+            // @ts-expect-error this is because this is express type and not fastify
+            const callbackPort = req.session.get("callbackPort");
+
            const email = profile?.emails?.[0]?.value;
            if (!email)
              throw new NotFoundError({
@@ -54,7 +73,7 @@ export const registerSsoRouter = async (server: FastifyZodProvider) => {
              firstName: profile?.name?.givenName || "",
              lastName: profile?.name?.familyName || "",
              authMethod: AuthMethod.GOOGLE,
-              callbackPort: req.query.state as string
+              callbackPort
            });
            cb(null, { isUserCompleted, providerAuthToken });
          } catch (error) {
@@ -76,10 +95,14 @@ export const registerSsoRouter = async (server: FastifyZodProvider) => {
          clientID: appCfg.CLIENT_ID_GITHUB_LOGIN as string,
          clientSecret: appCfg.CLIENT_SECRET_GITHUB_LOGIN as string,
          callbackURL: `${appCfg.SITE_URL}/api/v1/sso/github`,
-          scope: ["user:email"]
+          scope: ["user:email"],
+          // akhilmhdh: because the ts type for this is outdated by the maintainer
+          state: true as unknown as string
        },
        // eslint-disable-next-line
        async (req, accessToken, _refreshToken, profile, cb) => {
+          // @ts-expect-error this is because this is express type and not fastify
+          const callbackPort = req.session.get("callbackPort");
          try {
            const ghEmails = await fetchGithubEmails(accessToken);
            const { email } = ghEmails.filter((gitHubEmail) => gitHubEmail.primary)[0];
@@ -88,7 +111,7 @@ export const registerSsoRouter = async (server: FastifyZodProvider) => {
              firstName: profile.displayName,
              lastName: "",
              authMethod: AuthMethod.GITHUB,
-              callbackPort: req.query.state as string
+              callbackPort
            });
            return cb(null, { isUserCompleted, providerAuthToken });
          } catch (error) {
@@ -112,17 +135,20 @@ export const registerSsoRouter = async (server: FastifyZodProvider) => {
          clientID: appCfg.CLIENT_ID_GITLAB_LOGIN,
          clientSecret: appCfg.CLIENT_SECRET_GITLAB_LOGIN,
          callbackURL: `${appCfg.SITE_URL}/api/v1/sso/gitlab`,
-          baseURL: appCfg.CLIENT_GITLAB_LOGIN_URL
+          baseURL: appCfg.CLIENT_GITLAB_LOGIN_URL,
+          state: true
        },
        async (req: any, _accessToken: string, _refreshToken: string, profile: any, cb: any) => {
          try {
+            const callbackPort = req.session.get("callbackPort");
+
            const email = profile.emails[0].value;
            const { isUserCompleted, providerAuthToken } = await server.services.login.oauth2Login({
              email,
              firstName: profile.displayName,
              lastName: "",
              authMethod: AuthMethod.GITLAB,
-              callbackPort: req.query.state as string
+              callbackPort
            });

            return cb(null, { isUserCompleted, providerAuthToken });
@@ -143,17 +169,24 @@ export const registerSsoRouter = async (server: FastifyZodProvider) => {
        callback_port: z.string().optional()
      })
    },
-    preValidation: (req, res) =>
-      (
-        passport.authenticate("google", {
-          scope: ["profile", "email"],
-          session: false,
-          state: req.query.callback_port,
-          authInfo: false
-          // this is due to zod type difference
-          // eslint-disable-next-line @typescript-eslint/no-explicit-any
-        }) as any
-      )(req, res),
+    preValidation: [
+      async (req, res) => {
+        const { callback_port: callbackPort } = req.query;
+        // ensure fresh session state per login attempt
+        await req.session.regenerate();
+        if (callbackPort) {
+          req.session.set("callbackPort", callbackPort);
+        }
+        return (
+          passport.authenticate("google", {
+            scope: ["profile", "email"],
+            authInfo: false
+            // this is due to zod type difference
+            // eslint-disable-next-line @typescript-eslint/no-explicit-any
+          }) as any
+        )(req, res);
+      }
+    ],
    handler: () => {}
  });

@@ -166,7 +199,8 @@ export const registerSsoRouter = async (server: FastifyZodProvider) => {
      authInfo: false
      // this is due to zod type difference
    }) as never,
-    handler: (req, res) => {
+    handler: async (req, res) => {
+      await req.session.destroy();
      if (req.passportUser.isUserCompleted) {
        return res.redirect(
          `${appCfg.SITE_URL}/login/sso?token=${encodeURIComponent(req.passportUser.providerAuthToken)}`
@@ -186,15 +220,24 @@ export const registerSsoRouter = async (server: FastifyZodProvider) => {
        callback_port: z.string().optional()
      })
    },
-    preValidation: (req, res) =>
-      (
-        passport.authenticate("github", {
-          session: false,
-          state: req.query.callback_port,
-          authInfo: false
-          // this is due to zod type difference
-        }) as any
-      )(req, res),
+    preValidation: [
+      async (req, res) => {
+        const { callback_port: callbackPort } = req.query;
+        // ensure fresh session state per login attempt
+        await req.session.regenerate();
+        if (callbackPort) {
+          req.session.set("callbackPort", callbackPort);
+        }
+
+        return (
+          passport.authenticate("github", {
+            session: false,
+            authInfo: false
+            // this is due to zod type difference
+          }) as any
+        )(req, res);
+      }
+    ],
    handler: () => {}
  });

@@ -245,7 +288,8 @@ export const registerSsoRouter = async (server: FastifyZodProvider) => {
      authInfo: false
      // this is due to zod type difference
    }) as any,
-    handler: (req, res) => {
+    handler: async (req, res) => {
+      await req.session.destroy();
      if (req.passportUser.isUserCompleted) {
        return res.redirect(
          `${appCfg.SITE_URL}/login/sso?token=${encodeURIComponent(req.passportUser.providerAuthToken)}`
@@ -265,16 +309,25 @@ export const registerSsoRouter = async (server: FastifyZodProvider) => {
        callback_port: z.string().optional()
      })
    },
-    preValidation: (req, res) =>
-      (
-        passport.authenticate("gitlab", {
-          session: false,
-          state: req.query.callback_port,
-          authInfo: false
-          // this is due to zod type difference
-          // eslint-disable-next-line @typescript-eslint/no-explicit-any
-        }) as any
-      )(req, res),
+    preValidation: [
+      async (req, res) => {
+        const { callback_port: callbackPort } = req.query;
+        // ensure fresh session state per login attempt
+        await req.session.regenerate();
+        if (callbackPort) {
+          req.session.set("callbackPort", callbackPort);
+        }
+
+        return (
+          passport.authenticate("gitlab", {
+            session: false,
+            authInfo: false
+            // this is due to zod type difference
+            // eslint-disable-next-line @typescript-eslint/no-explicit-any
+          }) as any
+        )(req, res);
+      }
+    ],
    handler: () => {}
  });

@@ -288,7 +341,8 @@ export const registerSsoRouter = async (server: FastifyZodProvider) => {
      // this is due to zod type difference
      // eslint-disable-next-line @typescript-eslint/no-explicit-any
    }) as any,
-    handler: (req, res) => {
+    handler: async (req, res) => {
+      await req.session.destroy();
      if (req.passportUser.isUserCompleted) {
        return res.redirect(
          `${appCfg.SITE_URL}/login/sso?token=${encodeURIComponent(req.passportUser.providerAuthToken)}`
--- a/backend/src/services/smtp/templates/organizationInvitation.handlebars
+++ b/backend/src/services/smtp/templates/organizationInvitation.handlebars
@@ -8,9 +8,9 @@
 </head>
 <body>
    <h2>Join your organization on Infisical</h2>
-    <p>{{inviterFirstName}} ({{inviterUsername}}) has invited you to their Infisical organization — {{organizationName}}</p>
-    <a href="{{callback_url}}?token={{token}}{{#if metadata}}&metadata={{metadata}}{{/if}}&to={{email}}&organization_id={{organizationId}}">Join now</a>
+    <p>{{inviterFirstName}} ({{inviterUsername}}) has invited you to their Infisical organization named {{organizationName}}</p>
+    <a href="{{callback_url}}?token={{token}}{{#if metadata}}&metadata={{metadata}}{{/if}}&to={{email}}&organization_id={{organizationId}}">Click to join</a>
    <h3>What is Infisical?</h3>
    <p>Infisical is an easy-to-use end-to-end encrypted tool that enables developers to sync and manage their secrets and configs.</p>
 </body>
-</html>
+</html>
--- a/backend/src/services/smtp/templates/workspaceInvitation.handlebars
+++ b/backend/src/services/smtp/templates/workspaceInvitation.handlebars
@@ -6,10 +6,10 @@
  </head>
  <body>
    <h2>Join your team on Infisical</h2>
-    <p>You have been invited to a new Infisical project — {{workspaceName}}</p>
-    <a href="{{callback_url}}">Join now</a>
+    <p>You have been invited to a new Infisical project named {{workspaceName}}</p>
+    <a href="{{callback_url}}">Click to join</a>
    <h3>What is Infisical?</h3>
    <p>Infisical is an easy-to-use end-to-end encrypted tool that enables developers to sync and manage their secrets
      and configs.</p>
  </body>
-</html>
+</html>