Skip to content

fix: resolve OIDC account linking and show SSO error messages #28

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
TerrifiedBug merged 2 commits into from
Mar 7, 2026
Merged

fix: resolve OIDC account linking and show SSO error messages #28

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
353c98a
chore: add .worktrees/ to gitignore
TerrifiedBug Mar 7, 2026
05e9050
fix: resolve OIDC account linking and show SSO error messages
TerrifiedBug Mar 7, 2026
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
4 changes: 3 additions & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,6 @@ next-env.d.ts
# codeql
.codeql/

/scripts/*
/scripts/*
# worktrees
.worktrees/
18 changes: 17 additions & 1 deletion src/app/(auth)/login/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import { useState, useEffect } from "react";
import { signIn } from "next-auth/react";
import { useRouter } from "next/navigation";
import { useRouter, useSearchParams } from "next/navigation";
import { Shield, KeyRound, Loader2 } from "lucide-react";
import {
Card,
Expand All @@ -17,14 +17,30 @@ import { Button } from "@/components/ui/button";
import { Label } from "@/components/ui/label";
import { Separator } from "@/components/ui/separator";

const SSO_ERROR_MESSAGES: Record<string, string> = {
local_account: "This email is registered as a local account. Ask an admin to link it to SSO before signing in.",
OAuthAccountNotLinked: "This email is already associated with another account. Ask an admin to link it to SSO.",
OAuthCallback: "SSO sign-in failed. Please try again or contact your administrator.",
AccessDenied: "Access denied. You may not have permission to sign in via SSO.",
};

export default function LoginPage() {
const router = useRouter();
const searchParams = useSearchParams();
const [email, setEmail] = useState("");
const [password, setPassword] = useState("");
const [totpCode, setTotpCode] = useState("");
const [totpRequired, setTotpRequired] = useState(false);
const [error, setError] = useState<string | null>(null);
const [loading, setLoading] = useState(false);

useEffect(() => {
const errorParam = searchParams.get("error");
if (errorParam) {
setError(SSO_ERROR_MESSAGES[errorParam] ?? "An error occurred during sign-in. Please try again.");
window.history.replaceState({}, "", "/login");
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 7, 2026

Choose a reason for hiding this comment

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

replaceState doesn't sync Next.js router state

window.history.replaceState updates the browser URL directly but does NOT update Next.js's internal router state. As a result, useSearchParams() may still return the old params (containing ?error=...) on subsequent renders, causing this useEffect to re-run and redundantly re-set the same error message.

The recommended pattern in Next.js App Router is to use router.replace('/login') instead, which flushes both the browser URL and Next.js's searchParams state:

Suggested change
window.history.replaceState({}, "", "/login");
router.replace("/login");

This ensures searchParams is cleanly emptied after the error is consumed, preventing any stale re-invocation of the effect.

Rule Used: ## Code Style & Conventions

TypeScript Conven... (source)

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/app/(auth)/login/page.tsx
Line: 41

Comment:
**`replaceState` doesn't sync Next.js router state**

`window.history.replaceState` updates the browser URL directly but does NOT update Next.js's internal router state. As a result, `useSearchParams()` may still return the old params (containing `?error=...`) on subsequent renders, causing this `useEffect` to re-run and redundantly re-set the same error message.

The recommended pattern in Next.js App Router is to use `router.replace('/login')` instead, which flushes both the browser URL and Next.js's `searchParams` state:

```suggestion
      router.replace("/login");
```

This ensures `searchParams` is cleanly emptied after the error is consumed, preventing any stale re-invocation of the effect.

**Rule Used:** ## Code Style & Conventions

### TypeScript Conven... ([source](https://app.greptile.com/review/custom-context?memory=6ae51394-d0b6-4686-bc4c-1ad840c2e310))

How can I resolve this? If you propose a fix, please make it concise.

}
}, [searchParams]);
const [oidcStatus, setOidcStatus] = useState<{
enabled: boolean;
displayName: string;
Expand Down
5 changes: 3 additions & 2 deletions src/auth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ async function getAuthInstance() {
issuer: oidc.issuer,
clientId: oidc.clientId,
clientSecret: oidc.clientSecret,
allowDangerousEmailAccountLinking: false,
allowDangerousEmailAccountLinking: true,
client: {
token_endpoint_auth_method: oidc.tokenEndpointAuthMethod,
},
Expand All @@ -201,6 +201,7 @@ async function getAuthInstance() {
const groupsClaim = settings?.oidcGroupsClaim ?? "groups";
const profileData = profile as Record<string, unknown> | undefined;
const userGroups = (profileData?.[groupsClaim] as string[] | undefined) ?? [];
console.log(`[oidc] User ${user.email} groups (claim "${groupsClaim}"):`, userGroups);
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 7, 2026

Choose a reason for hiding this comment

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

Permanent PII logging on every OIDC login

This line logs the user's email address and their full group membership list on every SSO login. These are server logs that may be collected by log aggregation pipelines, and group membership can reveal organizational structure and access levels.

The PR description calls this "debug logging," but it's unconditional and permanent — there's no debug flag guarding it. Consider either removing it after diagnosis or gating it on an environment variable:

Suggested change
console.log(`[oidc] User ${user.email} groups (claim "${groupsClaim}"):`, userGroups);
if (process.env.DEBUG_OIDC_GROUPS === "true") {
console.log(`[oidc] User ${user.email} groups (claim "${groupsClaim}"):`, userGroups);
}

Rule Used: ## Security & Cryptography Review Rules

When revi... (source)

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/auth.ts
Line: 204

Comment:
**Permanent PII logging on every OIDC login**

This line logs the user's email address and their full group membership list on every SSO login. These are server logs that may be collected by log aggregation pipelines, and group membership can reveal organizational structure and access levels.

The PR description calls this "debug logging," but it's unconditional and permanent — there's no debug flag guarding it. Consider either removing it after diagnosis or gating it on an environment variable:

```suggestion
              if (process.env.DEBUG_OIDC_GROUPS === "true") {
                console.log(`[oidc] User ${user.email} groups (claim "${groupsClaim}"):`, userGroups);
              }
```

**Rule Used:** ## Security & Cryptography Review Rules

When revi... ([source](https://app.greptile.com/review/custom-context?memory=7cb20c56-ca6a-40aa-8660-7fa75e6e3db2))

How can I resolve this? If you propose a fix, please make it concise.


// Ensure user exists in the database
let dbUser = await prisma.user.findUnique({
Expand Down Expand Up @@ -231,7 +232,7 @@ async function getAuthInstance() {
console.warn(
`OIDC login blocked: local account exists for ${dbUser.email}. Admin must explicitly link accounts.`,
);
return false;
return "/login?error=local_account";
}

// Refresh profile picture on each OIDC sign-in
Expand Down
Loading