From 3216e7c781a9bb6399d33255e6b10275b3cc81f9 Mon Sep 17 00:00:00 2001 From: Jon Koops Date: Tue, 16 Apr 2024 17:24:22 +0200 Subject: [PATCH] Only allow a known refferer URI for the Account Console (#28743) Closes #27628 Signed-off-by: Jon Koops --- js/apps/account-ui/pom.xml | 4 +- js/apps/account-ui/src/constants.ts | 2 + js/apps/account-ui/src/environment.ts | 16 ++--- js/apps/account-ui/src/root/Header.tsx | 18 +++-- js/apps/account-ui/test/referrer.spec.ts | 67 +++++++++---------- .../resources/account/AccountConsole.java | 41 +++++++----- 6 files changed, 73 insertions(+), 75 deletions(-) diff --git a/js/apps/account-ui/pom.xml b/js/apps/account-ui/pom.xml index fbc8f53c65..655e05e368 100644 --- a/js/apps/account-ui/pom.xml +++ b/js/apps/account-ui/pom.xml @@ -142,7 +142,9 @@ "updateEmailFeatureEnabled": ${updateEmailFeatureEnabled?c}, "updateEmailActionEnabled": ${updateEmailActionEnabled?c}, "isViewGroupsEnabled": ${isViewGroupsEnabled?c} - } + }, + "referrerName": "${referrerName!""}", + "referrerUrl": "${referrer_uri!""}" } diff --git a/js/apps/account-ui/src/constants.ts b/js/apps/account-ui/src/constants.ts index 25a48547d3..fbbb676694 100644 --- a/js/apps/account-ui/src/constants.ts +++ b/js/apps/account-ui/src/constants.ts @@ -1,2 +1,4 @@ export const DEFAULT_REALM = "master"; export const ROOT_PATH = "/realms/:realm/account"; +export const ADMIN_USER = "admin"; +export const ADMIN_PASSWORD = "admin"; diff --git a/js/apps/account-ui/src/environment.ts b/js/apps/account-ui/src/environment.ts index eaaf00c720..89770744d1 100644 --- a/js/apps/account-ui/src/environment.ts +++ b/js/apps/account-ui/src/environment.ts @@ -34,10 +34,10 @@ export type Environment = { locale: string; /** Feature flags */ features: Feature; - /** Client id of the application to add back link */ - referrer?: string; - /** URI to the referrer application in the back link */ - referrer_uri?: string; + /** Name of the referrer application in the back link */ + referrerName?: string; + /** UR to the referrer application in the back link */ + referrerUrl?: string; }; // Detect the current realm from the URL. @@ -94,14 +94,6 @@ function getInjectedEnvironment(): Record { console.error("Unable to parse environment variables."); } - const searchParams = new URLSearchParams(location.search); - if (searchParams.has("referrer_uri")) { - env["referrer_uri"] = searchParams.get("referrer_uri")!; - } - if (searchParams.has("referrer")) { - env["referrer"] = searchParams.get("referrer")!; - } - // Otherwise, return an empty record. return env; } diff --git a/js/apps/account-ui/src/root/Header.tsx b/js/apps/account-ui/src/root/Header.tsx index bfd9ad1c36..5abde32ab9 100644 --- a/js/apps/account-ui/src/root/Header.tsx +++ b/js/apps/account-ui/src/root/Header.tsx @@ -1,3 +1,5 @@ +import { Button } from "@patternfly/react-core"; +import { ExternalLinkSquareAltIcon } from "@patternfly/react-icons"; import { KeycloakMasthead, KeycloakProvider, @@ -7,28 +9,30 @@ import { import { useMemo } from "react"; import { useTranslation } from "react-i18next"; import { useHref } from "react-router-dom"; -import { useEnvironment } from "./KeycloakContext"; +import { label } from "ui-shared"; + +import { environment } from "../environment"; import { joinPath } from "../utils/joinPath"; -import { ExternalLinkSquareAltIcon } from "@patternfly/react-icons"; -import { Button } from "@patternfly/react-core"; +import { useEnvironment } from "./KeycloakContext"; import style from "./header.module.css"; -import { environment } from "../environment"; const ReferrerLink = () => { const { t } = useTranslation(); - return environment.referrer_uri ? ( + return environment.referrerUrl ? ( ) : null; }; diff --git a/js/apps/account-ui/test/referrer.spec.ts b/js/apps/account-ui/test/referrer.spec.ts index 88aae7376f..d99560497a 100644 --- a/js/apps/account-ui/test/referrer.spec.ts +++ b/js/apps/account-ui/test/referrer.spec.ts @@ -1,53 +1,46 @@ import { expect, test } from "@playwright/test"; import { login } from "./login"; +import { getAdminUrl } from "./utils"; +import { ADMIN_PASSWORD, ADMIN_USER, DEFAULT_REALM } from "../src/constants"; + +// NOTE: This test suite will only pass when running a production build, as the referrer is extracted on the server side. +// This will change once https://github.com/keycloak/keycloak/pull/27311 has been merged. test.describe("Signing in with referrer link", () => { - // Tests for keycloak account console, section Signing in in Account security - test("Should see referrer", async ({ page }) => { - const queryParams = { - referrer: "my-app", - referrer_uri: "http://localhost:3000", - }; - await login(page, "jdoe", "jdoe", "groups", queryParams); + test("shows a referrer link when a matching client exists", async ({ + page, + }) => { + const referrer = "security-admin-console"; + const referrerUrl = getAdminUrl(); + const referrerName = "security admin console"; - await expect(page.getByTestId("referrer-link")).toContainText("my-app"); + const queryParams = { + referrer, + referrer_uri: referrerUrl, + }; + + await login(page, ADMIN_USER, ADMIN_PASSWORD, DEFAULT_REALM, queryParams); + await expect(page.getByTestId("referrer-link")).toContainText(referrerName); + + // Navigate around to ensure the referrer is still shown. await page.getByTestId("accountSecurity").click(); await expect(page.getByTestId("account-security/signing-in")).toBeVisible(); - await expect(page.getByTestId("referrer-link")).toContainText("my-app"); + await expect(page.getByTestId("referrer-link")).toContainText(referrerName); }); - // Tests for keycloak account console, section Signing in in Account security - test("Should see no referrer", async ({ page }) => { - const queryParams = {}; - await login(page, "jdoe", "jdoe", "groups", queryParams); + test("shows no referrer link when an invalid URL is passed", async ({ + page, + }) => { + const referrer = "security-admin-console"; + const referrerUrl = "http://i-am-not-an-allowed-url.com"; - await expect(page.getByTestId("referrer-link")).toBeHidden(); - await page.getByTestId("accountSecurity").click(); - await expect(page.getByTestId("account-security/signing-in")).toBeVisible(); - await expect(page.getByTestId("referrer-link")).toBeHidden(); - }); - - test("Should see no referrer after relogin", async ({ page }) => { const queryParams = { - referrer: "my-app", - referrer_uri: "http://localhost:3000", + referrer, + referrer_uri: referrerUrl, }; - await login(page, "jdoe", "jdoe", "groups", queryParams); - await expect(page.getByTestId("referrer-link")).toContainText("my-app"); - await page.getByTestId("accountSecurity").click(); - await expect(page.getByTestId("account-security/signing-in")).toBeVisible(); - await expect(page.getByTestId("referrer-link")).toContainText("my-app"); - - await page.getByTestId("options").click(); - await page.getByRole("menuitem", { name: "Sign out" }).click(); - - const queryParamsNoReferrer = {}; - await login(page, "jdoe", "jdoe", "groups", queryParamsNoReferrer); - - await expect(page.getByTestId("referrer-link")).toBeHidden(); - await page.getByTestId("accountSecurity").click(); - await expect(page.getByTestId("account-security/signing-in")).toBeVisible(); + await login(page, ADMIN_USER, ADMIN_PASSWORD, DEFAULT_REALM, queryParams); + await expect(page.getByText("Manage your basic information")).toBeVisible(); await expect(page.getByTestId("referrer-link")).toBeHidden(); }); }); diff --git a/services/src/main/java/org/keycloak/services/resources/account/AccountConsole.java b/services/src/main/java/org/keycloak/services/resources/account/AccountConsole.java index f028578b64..39fa790461 100644 --- a/services/src/main/java/org/keycloak/services/resources/account/AccountConsole.java +++ b/services/src/main/java/org/keycloak/services/resources/account/AccountConsole.java @@ -201,40 +201,45 @@ public class AccountConsole implements AccountResourceProvider { return propertyValue; } - + @GET @Path("index.html") public Response getIndexHtmlRedirect() { return Response.status(302).location(session.getContext().getUri().getRequestUriBuilder().path("../").build()).build(); } - private String[] getReferrer() { String referrer = session.getContext().getUri().getQueryParameters().getFirst("referrer"); + if (referrer == null) { return null; } + ClientModel referrerClient = realm.getClientByClientId(referrer); + + if (referrerClient == null) { + return null; + } + String referrerUri = session.getContext().getUri().getQueryParameters().getFirst("referrer_uri"); - ClientModel referrerClient = realm.getClientByClientId(referrer); - if (referrerClient != null) { - if (referrerUri != null) { - referrerUri = RedirectUtils.verifyRedirectUri(session, referrerUri, referrerClient); - } else { - referrerUri = ResolveRelative.resolveRelativeUri(session, referrerClient.getRootUrl(), referrerClient.getBaseUrl()); - } - - if (referrerUri != null) { - String referrerName = referrerClient.getName(); - if (Validation.isBlank(referrerName)) { - referrerName = referrer; - } - return new String[]{referrer, referrerName, referrerUri}; - } + if (referrerUri != null) { + referrerUri = RedirectUtils.verifyRedirectUri(session, referrerUri, referrerClient); + } else { + referrerUri = ResolveRelative.resolveRelativeUri(session, referrerClient.getRootUrl(), referrerClient.getBaseUrl()); } - return null; + if (referrerUri == null) { + return null; + } + + String referrerName = referrerClient.getName(); + + if (Validation.isBlank(referrerName)) { + referrerName = referrer; + } + + return new String[]{referrer, referrerName, referrerUri}; } }