From e4b149a849a78031c9300e24075913d7009476d0 Mon Sep 17 00:00:00 2001 From: = Date: Tue, 26 Nov 2024 21:19:32 +0530 Subject: [PATCH] feat: resolved csrf for oauth2 using state parameter --- backend/e2e-test/vitest-environment-knex.ts | 2 +- backend/src/@types/fastify.d.ts | 7 + backend/src/ee/routes/v1/oidc-router.ts | 4 +- backend/src/main.ts | 4 +- backend/src/server/app.ts | 5 +- backend/src/server/routes/v1/sso-router.ts | 134 ++++++++++++------ .../organizationInvitation.handlebars | 6 +- .../templates/workspaceInvitation.handlebars | 6 +- 8 files changed, 116 insertions(+), 52 deletions(-) diff --git a/backend/e2e-test/vitest-environment-knex.ts b/backend/e2e-test/vitest-environment-knex.ts index 866b0f45f..e38921bbb 100644 --- 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; diff --git a/backend/src/@types/fastify.d.ts b/backend/src/@types/fastify.d.ts index 2843648da..4221eadcb 100644 --- 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; diff --git a/backend/src/ee/routes/v1/oidc-router.ts b/backend/src/ee/routes/v1/oidc-router.ts index e675121e9..cd25c5be5 100644 --- 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 }); diff --git a/backend/src/main.ts b/backend/src/main.ts index 7f62d6b1e..02b20b6f5 100644 --- 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"; @@ -57,11 +58,12 @@ const run = async () => { const smtp = smtpServiceFactory(formatSmtpConfig()); const queue = queueServiceFactory(appCfg.REDIS_URL); 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 diff --git a/backend/src/server/app.ts b/backend/src/server/app.ts index cf7dd622a..b9e17525c 100644 --- a/backend/src/server/app.ts +++ b/backend/src/server/app.ts @@ -11,6 +11,7 @@ import helmet from "@fastify/helmet"; import type { FastifyRateLimitOptions } from "@fastify/rate-limit"; import ratelimiter from "@fastify/rate-limit"; import fastify from "fastify"; +import { Redis } from "ioredis"; import { Knex } from "knex"; import { Logger } from "pino"; @@ -39,10 +40,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({ @@ -56,6 +58,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; diff --git a/backend/src/server/routes/v1/sso-router.ts b/backend/src/server/routes/v1/sso-router.ts index 9007ca828..a6c66ad60 100644 --- 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"; @@ -21,8 +22,22 @@ import { AuthMethod } from "@app/services/auth/auth-type"; 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 @@ -35,11 +50,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({ @@ -52,7 +71,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) { @@ -74,10 +93,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]; @@ -86,7 +109,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) { @@ -110,17 +133,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 }); @@ -141,17 +167,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: () => {} }); @@ -164,7 +197,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)}` @@ -184,15 +218,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: () => {} }); @@ -205,7 +248,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)}` @@ -225,16 +269,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: () => {} }); @@ -248,7 +301,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)}` diff --git a/backend/src/services/smtp/templates/organizationInvitation.handlebars b/backend/src/services/smtp/templates/organizationInvitation.handlebars index 3ee16ee37..c3ac9556d 100644 --- a/backend/src/services/smtp/templates/organizationInvitation.handlebars +++ b/backend/src/services/smtp/templates/organizationInvitation.handlebars @@ -8,9 +8,9 @@

Join your organization on Infisical

-

{{inviterFirstName}} ({{inviterUsername}}) has invited you to their Infisical organization — {{organizationName}}

- Join now +

{{inviterFirstName}} ({{inviterUsername}}) has invited you to their Infisical organization named {{organizationName}}

+ Click to join

What is Infisical?

Infisical is an easy-to-use end-to-end encrypted tool that enables developers to sync and manage their secrets and configs.

- \ No newline at end of file + diff --git a/backend/src/services/smtp/templates/workspaceInvitation.handlebars b/backend/src/services/smtp/templates/workspaceInvitation.handlebars index 39a9b74ba..b82b8b2c2 100644 --- a/backend/src/services/smtp/templates/workspaceInvitation.handlebars +++ b/backend/src/services/smtp/templates/workspaceInvitation.handlebars @@ -6,10 +6,10 @@

Join your team on Infisical

-

You have been invited to a new Infisical project — {{workspaceName}}

- Join now +

You have been invited to a new Infisical project named {{workspaceName}}

+ Click to join

What is Infisical?

Infisical is an easy-to-use end-to-end encrypted tool that enables developers to sync and manage their secrets and configs.

- \ No newline at end of file +