From 4d803b5c975ee93f30fbdd964462a9bfff00f364 Mon Sep 17 00:00:00 2001 From: Jonathan Payne Date: Wed, 30 Sep 2026 12:26:23 -0400 Subject: [PATCH 1/5] OpenConceptLab/ocl_issues#2856 | Signing in from a page with a query string no longer fails with "Incorrect redirect_uri" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Visitors from openconceptlab.org land on #/?referrer=…, and Sign in used the page URL, query string included, as the OIDC redirect_uri. The callback rebuilt the redirect_uri for the code exchange from the path alone, so Keycloak refused the exchange. Retrying from the callback page then carried its stale state and code into the next sign-in. Sign-in now always uses LOGIN_REDIRECT_URL, and the page to return to (its hash route) waits in this tab's sessionStorage, as the community site already does. The callback still honors next for sign-ins started before this deploy. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/common/utils.js | 22 +++++++++++++++++++--- src/components/users/OIDLoginCallback.jsx | 9 +++++++-- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/common/utils.js b/src/common/utils.js index 0537329bf..c32a0e6a0 100644 --- a/src/common/utils.js +++ b/src/common/utils.js @@ -836,6 +836,7 @@ export const isDeprecatedBrowser = () => isIE() || isOpera(); const PKCE_CODE_VERIFIER_KEY = 'pkce_code_verifier' const OAUTH_STATE_KEY = 'oauth_state' +const OAUTH_RETURN_TO_KEY = 'oauth_return_to' const base64UrlEncode = buffer => { const bytes = new Uint8Array(buffer) @@ -892,15 +893,30 @@ export const consumeAndValidateOAuthState = returnedState => { return !returnedState || returnedState === storedState } +// Keycloak only redeems a code when the token request repeats the sign-in's redirect_uri exactly, and the +// callback can't rebuild a page's query string (e.g. ?referrer= on links from openconceptlab.org). So sign-in +// always goes through LOGIN_REDIRECT_URL, and the page to come back to (its hash route) waits here, in this tab. +const prepareOAuthReturnTo = returnTo => { + const route = returnTo?.includes('#') ? returnTo.slice(returnTo.indexOf('#') + 1) : null + if(route?.startsWith('/') && !route.startsWith('/oidc/login')) + sessionStorage.setItem(OAUTH_RETURN_TO_KEY, route) + else + sessionStorage.removeItem(OAUTH_RETURN_TO_KEY) +} + +export const consumeOAuthReturnTo = () => { + const route = sessionStorage.getItem(OAUTH_RETURN_TO_KEY) + sessionStorage.removeItem(OAUTH_RETURN_TO_KEY) + return route +} + export const getLoginURL = async returnTo => { const oidClientID = window.OIDC_RP_CLIENT_ID || process.env.OIDC_RP_CLIENT_ID let redirectURL = window.LOGIN_REDIRECT_URL || process.env.LOGIN_REDIRECT_URL redirectURL = redirectURL.replace(/([^:]\/)\/+/g, "$1"); - if(returnTo && returnTo.includes('/#/') && returnTo.split('/#/')[1]) - redirectURL = returnTo.replace('/#/', '/') - + prepareOAuthReturnTo(returnTo) const codeChallenge = await preparePKCECodeChallenge() const state = prepareOAuthState() const nonce = generateSecureRandomString(32) diff --git a/src/components/users/OIDLoginCallback.jsx b/src/components/users/OIDLoginCallback.jsx index 9f58ed7f3..7f243dbdc 100644 --- a/src/components/users/OIDLoginCallback.jsx +++ b/src/components/users/OIDLoginCallback.jsx @@ -3,7 +3,7 @@ import React from 'react'; import { withTranslation } from 'react-i18next'; import Button from '@mui/material/Button'; import { - refreshCurrentUserCache, consumeStoredPKCECodeVerifier, consumeAndValidateOAuthState, + refreshCurrentUserCache, consumeStoredPKCECodeVerifier, consumeAndValidateOAuthState, consumeOAuthReturnTo, isSignupOAuthState, isLoggedIn, getLoginURL } from '../../common/utils'; import APIService from '../../services/APIService' @@ -16,6 +16,7 @@ class OIDLoginCallback extends React.Component { super(props) this.state = { next: null, + returnTo: null, } } componentDidMount() { @@ -32,12 +33,14 @@ class OIDLoginCallback extends React.Component { const { setAlert } = this.context const isStateValid = consumeAndValidateOAuthState(state) const codeVerifier = consumeStoredPKCECodeVerifier() + const returnTo = consumeOAuthReturnTo() if(!isStateValid || !codeVerifier) { this.onSignInStartedElsewhere(state, next) return } setAlert({message: this.props.t('auth.signing_in'), severity: 'info'}) - this.setState({next: next && next !== '/' ? next : null }, () => { + // next is only set by sign-ins started before redirect_uri became fixed; new ones come back via returnTo. + this.setState({next: next && next !== '/' ? next : null, returnTo: returnTo }, () => { const redirectURL = this.state.next ? window.location.origin + this.state.next : (window.LOGIN_REDIRECT_URL || process.env.LOGIN_REDIRECT_URL) const clientId = window.OIDC_RP_CLIENT_ID || process.env.OIDC_RP_CLIENT_ID @@ -88,6 +91,8 @@ class OIDLoginCallback extends React.Component { refreshCurrentUserCache(() => { if(this.state.next) window.location.hash = '#' + this.state.next + else if(this.state.returnTo) + window.location.hash = '#' + this.state.returnTo else { let returnToURL = '/' if(this.props?.location?.search) { From 8035c1eab406df060cb84b2f32771cb01a119146 Mon Sep 17 00:00:00 2001 From: Jonathan Payne Date: Wed, 30 Sep 2026 12:29:32 -0400 Subject: [PATCH 2/5] closes OpenConceptLab/ocl_issues#2856 | Opening the menu no longer blanks the app for people who follow a user with a profile picture UserIcon spread its sx prop into the style. LeftMenu passes sx as an array for followed items, so the image got style {0: {...}}, React threw setting style[0], and the whole app unmounted. Only a plain-object sx is used as the image style now. The MUI icons get sx unchanged, so an array sx also keeps its color there. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/components/users/UserIcon.jsx | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/components/users/UserIcon.jsx b/src/components/users/UserIcon.jsx index 65b5d3e97..9cb9b6787 100644 --- a/src/components/users/UserIcon.jsx +++ b/src/components/users/UserIcon.jsx @@ -1,21 +1,24 @@ import React from 'react'; +import isPlainObject from 'lodash/isPlainObject' import PersonIcon from '@mui/icons-material/Face2'; import StrangerIcon from '@mui/icons-material/Person'; import { isLoggedIn } from '../../common/utils'; import UserTooltip from './UserTooltip' const UserIcon = ({ user, color, logoClassName, sx, authenticated, noTooltip }) => { - const iconStyle = {...(sx || {})} + // sx may be an array (LeftMenu passes one for followed items), and spreading that into an style + // blanks the app. Only a plain object doubles as the image's style; the MUI icons take sx as is. + const imgStyle = isPlainObject(sx) ? sx : undefined return noTooltip ? ( user?.logo_url ? : (authenticated || isLoggedIn()) ? - : - + : + ) : ( { @@ -23,11 +26,11 @@ const UserIcon = ({ user, color, logoClassName, sx, authenticated, noTooltip }) : (authenticated || isLoggedIn()) ? - : - + : + } ) From c6924a6e69483fa947666eab0ce7b72824829a13 Mon Sep 17 00:00:00 2001 From: Jonathan Payne Date: Wed, 30 Sep 2026 12:41:07 -0400 Subject: [PATCH 3/5] OpenConceptLab/ocl_issues#2856 | The page saved at sign-in wins over next; sign-up and reset clear it; /signin and /signup aren't return pages From the Codex review of #58: - The page saved at sign-in now wins over next, which the callback rewrite also sets when LOGIN_REDIRECT_URL isn't the site root. - Sign-up and password reset clear a page left by an abandoned sign-in in the same tab, so they land on the dashboard as before. - /signin and /signup are refused as return pages, like /oidc/login, since landing on them starts another round trip. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/common/utils.js | 4 +++- src/components/users/OIDLoginCallback.jsx | 8 ++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/common/utils.js b/src/common/utils.js index c32a0e6a0..81d14bb2c 100644 --- a/src/common/utils.js +++ b/src/common/utils.js @@ -898,7 +898,7 @@ export const consumeAndValidateOAuthState = returnedState => { // always goes through LOGIN_REDIRECT_URL, and the page to come back to (its hash route) waits here, in this tab. const prepareOAuthReturnTo = returnTo => { const route = returnTo?.includes('#') ? returnTo.slice(returnTo.indexOf('#') + 1) : null - if(route?.startsWith('/') && !route.startsWith('/oidc/login')) + if(route?.startsWith('/') && !/^\/(oidc\/login|signin|signup)(\/|\?|$)/.test(route)) sessionStorage.setItem(OAUTH_RETURN_TO_KEY, route) else sessionStorage.removeItem(OAUTH_RETURN_TO_KEY) @@ -932,6 +932,7 @@ export const getResetPasswordURL = async returnTo => { redirectURL = redirectURL.replace(/([^:]\/)\/+/g, "$1"); + prepareOAuthReturnTo() const codeChallenge = await preparePKCECodeChallenge() return `${getAPIURL()}/users/password/reset/?client_id=${oidClientID}&redirect_uri=${redirectURL}&code_challenge=${codeChallenge}&code_challenge_method=S256` @@ -943,6 +944,7 @@ export const getRegisterURL = async returnTo => { redirectURL = redirectURL.replace(/([^:]\/)\/+/g, "$1"); + prepareOAuthReturnTo() const codeChallenge = await preparePKCECodeChallenge() const state = prepareOAuthState(SIGNUP_STATE_PREFIX) const nonce = generateSecureRandomString(32) diff --git a/src/components/users/OIDLoginCallback.jsx b/src/components/users/OIDLoginCallback.jsx index 7f243dbdc..36b98cf56 100644 --- a/src/components/users/OIDLoginCallback.jsx +++ b/src/components/users/OIDLoginCallback.jsx @@ -39,7 +39,7 @@ class OIDLoginCallback extends React.Component { return } setAlert({message: this.props.t('auth.signing_in'), severity: 'info'}) - // next is only set by sign-ins started before redirect_uri became fixed; new ones come back via returnTo. + // next still decides the redirect_uri sent for sign-ins that started before redirect_uri was fixed. this.setState({next: next && next !== '/' ? next : null, returnTo: returnTo }, () => { const redirectURL = this.state.next ? window.location.origin + this.state.next : (window.LOGIN_REDIRECT_URL || process.env.LOGIN_REDIRECT_URL) const clientId = window.OIDC_RP_CLIENT_ID || process.env.OIDC_RP_CLIENT_ID @@ -89,10 +89,10 @@ class OIDLoginCallback extends React.Component { cacheUserData() { refreshCurrentUserCache(() => { - if(this.state.next) - window.location.hash = '#' + this.state.next - else if(this.state.returnTo) + if(this.state.returnTo) window.location.hash = '#' + this.state.returnTo + else if(this.state.next) + window.location.hash = '#' + this.state.next else { let returnToURL = '/' if(this.props?.location?.search) { From c707d33c2cca038e5b652d2a97ca42a78cc75f35 Mon Sep 17 00:00:00 2001 From: Jonathan Payne Date: Wed, 30 Sep 2026 12:50:02 -0400 Subject: [PATCH 4/5] OpenConceptLab/ocl_issues#2856 | The return-page guard matches paths case-insensitively, like the router From the Codex review: the router matches /SIGNUP and /OIDC/login the same as their lowercase forms, and /signin#section slipped past the delimiters. The guard now tests the path before any ? or #, ignoring case. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/common/utils.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/common/utils.js b/src/common/utils.js index 81d14bb2c..d8583fdcf 100644 --- a/src/common/utils.js +++ b/src/common/utils.js @@ -898,7 +898,9 @@ export const consumeAndValidateOAuthState = returnedState => { // always goes through LOGIN_REDIRECT_URL, and the page to come back to (its hash route) waits here, in this tab. const prepareOAuthReturnTo = returnTo => { const route = returnTo?.includes('#') ? returnTo.slice(returnTo.indexOf('#') + 1) : null - if(route?.startsWith('/') && !/^\/(oidc\/login|signin|signup)(\/|\?|$)/.test(route)) + const path = route?.split(/[?#]/)[0] + // The router matches paths case-insensitively, so /SIGNUP would start a sign-up too. + if(path?.startsWith('/') && !/^\/(oidc\/login|signin|signup)(\/|$)/i.test(path)) sessionStorage.setItem(OAUTH_RETURN_TO_KEY, route) else sessionStorage.removeItem(OAUTH_RETURN_TO_KEY) From 9a1f83f4064ec930a9d08ad5c22ba8909ab6daad Mon Sep 17 00:00:00 2001 From: Jonathan Payne Date: Wed, 30 Sep 2026 12:55:11 -0400 Subject: [PATCH 5/5] OpenConceptLab/ocl_issues#2856 | The return-page guard decodes the path once, like the router From the Codex review: history decodes a route path once before the router matches it, so /%73ignin or /%53IGNUP opened the sign-in or sign-up page after all. The guard now checks the decoded path, and a malformed encoding, which would make the router throw, isn't saved. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/common/utils.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/common/utils.js b/src/common/utils.js index d8583fdcf..3d971328a 100644 --- a/src/common/utils.js +++ b/src/common/utils.js @@ -893,12 +893,22 @@ export const consumeAndValidateOAuthState = returnedState => { return !returnedState || returnedState === storedState } +// A route's path as the router sees it: decoded once, as history does, so /%73ignup is /signup. A malformed +// encoding makes the router throw, so it has no path. +const routePath = route => { + try { + return decodeURI(route.split(/[?#]/)[0]) + } catch { + return null + } +} + // Keycloak only redeems a code when the token request repeats the sign-in's redirect_uri exactly, and the // callback can't rebuild a page's query string (e.g. ?referrer= on links from openconceptlab.org). So sign-in // always goes through LOGIN_REDIRECT_URL, and the page to come back to (its hash route) waits here, in this tab. const prepareOAuthReturnTo = returnTo => { const route = returnTo?.includes('#') ? returnTo.slice(returnTo.indexOf('#') + 1) : null - const path = route?.split(/[?#]/)[0] + const path = route && routePath(route) // The router matches paths case-insensitively, so /SIGNUP would start a sign-up too. if(path?.startsWith('/') && !/^\/(oidc\/login|signin|signup)(\/|$)/i.test(path)) sessionStorage.setItem(OAUTH_RETURN_TO_KEY, route)