Merge pull request #3495 from Infisical/ENG-2656

feat(login): Update all SSO login methods to use PKCE

backend/package-lock.json

@@ -92,10 +92,10 @@
         "ora": "^7.0.1",
         "oracledb": "^6.4.0",
         "otplib": "^12.0.1",
-        "passport-github": "^1.1.0",
         "passport-gitlab2": "^5.0.0",
         "passport-google-oauth20": "^2.0.0",
         "passport-ldapauth": "^3.0.1",
+        "passport-oauth2": "^1.8.0",
         "pg": "^8.11.3",
         "pg-boss": "^10.1.5",
         "pg-query-stream": "^4.5.3",
@@ -136,7 +136,6 @@
         "@types/lodash.isequal": "^4.5.8",
         "@types/node": "^20.17.30",
         "@types/nodemailer": "^6.4.14",
-        "@types/passport-github": "^1.1.12",
         "@types/passport-google-oauth20": "^2.0.14",
         "@types/pg": "^8.10.9",
         "@types/picomatch": "^2.3.3",
@@ -10124,17 +10123,6 @@
         "@types/express": "*"
       }
     },
-    "node_modules/@types/passport-github": {
-      "version": "1.1.12",
-      "resolved": "https://registry.npmjs.org/@types/passport-github/-/passport-github-1.1.12.tgz",
-      "integrity": "sha512-VJpMEIH+cOoXB694QgcxuvWy2wPd1Oq3gqrg2Y9DMVBYs9TmH9L14qnqPDZsNMZKBDH+SvqRsGZj9SgHYeDgcA==",
-      "dev": true,
-      "dependencies": {
-        "@types/express": "*",
-        "@types/passport": "*",
-        "@types/passport-oauth2": "*"
-      }
-    },
     "node_modules/@types/passport-google-oauth20": {
       "version": "2.0.14",
       "resolved": "https://registry.npmjs.org/@types/passport-google-oauth20/-/passport-google-oauth20-2.0.14.tgz",
@@ -18397,9 +18385,10 @@
       "integrity": "sha512-p1TRH/edngVEHVbwqWnxUViEmq5znDvyB+Sik5cmuLpGOIfDf/39zLiq3swPF8Vakqn+gvNiOQAZu8djYlQILA=="
     },
     "node_modules/oauth": {
-      "version": "0.9.15",
+      "version": "0.10.2",
-      "resolved": "https://registry.npmjs.org/oauth/-/oauth-0.9.15.tgz",
+      "resolved": "https://registry.npmjs.org/oauth/-/oauth-0.10.2.tgz",
-      "integrity": "sha512-a5ERWK1kh38ExDEfoO6qUHJb32rd7aYmPHuyCu3Fta/cnICvYmgd2uhuKXvPD+PXB+gCEYYEaQdIRAjCOwAKNA=="
+      "integrity": "sha512-JtFnB+8nxDEXgNyniwz573xxbKSOu3R8D40xQKqcjwJ2CDkYqUDI53o6IuzDJBx60Z8VKCm271+t8iFjakrl8Q==",
+      "license": "MIT"
     },
     "node_modules/object-assign": {
       "version": "4.1.1",
@@ -19082,17 +19071,6 @@
         "url": "https://github.com/sponsors/jaredhanson"
       }
     },
-    "node_modules/passport-github": {
-      "version": "1.1.0",
-      "resolved": "https://registry.npmjs.org/passport-github/-/passport-github-1.1.0.tgz",
-      "integrity": "sha512-XARXJycE6fFh/dxF+Uut8OjlwbFEXgbPVj/+V+K7cvriRK7VcAOm+NgBmbiLM9Qv3SSxEAV+V6fIk89nYHXa8A==",
-      "dependencies": {
-        "passport-oauth2": "1.x.x"
-      },
-      "engines": {
-        "node": ">= 0.4.0"
-      }
-    },
     "node_modules/passport-gitlab2": {
       "version": "5.0.0",
       "resolved": "https://registry.npmjs.org/passport-gitlab2/-/passport-gitlab2-5.0.0.tgz",
@@ -19128,12 +19106,13 @@
       }
     },
     "node_modules/passport-oauth2": {
-      "version": "1.7.0",
+      "version": "1.8.0",
-      "resolved": "https://registry.npmjs.org/passport-oauth2/-/passport-oauth2-1.7.0.tgz",
+      "resolved": "https://registry.npmjs.org/passport-oauth2/-/passport-oauth2-1.8.0.tgz",
-      "integrity": "sha512-j2gf34szdTF2Onw3+76alNnaAExlUmHvkc7cL+cmaS5NzHzDP/BvFHJruueQ9XAeNOdpI+CH+PWid8RA7KCwAQ==",
+      "integrity": "sha512-cjsQbOrXIDE4P8nNb3FQRCCmJJ/utnFKEz2NX209f7KOHPoX18gF7gBzBbLLsj2/je4KrgiwLLGjf0lm9rtTBA==",
+      "license": "MIT",
       "dependencies": {
         "base64url": "3.x.x",
-        "oauth": "0.9.x",
+        "oauth": "0.10.x",
         "passport-strategy": "1.x.x",
         "uid2": "0.0.x",
         "utils-merge": "1.x.x"

backend/package.json

@@ -91,7 +91,6 @@
     "@types/lodash.isequal": "^4.5.8",
     "@types/node": "^20.17.30",
     "@types/nodemailer": "^6.4.14",
-    "@types/passport-github": "^1.1.12",
     "@types/passport-google-oauth20": "^2.0.14",
     "@types/pg": "^8.10.9",
     "@types/picomatch": "^2.3.3",
@@ -209,10 +208,10 @@
     "ora": "^7.0.1",
     "oracledb": "^6.4.0",
     "otplib": "^12.0.1",
-    "passport-github": "^1.1.0",
     "passport-gitlab2": "^5.0.0",
     "passport-google-oauth20": "^2.0.0",
     "passport-ldapauth": "^3.0.1",
+    "passport-oauth2": "^1.8.0",
     "pg": "^8.11.3",
     "pg-boss": "^10.1.5",
     "pg-query-stream": "^4.5.3",

backend/src/ee/services/oidc/oidc-config-service.ts

@@ -685,10 +685,16 @@ export const oidcConfigServiceFactory = ({
       id_token_signed_response_alg: oidcCfg.jwtSignatureAlgorithm
     });
 
+    // Check if the OIDC provider supports PKCE
+    const codeChallengeMethods = client.issuer.metadata.code_challenge_methods_supported;
+    const supportsPKCE = Array.isArray(codeChallengeMethods) && codeChallengeMethods.includes("S256");
+
     const strategy = new OpenIdStrategy(
       {
         client,
-        passReqToCallback: true
+        passReqToCallback: true,
+        usePKCE: supportsPKCE,
+        params: supportsPKCE ? { code_challenge_method: "S256" } : undefined
       },
       // eslint-disable-next-line @typescript-eslint/no-explicit-any
       (_req: any, tokenSet: TokenSet, cb: any) => {

backend/src/lib/requests/github.ts

@@ -16,3 +16,17 @@ export const fetchGithubEmails = async (accessToken: string) => {
   });
   return data;
 };
+
+type TGithubUser = {
+  name?: string;
+  login: string;
+};
+
+export const fetchGithubUser = async (accessToken: string) => {
+  const { data } = await request.get<TGithubUser>(`${INTEGRATION_GITHUB_API_URL}/user`, {
+    headers: {
+      Authorization: `Bearer ${accessToken}`
+    }
+  });
+  return data;
+};

backend/src/server/routes/v1/sso-router.ts

@@ -9,9 +9,9 @@
 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";
+import { Strategy as OAuth2Strategy } from "passport-oauth2";
 import { z } from "zod";
 
 import { INFISICAL_PROVIDER_GITHUB_ACCESS_TOKEN } from "@app/lib/config/const";
@@ -19,7 +19,7 @@ import { getConfig } from "@app/lib/config/env";
 import { BadRequestError, NotFoundError } from "@app/lib/errors";
 import { logger } from "@app/lib/logger";
 import { ms } from "@app/lib/ms";
-import { fetchGithubEmails } from "@app/lib/requests/github";
+import { fetchGithubEmails, fetchGithubUser } from "@app/lib/requests/github";
 import { authRateLimit } from "@app/server/config/rateLimiter";
 import { AuthMethod } from "@app/services/auth/auth-type";
 import { OrgAuthMethod } from "@app/services/org/org-types";
@@ -44,6 +44,7 @@ export const registerSsoRouter = async (server: FastifyZodProvider) => {
   });
   await server.register(passport.initialize());
   await server.register(passport.secureSession());
+
   // passport oauth strategy for Google
   const isGoogleOauthActive = Boolean(appCfg.CLIENT_ID_GOOGLE_LOGIN && appCfg.CLIENT_SECRET_GOOGLE_LOGIN);
   if (isGoogleOauthActive) {
@@ -54,8 +55,9 @@ 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
+          state: true,
+          pkce: true
         },
         // eslint-disable-next-line
         async (req, _accessToken, _refreshToken, profile, cb) => {
@@ -91,34 +93,44 @@ export const registerSsoRouter = async (server: FastifyZodProvider) => {
   const isGithubOauthActive = Boolean(appCfg.CLIENT_SECRET_GITHUB_LOGIN && appCfg.CLIENT_ID_GITHUB_LOGIN);
   if (isGithubOauthActive) {
     passport.use(
-      new GitHubStrategy(
+      "github",
+      new OAuth2Strategy(
         {
-          passReqToCallback: true,
+          authorizationURL: "https://github.com/login/oauth/authorize",
-          clientID: appCfg.CLIENT_ID_GITHUB_LOGIN as string,
+          tokenURL: "https://github.com/login/oauth/access_token",
-          clientSecret: appCfg.CLIENT_SECRET_GITHUB_LOGIN as string,
+          clientID: appCfg.CLIENT_ID_GITHUB_LOGIN!,
+          clientSecret: appCfg.CLIENT_SECRET_GITHUB_LOGIN!,
           callbackURL: `${appCfg.SITE_URL}/api/v1/sso/github`,
           scope: ["user:email", "read:org"],
-          // akhilmhdh: because the ts type for this is outdated by the maintainer
+          state: true,
-          state: true as unknown as string
+          pkce: true,
+          passReqToCallback: true
         },
         // eslint-disable-next-line
-        async (req, accessToken, _refreshToken, profile, cb) => {
+        async (req: any, accessToken: string, _refreshToken: string, _profile: any, done: Function) => {
-          // @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];
+
+            if (!email) throw new Error("No primary email found");
+
+            // profile does not get automatically populated so we need to manually fetch user info
+            const user = await fetchGithubUser(accessToken);
+
+            const callbackPort = req.session.get("callbackPort");
+
             const { isUserCompleted, providerAuthToken } = await server.services.login.oauth2Login({
               email,
-              firstName: profile.displayName || profile.username || "",
+              firstName: user.name || user.login,
               lastName: "",
               authMethod: AuthMethod.GITHUB,
               callbackPort
             });
-            return cb(null, { isUserCompleted, providerAuthToken, externalProviderAccessToken: accessToken });
+
-          } catch (error) {
+            done(null, { isUserCompleted, providerAuthToken, externalProviderAccessToken: accessToken });
-            logger.error(error);
+          } catch (err) {
-            cb(error as Error, false);
+            logger.error(err);
+            done(err as Error, false);
           }
         }
       )
@@ -138,7 +150,8 @@ export const registerSsoRouter = async (server: FastifyZodProvider) => {
           clientSecret: appCfg.CLIENT_SECRET_GITLAB_LOGIN,
           callbackURL: `${appCfg.SITE_URL}/api/v1/sso/gitlab`,
           baseURL: appCfg.CLIENT_GITLAB_LOGIN_URL,
-          state: true
+          state: true,
+          pkce: true
         },
         async (req: any, _accessToken: string, _refreshToken: string, profile: any, cb: any) => {
           try {