Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion frontend/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ import { PostBody } from "./PostBody";
import { decodeHtmlEntities } from "./postBodyDisplay";
import { FiveW1H } from "./components/FiveW1H";
import { subgraphForPost } from "./lineageLayout";
import { rememberOidcReturnUrl, returnUrlFromLocation } from "./oidcReturnUrl";
import { rememberOidcReturnUrl, returnUrlFromLocation, stripOidcCallbackParams } from "./oidcReturnUrl";
import {
isSupportedLocale,
LOCALE_LABELS,
Expand Down Expand Up @@ -1829,6 +1829,7 @@ function PostDetailPopup({

const permanentLink = (() => {
const url = new URL(window.location.href);
stripOidcCallbackParams(url);
url.searchParams.set("post", postId);
url.hash = "";
return url.toString();
Expand Down
37 changes: 37 additions & 0 deletions frontend/src/oidcReturnUrl.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
rememberOidcReturnUrl,
restoreOidcReturnUrl,
returnUrlFromLocation,
stripOidcCallbackParams,
} from "./oidcReturnUrl";

describe("OIDC return URL handling", () => {
Expand All @@ -18,6 +19,17 @@ describe("OIDC return URL handling", () => {
expect(returnUrlFromLocation({ pathname: "//evil.example", search: "", hash: "" })).toBe("/");
});

it("strips OIDC callback params from a restored post-redirect location", () => {
// After Keycloak redirects back, window.location still carries the
// one-time code/state; a return URL built from it must not.
const cleaned = returnUrlFromLocation({
pathname: "/",
search: "?post=abc&code=xyz&state=s&session_state=t&iss=i",
hash: "",
});
expect(cleaned).toBe("/?post=abc");
});

it("restores an object or serialized OIDC state before storage fallback", () => {
rememberOidcReturnUrl("/?post=stored-before-direct");
expect(restoreOidcReturnUrl("/?post=from-direct-state")).toBe(
Expand Down Expand Up @@ -67,3 +79,28 @@ describe("OIDC return URL handling", () => {
expect(window.localStorage.getItem("lineageweave.oidc.returnUrl")).toBeNull();
});
});

describe("stripOidcCallbackParams", () => {
it("removes the Keycloak auth-exchange params but keeps app deep-link params", () => {
const url = new URL(
"http://localhost:15173/?state=abc&session_state=def&iss=http%3A%2F%2Fidp&code=xyz&post=post-1&workspace=board",
);

stripOidcCallbackParams(url);

expect(url.searchParams.get("state")).toBeNull();
expect(url.searchParams.get("session_state")).toBeNull();
expect(url.searchParams.get("iss")).toBeNull();
expect(url.searchParams.get("code")).toBeNull();
expect(url.searchParams.get("post")).toBe("post-1");
expect(url.searchParams.get("workspace")).toBe("board");
});

it("is a no-op when no OIDC params are present", () => {
const url = new URL("http://localhost:15173/?post=post-1");

stripOidcCallbackParams(url);

expect(url.toString()).toBe("http://localhost:15173/?post=post-1");
});
});
21 changes: 20 additions & 1 deletion frontend/src/oidcReturnUrl.ts
Comment thread
seonghobae marked this conversation as resolved.
Original file line number Diff line number Diff line change
@@ -1,6 +1,18 @@
export const OIDC_RETURN_URL_STORAGE_KEY = "lineageweave.oidc.returnUrl";
const MAX_OIDC_RETURN_URL_LENGTH = 4096;

/** Authorization-code response params Keycloak appends to the redirect URI
* (RFC 6749 sec. 4.1.2; `session_state` per OIDC Session Management). Any
* link built from `window.location` must strip these -- they're a one-time
* auth exchange, never part of a shareable URL. */
const OIDC_CALLBACK_PARAMS = ["code", "state", "session_state", "iss"] as const;

/** Removes OIDC callback artifacts from `url` in place -- call before turning
* `window.location` into a link a user can copy or share. */
export function stripOidcCallbackParams(url: URL): void {
OIDC_CALLBACK_PARAMS.forEach((param) => url.searchParams.delete(param));
}

type UrlLike = Pick<Location, "pathname" | "search" | "hash">;

function isSafeReturnUrl(value: string): boolean {
Expand All @@ -12,7 +24,14 @@ function isSafeReturnUrl(value: string): boolean {
}

export function returnUrlFromLocation(location: UrlLike = window.location): string {
const value = `${location.pathname}${location.search}${location.hash}`;
// A restored return URL can itself be a post-redirect URL still carrying
// Keycloak callback artifacts (devin review thread on PR #576): strip
// them here too, so no consumer of this module re-mints a URL with a
// one-time authorization code in it.
const params = new URLSearchParams(location.search);
OIDC_CALLBACK_PARAMS.forEach((param) => params.delete(param));
const cleanedSearch = params.toString();
const value = `${location.pathname}${cleanedSearch ? `?${cleanedSearch}` : ""}${location.hash}`;
Comment thread
seonghobae marked this conversation as resolved.
return isSafeReturnUrl(value) ? value : "/";
}

Expand Down
Loading