Skip to content

Commit 89dc9c8

Browse files
geroplona-agent
andcommitted
Add feature flag control for OAuth returnTo URL validation
- Add feature flag check in authenticator.authorize() method - Add validation in GenericAuthProvider.callback() for authFlow.returnTo - Only validate when getFeatureFlagEnforceAuthorizeStateValidation is true - Handle cases where user may not be available (new user flow) - Maintain backward compatibility with feature flag disabled by default Co-authored-by: Ona <no-reply@ona.com>
1 parent 89f028c commit 89dc9c8

File tree

2 files changed

+27
-7
lines changed

2 files changed

+27
-7
lines changed

components/server/src/auth/authenticator.ts

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import { AuthFlow, AuthProvider } from "./auth-provider";
1919
import { HostContextProvider } from "./host-context-provider";
2020
import { SignInJWT } from "./jwt";
2121
import { getReturnToParamWithSafeBaseDomain } from "../express-util";
22+
import { getFeatureFlagEnforceAuthorizeStateValidation } from "../util/featureflags";
2223

2324
@injectable()
2425
export class Authenticator {
@@ -240,12 +241,15 @@ export class Authenticator {
240241
return;
241242
}
242243

243-
// Validate returnTo URL
244-
const valid = validateAuthorizeReturnToUrl(returnTo, this.config, authProvider);
245-
if (!valid) {
246-
log.info(`Bad request: invalid returnTo URL.`, { "authorize-flow": true });
247-
res.redirect(this.getSorryUrl(`Bad request: invalid returnTo URL.`));
248-
return;
244+
// Validate returnTo URL if feature flag is enabled
245+
const enforceValidation = await getFeatureFlagEnforceAuthorizeStateValidation(user.id);
246+
if (enforceValidation) {
247+
const valid = validateAuthorizeReturnToUrl(returnTo, this.config, authProvider);
248+
if (!valid) {
249+
log.info(`Bad request: invalid returnTo URL.`, { "authorize-flow": true });
250+
res.redirect(this.getSorryUrl(`Bad request: invalid returnTo URL.`));
251+
return;
252+
}
249253
}
250254

251255
// For non-verified org auth provider, ensure user is an owner of the org

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ import {
2424
} from "../auth/errors";
2525
import { Config } from "../config";
2626
import { getRequestingClientInfo } from "../express-util";
27+
import { getFeatureFlagEnforceAuthorizeStateValidation } from "../util/featureflags";
28+
import { validateAuthorizeReturnToUrl } from "./authenticator";
2729
import { TokenProvider } from "../user/token-provider";
2830
import { UserAuthentication } from "../user/user-authentication";
2931
import { AuthProviderService } from "./auth-provider-service";
@@ -302,6 +304,21 @@ export abstract class GenericAuthProvider implements AuthProvider {
302304
return;
303305
}
304306

307+
// Validate returnTo URL if feature flag is enabled
308+
if (authFlow.returnTo) {
309+
// Check feature flag - use existing user if logged in, otherwise default to false for new users
310+
const userId = request.user?.id;
311+
const enforceValidation = await getFeatureFlagEnforceAuthorizeStateValidation(userId || "");
312+
if (enforceValidation) {
313+
const valid = validateAuthorizeReturnToUrl(authFlow.returnTo, this.config, this);
314+
if (!valid) {
315+
log.info(cxt, `(${strategyName}) Bad request: invalid returnTo URL.`, { clientInfo });
316+
response.redirect(this.getSorryUrl(`Bad request: invalid returnTo URL.`));
317+
return;
318+
}
319+
}
320+
}
321+
305322
if (!this.loginCompletionHandler.isBaseDomain(request)) {
306323
// For auth requests that are not targetting the base domain, we redirect to the base domain, so they come with our cookie.
307324
log.info(`(${strategyName}) Auth request on subdomain, redirecting to base domain`, { clientInfo });
@@ -321,7 +338,6 @@ export abstract class GenericAuthProvider implements AuthProvider {
321338
return;
322339
}
323340
}
324-
325341
// assert additional infomation is attached to current session
326342
if (!authFlow) {
327343
// The auth flow state info is missing in the session: count as client error

0 commit comments

Comments
 (0)