mirror of
https://github.com/andwati/zitadel.git
synced 2026-10-23 07:52:15 +00:00
fix(login): redirect to loginname instead of empty accounts page when org scope filters all sessions (#12346)
Closes #11914 # Which Problems Are Solved When an OIDC auth request includes an organization scope (`urn:zitadel:iam:org:id:{id}` or `urn:zitadel:iam:org:domain:primary:{domain}`), users with existing browser sessions from *other* organizations were shown an empty Account Selection page with no selectable accounts. The only option was to click "Add another account", adding a confusing and unnecessary extra step. This happened because the `/login` route checked `sessions.length` (all browser sessions, unfiltered) to decide whether to enter the "reuse existing session" branch. Inside that branch, `findValidSession` correctly filtered by organization and returned no match — but the fallback redirected to `/accounts`, which also filters by org and rendered empty. # How the Problems Are Solved Before deciding to redirect to `/accounts`, pre-filter sessions by the requested organization using the same logic already used by the accounts page and `findValidSession`. If no sessions are eligible for the target organization, redirect directly to `/loginname` instead. This applies to both the **default prompt** and **`prompt=select_account`** branches. For `select_account`, this matches the behavior of major OIDC providers like Google and Microsoft, which skip the account chooser and go straight to the login input when there are no sessions to select from. The OIDC spec describes `select_account` as enabling selection "amongst multiple accounts that they might have current sessions for" — showing an empty picker serves no purpose. --------- Co-authored-by: Copilot Autofix powered by AI <[email protected]>
This commit is contained in:
co-authored by
Copilot Autofix powered by AI
parent
b4e205c601
commit
f32acbfd24
@@ -179,3 +179,179 @@ describe("handleOIDCFlowInitiation — locale / cookie handling", () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("handleOIDCFlowInitiation — org-scoped session filtering", () => {
|
||||
let mockGetAuthRequest: ReturnType<typeof vi.fn>;
|
||||
let mockConstructUrl: ReturnType<typeof vi.fn>;
|
||||
let mockFindValidSession: ReturnType<typeof vi.fn>;
|
||||
|
||||
const orgSession = {
|
||||
id: "session-org",
|
||||
factors: { user: { id: "user1", organizationId: "111111", loginName: "[email protected]" } },
|
||||
};
|
||||
const otherOrgSession = {
|
||||
id: "session-other",
|
||||
factors: { user: { id: "user2", organizationId: "999999", loginName: "[email protected]" } },
|
||||
};
|
||||
|
||||
beforeEach(async () => {
|
||||
vi.clearAllMocks();
|
||||
vi.unstubAllEnvs();
|
||||
|
||||
const zitadel = await import("@/lib/zitadel");
|
||||
const serviceUrl = await import("@/lib/service-url");
|
||||
const session = await import("@/lib/session");
|
||||
const authUtils = await import("@/lib/auth-utils");
|
||||
|
||||
mockGetAuthRequest = vi.mocked(zitadel.getAuthRequest);
|
||||
mockConstructUrl = vi.mocked(serviceUrl.constructUrl);
|
||||
mockFindValidSession = vi.mocked(session.findValidSession);
|
||||
vi.mocked(authUtils.getValidLocaleFromUILocales).mockReturnValue(null);
|
||||
|
||||
mockConstructUrl.mockImplementation((_req: any, path: string) => {
|
||||
return new URL(`https://example.com${path}`);
|
||||
});
|
||||
|
||||
mockFindValidSession.mockResolvedValue(null);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllEnvs();
|
||||
});
|
||||
|
||||
test("should redirect to /loginname when org scope filters out all sessions (default prompt)", async () => {
|
||||
mockGetAuthRequest.mockResolvedValue({
|
||||
authRequest: {
|
||||
id: "abc123",
|
||||
uiLocales: [],
|
||||
scope: ["urn:zitadel:iam:org:id:111111"],
|
||||
prompt: [],
|
||||
loginHint: undefined,
|
||||
},
|
||||
});
|
||||
|
||||
const res = await handleOIDCFlowInitiation(
|
||||
makeBaseParams({
|
||||
sessions: [otherOrgSession] as any,
|
||||
sessionCookies: [{ id: "session-other", token: "tok" }],
|
||||
}),
|
||||
);
|
||||
|
||||
const location = res.headers.get("location") ?? "";
|
||||
expect(location).toContain("/loginname");
|
||||
expect(location).not.toContain("/accounts");
|
||||
});
|
||||
|
||||
test("should include submit=true when loginHint is set and org scope filters out all sessions", async () => {
|
||||
mockGetAuthRequest.mockResolvedValue({
|
||||
authRequest: {
|
||||
id: "abc123",
|
||||
uiLocales: [],
|
||||
scope: ["urn:zitadel:iam:org:id:111111"],
|
||||
prompt: [],
|
||||
loginHint: "[email protected]",
|
||||
},
|
||||
});
|
||||
|
||||
const res = await handleOIDCFlowInitiation(
|
||||
makeBaseParams({
|
||||
sessions: [otherOrgSession] as any,
|
||||
sessionCookies: [{ id: "session-other", token: "tok" }],
|
||||
}),
|
||||
);
|
||||
|
||||
const location = res.headers.get("location") ?? "";
|
||||
expect(location).toContain("/loginname");
|
||||
expect(location).toContain("loginName=user%40example.com");
|
||||
expect(location).toContain("submit=true");
|
||||
expect(location).not.toContain("/accounts");
|
||||
});
|
||||
|
||||
test("should redirect to /accounts when org-eligible sessions exist but none are valid (default prompt)", async () => {
|
||||
mockGetAuthRequest.mockResolvedValue({
|
||||
authRequest: {
|
||||
id: "abc123",
|
||||
uiLocales: [],
|
||||
scope: ["urn:zitadel:iam:org:id:111111"],
|
||||
prompt: [],
|
||||
loginHint: undefined,
|
||||
},
|
||||
});
|
||||
|
||||
const res = await handleOIDCFlowInitiation(
|
||||
makeBaseParams({
|
||||
sessions: [orgSession] as any,
|
||||
sessionCookies: [{ id: "session-org", token: "tok" }],
|
||||
}),
|
||||
);
|
||||
|
||||
const location = res.headers.get("location") ?? "";
|
||||
expect(location).toContain("/accounts");
|
||||
});
|
||||
|
||||
test("should redirect to /loginname when org scope filters out all sessions (SELECT_ACCOUNT prompt)", async () => {
|
||||
const { Prompt } = await import("@zitadel/proto/zitadel/oidc/v2/authorization_pb");
|
||||
|
||||
mockGetAuthRequest.mockResolvedValue({
|
||||
authRequest: {
|
||||
id: "abc123",
|
||||
uiLocales: [],
|
||||
scope: ["urn:zitadel:iam:org:id:111111"],
|
||||
prompt: [Prompt.SELECT_ACCOUNT],
|
||||
loginHint: undefined,
|
||||
},
|
||||
});
|
||||
|
||||
const res = await handleOIDCFlowInitiation(
|
||||
makeBaseParams({
|
||||
sessions: [otherOrgSession] as any,
|
||||
sessionCookies: [{ id: "session-other", token: "tok" }],
|
||||
}),
|
||||
);
|
||||
|
||||
const location = res.headers.get("location") ?? "";
|
||||
expect(location).toContain("/loginname");
|
||||
expect(location).not.toContain("/accounts");
|
||||
});
|
||||
|
||||
test("should redirect to /accounts when org-eligible sessions exist (SELECT_ACCOUNT prompt)", async () => {
|
||||
const { Prompt } = await import("@zitadel/proto/zitadel/oidc/v2/authorization_pb");
|
||||
|
||||
mockGetAuthRequest.mockResolvedValue({
|
||||
authRequest: {
|
||||
id: "abc123",
|
||||
uiLocales: [],
|
||||
scope: ["urn:zitadel:iam:org:id:111111"],
|
||||
prompt: [Prompt.SELECT_ACCOUNT],
|
||||
loginHint: undefined,
|
||||
},
|
||||
});
|
||||
|
||||
const res = await handleOIDCFlowInitiation(
|
||||
makeBaseParams({
|
||||
sessions: [orgSession] as any,
|
||||
sessionCookies: [{ id: "session-org", token: "tok" }],
|
||||
}),
|
||||
);
|
||||
|
||||
const location = res.headers.get("location") ?? "";
|
||||
expect(location).toContain("/accounts");
|
||||
});
|
||||
|
||||
test("should redirect to /loginname when no org scope and no sessions", async () => {
|
||||
mockGetAuthRequest.mockResolvedValue({
|
||||
authRequest: {
|
||||
id: "abc123",
|
||||
uiLocales: [],
|
||||
scope: [],
|
||||
prompt: [],
|
||||
loginHint: undefined,
|
||||
},
|
||||
});
|
||||
|
||||
const res = await handleOIDCFlowInitiation(makeBaseParams({ sessions: [] }));
|
||||
|
||||
const location = res.headers.get("location") ?? "";
|
||||
expect(location).toContain("/loginname");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -74,6 +74,47 @@ const gotoAccounts = ({
|
||||
return NextResponse.redirect(accountsUrl);
|
||||
};
|
||||
|
||||
const gotoLoginname = ({
|
||||
request,
|
||||
requestId,
|
||||
loginHint,
|
||||
organization,
|
||||
suffix,
|
||||
}: {
|
||||
request: NextRequest;
|
||||
requestId: string;
|
||||
loginHint?: string;
|
||||
organization?: string;
|
||||
suffix?: string;
|
||||
}): NextResponse<unknown> => {
|
||||
const loginNameUrl = constructUrl(request, "/loginname");
|
||||
loginNameUrl.searchParams.set("requestId", requestId);
|
||||
|
||||
if (loginHint) {
|
||||
loginNameUrl.searchParams.set("loginName", loginHint);
|
||||
loginNameUrl.searchParams.set("submit", "true");
|
||||
}
|
||||
if (organization) {
|
||||
loginNameUrl.searchParams.set("organization", organization);
|
||||
}
|
||||
if (suffix) {
|
||||
loginNameUrl.searchParams.set("suffix", suffix);
|
||||
}
|
||||
|
||||
return NextResponse.redirect(loginNameUrl);
|
||||
};
|
||||
|
||||
/**
|
||||
* Filter sessions by organization, matching the same logic used in
|
||||
* the accounts page and findValidSession.
|
||||
*/
|
||||
function getEligibleSessions(sessions: Session[], organization?: string): Session[] {
|
||||
if (!organization) {
|
||||
return sessions;
|
||||
}
|
||||
return sessions.filter((s) => s.factors?.user?.organizationId === organization);
|
||||
}
|
||||
|
||||
export interface FlowInitiationParams {
|
||||
serviceConfig: ServiceConfig;
|
||||
requestId: string;
|
||||
@@ -225,7 +266,20 @@ export async function handleOIDCFlowInitiation(params: FlowInitiationParams): Pr
|
||||
|
||||
// use existing session and hydrate it for oidc
|
||||
if (authRequest && sessions.length) {
|
||||
// Pre-filter sessions by organization so we don't show an empty accounts
|
||||
// page when the org scope excludes all existing browser sessions.
|
||||
const eligibleSessions = getEligibleSessions(sessions, organization);
|
||||
|
||||
if (authRequest.prompt.includes(Prompt.SELECT_ACCOUNT)) {
|
||||
if (eligibleSessions.length === 0) {
|
||||
return gotoLoginname({
|
||||
request,
|
||||
requestId: `oidc_${authRequest.id}`,
|
||||
loginHint: authRequest.loginHint,
|
||||
organization,
|
||||
suffix,
|
||||
});
|
||||
}
|
||||
return gotoAccounts({
|
||||
request,
|
||||
requestId: `oidc_${authRequest.id}`,
|
||||
@@ -318,6 +372,17 @@ export async function handleOIDCFlowInitiation(params: FlowInitiationParams): Pr
|
||||
let selectedSession = await findValidSession({ serviceConfig, sessions, authRequest, organization });
|
||||
|
||||
if (!selectedSession || !selectedSession.id) {
|
||||
// If no sessions are eligible for this organization, go directly to
|
||||
// loginname instead of showing an empty accounts page.
|
||||
if (eligibleSessions.length === 0) {
|
||||
return gotoLoginname({
|
||||
request,
|
||||
requestId: `oidc_${authRequest.id}`,
|
||||
loginHint: authRequest.loginHint,
|
||||
organization,
|
||||
suffix,
|
||||
});
|
||||
}
|
||||
return gotoAccounts({
|
||||
request,
|
||||
requestId: `oidc_${authRequest.id}`,
|
||||
@@ -384,11 +449,11 @@ export async function handleOIDCFlowInitiation(params: FlowInitiationParams): Pr
|
||||
}
|
||||
|
||||
if (organization) {
|
||||
loginNameUrl.searchParams.append("organization", organization);
|
||||
loginNameUrl.searchParams.set("organization", organization);
|
||||
}
|
||||
|
||||
if (suffix) {
|
||||
loginNameUrl.searchParams.append("suffix", suffix);
|
||||
loginNameUrl.searchParams.set("suffix", suffix);
|
||||
}
|
||||
|
||||
return NextResponse.redirect(loginNameUrl);
|
||||
|
||||
Reference in New Issue
Block a user