Remove login routes when federated#1697
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR adds conditional routing to remove authentication-related routes when federated login is enabled, controlled by the FEDERATED_LOGIN_ENABLED feature flag.
- Introduces feature flag check for
FEDERATED_LOGIN_ENABLEDto conditionally include routes - Restructures route definitions to separate regular login routes from always-available routes
- Wraps authentication routes (sign-in, sign-up, password reset, email verification) in a conditional block
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }, | ||
| ]; | ||
|
|
||
| if (!isFederatedLogin) { |
There was a problem hiding this comment.
The condition !isFederatedLogin will evaluate to true when isFederatedLogin is undefined, causing regular login routes to be included even when the flag state hasn't loaded yet. This could lead to race conditions. Consider adding an explicit check: if (isFederatedLogin === false) to only include routes when the flag is definitively disabled.
| if (!isFederatedLogin) { | |
| if (isFederatedLogin === false) { |
|
Greptile OverviewGreptile SummaryConditionally registers login-related routes ( Confidence Score: 4/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User
participant Router as ApplicationRouter
participant GlobalLayout
participant AuthGuard as AllowOnlyLoggedInUserOnlyGuard
participant Flag as flagsHooks
participant Frontegg
Note over Router,Flag: Federated Login Enabled
Router->>Flag: useFlag(FEDERATED_LOGIN_ENABLED)
Flag-->>Router: isFederatedLogin = true
Router->>Router: Skip registering /sign-in, /sign-up, etc.
User->>GlobalLayout: Navigate to /flows
GlobalLayout->>GlobalLayout: Check UNAUTHENTICATED_ROUTES
GlobalLayout->>AuthGuard: Path not in unauthenticated list
AuthGuard->>Flag: useFlag(FEDERATED_LOGIN_ENABLED)
Flag-->>AuthGuard: isFederatedLogin = true
AuthGuard->>Frontegg: Check fronteggToken
alt User authenticated
Frontegg-->>AuthGuard: Valid token
AuthGuard->>User: Render protected content
else User not authenticated
Frontegg-->>AuthGuard: No token
AuthGuard->>User: Redirect to Frontegg login
end
Note over Router,Flag: Traditional Login (Federated Disabled)
Router->>Flag: useFlag(FEDERATED_LOGIN_ENABLED)
Flag-->>Router: isFederatedLogin = false
Router->>Router: Register /sign-in, /sign-up, etc.
User->>GlobalLayout: Navigate to /sign-in
GlobalLayout->>GlobalLayout: Path in UNAUTHENTICATED_ROUTES
GlobalLayout->>User: Render SignInPage (no auth guard)
|
| if (!isFederatedLogin) { | ||
| const regularLoginRoutes = [ | ||
| { | ||
| path: 'forget-password', | ||
| element: ( | ||
| <OpsErrorBoundary> | ||
| <PageTitle title="Forget Password"> | ||
| <ResetPasswordPage /> | ||
| </PageTitle> | ||
| </OpsErrorBoundary> | ||
| ), | ||
| errorElement: <RouteErrorBoundary />, | ||
| }, | ||
| { | ||
| path: 'reset-password', | ||
| element: ( | ||
| <OpsErrorBoundary> | ||
| <PageTitle title="Reset Password"> | ||
| <ChangePasswordPage /> | ||
| </PageTitle> | ||
| </OpsErrorBoundary> | ||
| ), | ||
| errorElement: <RouteErrorBoundary />, | ||
| }, | ||
| { | ||
| path: 'sign-in', | ||
| element: ( | ||
| <OpsErrorBoundary> | ||
| <PageTitle title="Sign In"> | ||
| <SignInPage /> | ||
| </PageTitle> | ||
| </OpsErrorBoundary> | ||
| ), | ||
| errorElement: <RouteErrorBoundary />, | ||
| }, | ||
| { | ||
| path: 'verify-email', | ||
| element: ( | ||
| <OpsErrorBoundary> | ||
| <PageTitle title="Verify Email"> | ||
| <VerifyEmail /> | ||
| </PageTitle> | ||
| </OpsErrorBoundary> | ||
| ), | ||
| errorElement: <RouteErrorBoundary />, | ||
| }, | ||
| { | ||
| path: 'sign-up', | ||
| element: ( | ||
| <OpsErrorBoundary> | ||
| <PageTitle title="Sign Up"> | ||
| <SignUpPage /> | ||
| </PageTitle> | ||
| </OpsErrorBoundary> | ||
| ), | ||
| errorElement: <RouteErrorBoundary />, | ||
| }, | ||
| ]; | ||
| routes.push(...regularLoginRoutes); | ||
| } |
There was a problem hiding this comment.
logic: UNAUTHENTICATED_ROUTES in global-layout.tsx:37-48 still includes these routes. When federated login is enabled, users navigating to /sign-in, /sign-up, etc. will bypass auth guards but hit 404s.
| if (!isFederatedLogin) { | |
| const regularLoginRoutes = [ | |
| { | |
| path: 'forget-password', | |
| element: ( | |
| <OpsErrorBoundary> | |
| <PageTitle title="Forget Password"> | |
| <ResetPasswordPage /> | |
| </PageTitle> | |
| </OpsErrorBoundary> | |
| ), | |
| errorElement: <RouteErrorBoundary />, | |
| }, | |
| { | |
| path: 'reset-password', | |
| element: ( | |
| <OpsErrorBoundary> | |
| <PageTitle title="Reset Password"> | |
| <ChangePasswordPage /> | |
| </PageTitle> | |
| </OpsErrorBoundary> | |
| ), | |
| errorElement: <RouteErrorBoundary />, | |
| }, | |
| { | |
| path: 'sign-in', | |
| element: ( | |
| <OpsErrorBoundary> | |
| <PageTitle title="Sign In"> | |
| <SignInPage /> | |
| </PageTitle> | |
| </OpsErrorBoundary> | |
| ), | |
| errorElement: <RouteErrorBoundary />, | |
| }, | |
| { | |
| path: 'verify-email', | |
| element: ( | |
| <OpsErrorBoundary> | |
| <PageTitle title="Verify Email"> | |
| <VerifyEmail /> | |
| </PageTitle> | |
| </OpsErrorBoundary> | |
| ), | |
| errorElement: <RouteErrorBoundary />, | |
| }, | |
| { | |
| path: 'sign-up', | |
| element: ( | |
| <OpsErrorBoundary> | |
| <PageTitle title="Sign Up"> | |
| <SignUpPage /> | |
| </PageTitle> | |
| </OpsErrorBoundary> | |
| ), | |
| errorElement: <RouteErrorBoundary />, | |
| }, | |
| ]; | |
| routes.push(...regularLoginRoutes); | |
| } | |
| if (!isFederatedLogin) { | |
| const regularLoginRoutes = [ | |
| { | |
| path: 'forget-password', | |
| element: ( | |
| <OpsErrorBoundary> | |
| <PageTitle title="Forget Password"> | |
| <ResetPasswordPage /> | |
| </PageTitle> | |
| </OpsErrorBoundary> | |
| ), | |
| errorElement: <RouteErrorBoundary />, | |
| }, | |
| { | |
| path: 'reset-password', | |
| element: ( | |
| <OpsErrorBoundary> | |
| <PageTitle title="Reset Password"> | |
| <ChangePasswordPage /> | |
| </PageTitle> | |
| </OpsErrorBoundary> | |
| ), | |
| errorElement: <RouteErrorBoundary />, | |
| }, | |
| { | |
| path: 'sign-in', | |
| element: ( | |
| <OpsErrorBoundary> | |
| <PageTitle title="Sign In"> | |
| <SignInPage /> | |
| </PageTitle> | |
| </OpsErrorBoundary> | |
| ), | |
| errorElement: <RouteErrorBoundary />, | |
| }, | |
| { | |
| path: 'verify-email', | |
| element: ( | |
| <OpsErrorBoundary> | |
| <PageTitle title="Verify Email"> | |
| <VerifyEmail /> | |
| </PageTitle> | |
| </OpsErrorBoundary> | |
| ), | |
| errorElement: <RouteErrorBoundary />, | |
| }, | |
| { | |
| path: 'sign-up', | |
| element: ( | |
| <OpsErrorBoundary> | |
| <PageTitle title="Sign Up"> | |
| <SignUpPage /> | |
| </PageTitle> | |
| </OpsErrorBoundary> | |
| ), | |
| errorElement: <RouteErrorBoundary />, | |
| }, | |
| ]; | |
| routes.push(...regularLoginRoutes); | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/react-ui/src/app/router.tsx
Line: 232:291
Comment:
**logic:** `UNAUTHENTICATED_ROUTES` in `global-layout.tsx:37-48` still includes these routes. When federated login is enabled, users navigating to `/sign-in`, `/sign-up`, etc. will bypass auth guards but hit 404s.
```suggestion
if (!isFederatedLogin) {
const regularLoginRoutes = [
{
path: 'forget-password',
element: (
<OpsErrorBoundary>
<PageTitle title="Forget Password">
<ResetPasswordPage />
</PageTitle>
</OpsErrorBoundary>
),
errorElement: <RouteErrorBoundary />,
},
{
path: 'reset-password',
element: (
<OpsErrorBoundary>
<PageTitle title="Reset Password">
<ChangePasswordPage />
</PageTitle>
</OpsErrorBoundary>
),
errorElement: <RouteErrorBoundary />,
},
{
path: 'sign-in',
element: (
<OpsErrorBoundary>
<PageTitle title="Sign In">
<SignInPage />
</PageTitle>
</OpsErrorBoundary>
),
errorElement: <RouteErrorBoundary />,
},
{
path: 'verify-email',
element: (
<OpsErrorBoundary>
<PageTitle title="Verify Email">
<VerifyEmail />
</PageTitle>
</OpsErrorBoundary>
),
errorElement: <RouteErrorBoundary />,
},
{
path: 'sign-up',
element: (
<OpsErrorBoundary>
<PageTitle title="Sign Up">
<SignUpPage />
</PageTitle>
</OpsErrorBoundary>
),
errorElement: <RouteErrorBoundary />,
},
];
routes.push(...regularLoginRoutes);
}
```
How can I resolve this? If you propose a fix, please make it concise.


Fixes OPS-3163