[server] Enforce state presence and validation on the /api/authorize endoint

Author: geropl

components/server/src/auth/authenticator.ts

@@ -18,6 +18,7 @@ import { UserService } from "../user/user-service";
 import { AuthFlow, AuthProvider } from "./auth-provider";
 import { HostContextProvider } from "./host-context-provider";
 import { SignInJWT } from "./jwt";
+import { getFeatureFlagEnforceAuthorizeStateValidation } from "../util/featureflags";
 
 @injectable()
 export class Authenticator {
@@ -121,6 +122,60 @@ export class Authenticator {
         return state;
     }
 
+    private async validateOAuthState(
+        userId: string,
+        stateParam: string | undefined,
+        host: string | undefined,
+        returnTo: string | undefined,
+    ): Promise<boolean> {
+        // Check feature flag for OAuth state validation
+        const enforceStateValidation = await getFeatureFlagEnforceAuthorizeStateValidation(userId);
+
+        if (!enforceStateValidation) {
+            log.info(`OAuth state validation disabled by feature flag`, {
+                host,
+                returnTo,
+                "authorize-flow": true,
+            });
+            return true; // Allow request to proceed
+        }
+
+        // Validate JWT state parameter when feature flag is enabled
+        if (!stateParam) {
+            log.warn(`Missing state parameter in authorize request`, {
+                host,
+                returnTo,
+                "authorize-flow": true,
+            });
+            return false;
+        }
+
+        try {
+            const flowState = await this.parseState(stateParam);
+
+            if (flowState.host !== host || flowState.returnTo !== returnTo) {
+                log.warn(`State parameter mismatch in authorize request`, {
+                    stateHost: flowState.host,
+                    requestHost: host,
+                    stateReturnTo: flowState.returnTo,
+                    requestReturnTo: returnTo,
+                    "authorize-flow": true,
+                });
+                return false;
+            }
+
+            log.info(`Valid state parameter in authorize request`, { host, "authorize-flow": true });
+            return true; // Validation passed
+        } catch (error) {
+            log.warn(`Invalid state parameter in authorize request`, error, {
+                host,
+                returnTo,
+                "authorize-flow": true,
+            });
+            return false;
+        }
+    }
+
     protected async getAuthProviderForHost(host: string): Promise<AuthProvider | undefined> {
         const hostContext = this.hostContextProvider.get(host);
         return hostContext && hostContext.authProvider;
@@ -232,6 +287,15 @@ export class Authenticator {
         const host: string | undefined = req.query.host?.toString();
         const scopes: string = req.query.scopes?.toString() || "";
         const override = req.query.override === "true";
+        const stateParam = req.query.state?.toString();
+
+        // Validate OAuth state parameter (feature-flagged)
+        const isStateValid = await this.validateOAuthState(user.id, stateParam, host, returnTo);
+        if (!isStateValid) {
+            res.redirect(this.getSorryUrl(`Invalid authorization request.`));
+            return;
+        }
+
         const authProvider = host && (await this.getAuthProviderForHost(host));
         if (!returnTo || !host || !authProvider) {
             log.info(`Bad request: missing parameters.`, { "authorize-flow": true });

components/server/src/auth/generic-auth-provider.ts

@@ -460,6 +460,7 @@ export abstract class GenericAuthProvider implements AuthProvider {
                     user: newUser,
                     returnToUrl: authFlow.returnTo,
                     authHost: authFlow.host,
+                    originalState: `${state}`,
                 });
             } else {
                 const { user, elevateScopes } = flowContext as VerifyResult.WithUser;
@@ -492,6 +493,7 @@ export abstract class GenericAuthProvider implements AuthProvider {
                         returnToUrl: returnTo,
                         authHost: host,
                         elevateScopes,
+                        originalState: `${state}`,
                     });
                 }
             }

components/server/src/auth/login-completion-handler.ts

@@ -35,7 +35,7 @@ export class LoginCompletionHandler {
     async complete(
         request: express.Request,
         response: express.Response,
-        { user, returnToUrl, authHost, elevateScopes }: LoginCompletionHandler.CompleteParams,
+        { user, returnToUrl, authHost, elevateScopes, originalState }: LoginCompletionHandler.CompleteParams,
     ) {
         const logContext = LogContext.from({ user, request });
 
@@ -64,7 +64,7 @@ export class LoginCompletionHandler {
                     pathname: "/authorize",
                     search: `returnTo=${encodeURIComponent(returnTo)}&host=${authHost}&scopes=${elevateScopes.join(
                         ",",
-                    )}`,
+                    )}&state=${encodeURIComponent(originalState)}`,
                 })
                 .toString();
             returnTo = elevateScopesUrl;
@@ -137,5 +137,6 @@ export namespace LoginCompletionHandler {
         returnToUrl?: string;
         authHost?: string;
         elevateScopes?: string[];
+        originalState: string;
     }
 }

components/server/src/util/featureflags.ts

@@ -11,3 +11,9 @@ export async function getFeatureFlagEnableExperimentalJBTB(userId: string): Prom
         user: { id: userId },
     });
 }
+
+export async function getFeatureFlagEnforceAuthorizeStateValidation(userId: string): Promise<boolean> {
+    return getExperimentsClientForBackend().getValueAsync("enforceAuthorizeStateValidation", false, {
+        user: { id: userId },
+    });
+}