Skip to content

Remove login routes when federated #1697

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
alexandrudanpop merged 1 commit into from
Dec 2, 2025
Merged

Remove login routes when federated #1697

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
124 changes: 69 additions & 55 deletions packages/react-ui/src/app/router.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ const createRoutes = () => {
FlagId.SHOW_DEMO_HOME_PAGE,
);

const { data: isFederatedLogin } = flagsHooks.useFlag<boolean | undefined>(
Comment thread
alexandrudanpop marked this conversation as resolved.
FlagId.FEDERATED_LOGIN_ENABLED,
);

const routes = [
{
path: 'flows',
Expand Down Expand Up @@ -140,61 +144,6 @@ const createRoutes = () => {
),
errorElement: <RouteErrorBoundary />,
},
{
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 />,
},
{
path: 'settings/appearance',
element: (
Expand Down Expand Up @@ -278,6 +227,70 @@ const createRoutes = () => {
),
errorElement: <RouteErrorBoundary />,
},
];

if (!isFederatedLogin) {
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
if (!isFederatedLogin) {
if (isFederatedLogin === false) {

Copilot uses AI. Check for mistakes.
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);
}
Comment on lines +232 to +291
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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.


const redirectRoutes = [
{
path: 'redirect',
element: <RedirectPage></RedirectPage>,
Expand All @@ -295,6 +308,7 @@ const createRoutes = () => {
errorElement: <RouteErrorBoundary />,
},
];
routes.push(...redirectRoutes);

if (isCloudConnectionPageEnabled) {
const CloudConnectionPage = lazy(
Expand Down
Loading