[paynejd]

Caution

Do not merge. This PR is on hold until the team agrees on it on Mon Nov 2, 2026. Nothing here deploys before then.

Part of OpenConceptLab/ocl_online#236: TBv2 is also served at app.v2.<env>.openconceptlab.org, ahead of TBv3 becoming the default at app.<env>.

app.<env> keeps serving TBv2, and nothing changes there. This PR only makes TBv2 behave correctly when it's opened at app.v2.<env>.

What changes on an app.v2.<env> host

	Before	After
Sign-in, sign-up, sign-out	Return to LOGIN_REDIRECT_URL (app.<env>), a different origin from the one the sign-in started on. The PKCE verifier is in the starting origin's sessionStorage, so the code exchange can't complete.	Return to the host they started on
getEnv()	development: the OpenMRS link goes to QA, errors are tagged dev	That host's environment
"Try the new TermBrowser" link	app.v3.v2.<env>	app.v3.<env>
Mapper link	map.v2.<env>	map.<env>

Why nothing changes on app.<env>

• LOGIN_REDIRECT_URL is the bare app.<env> origin in production, staging and QA, so window.location.origin gives the same redirect URI there. LOGIN_REDIRECT_URL still turns SSO on, as before.
• getEnv moves unchanged to src/common/env.js and is re-exported from utils.js, so every import keeps working. It strips app.v2. before matching, the way TBv3's getEnv strips app.v3..
• On an app.<env> host, toAppOrigin returns the origin as it is.

Changes

• src/common/env.js (new): getEnv, plus toAppOrigin, which maps app.v2.<env> to app.<env>.
• src/common/utils.js: toMapperURL and toV3URL use toAppOrigin. getLoginURL, getRegisterURL and getSSOLogoutURL fall back to window.location.origin.
• src/components/users/OIDLoginCallback.jsx: the code exchange's fallback redirect URI is window.location.origin.
• src/common/__tests__/env.test.js (new): getEnv on every app and app.v2 host, and toAppOrigin.

Testing

• node --test src/common/__tests__/*.test.js: 25 pass, including the 6 new ones.
• CI's ESLint command (eslint --ext .jsx,.js src/): clean.
• A production webpack build compiles every JS module. The only errors are node-sass's, which doesn't run on arm64 Macs.
• Signing in on app.v2.<env> needs the load balancer rules and Keycloak redirect URIs from ocl_online#236. It gets tested on app.v2.staging once they're in, before production.

Rollout (after Nov 2 sign-off)

This PR can deploy before or after the app.v2 hosts go live, since it changes nothing on app.<env>. One ordering rule: an environment's app.v2 Keycloak redirect and post-logout URIs must be registered before its app.v2 host is reachable. Otherwise Keycloak refuses the sign-out after the app has cleared its tokens, and silent SSO signs the user back in. The companion TBv3 change is OpenConceptLab/oclweb3 (same branch name).

Sign-out goes through Keycloak only when SSO is on (isSSOEnabled()). Before, a missing LOGIN_REDIRECT_URL stood in for that check.

🤖 Generated with Claude Code

[paynejd]

Codex review, pass 1 (codex-cli 0.166.0, commit 4827130)

Verdict: clean. No findings, so no further passes are needed.

findings.json

{
  "reviewer": "codex-cli 0.166.0",
  "perspective": "defects",
  "mode": "full",
  "commit": "4827130b2bef658df6482d59c6d4e18b3fe3ac51",
  "verdict": "clean",
  "prior": [],
  "findings": []
}

[paynejd]

Simplification review (claude subagent, commit 4827130)

Verdict: clean. Nothing to cut. Review loop done: Codex pass 1 clean, no commits since it.

simplify.json

{
  "reviewer": "claude subagent",
  "perspective": "simplify",
  "mode": "full",
  "commit": "4827130b2bef658df6482d59c6d4e18b3fe3ac51",
  "verdict": "clean",
  "prior": [],
  "findings": []
}

[paynejd]

Adversarial review: sign-in redirects (claude subagent, commit 4827130)

Verdict: nothing exploitable. The redirect URI is the page's own origin, and the returnTo/next handling is unchanged. The code exchange sends the authorize request's redirect_uri in every flow (sign-in, sign-up incl. verification in a new tab, callback with and without next, silent SSO). No code or token reaches a new origin. Findings and responses:

• A1 (P2, introduced): sign-in and sign-out now need the page's host to be registered in Keycloak. On a host Keycloak doesn't know, a refused sign-out leaves the Keycloak session in place after the app clears its tokens. Accepted, handled by ordering: each environment's app.v2 redirect and post-logout URIs are registered before its app.v2 host is reachable. The rollout notes above say so.
• A2 (P2, introduced): with the redirect always set, getSSOLogoutURL no longer stood in for the SSO check, so a stale id_token on an SSO-off deployment sent sign-out to the API. Fixed in 6cc36fa (isSSOEnabled() && idToken).
• A3 (P1, pre-existing on master since d94aa32): forceLoginUser in App.jsx uses pathname before it's declared, which affects deployments with SSO off. Out of scope here. Flagged for a separate ticket.

adversarial.json

{
  "reviewer": "claude subagent (adversarial)",
  "perspective": "defects",
  "mode": "full",
  "commit": "4827130b2bef658df6482d59c6d4e18b3fe3ac51",
  "verdict": "findings",
  "prior": [],
  "findings": [
    {
      "id": "A1",
      "severity": "P2",
      "tag": null,
      "file": "src/common/utils.js",
      "line": 978,
      "claim": "Introduced: sign-in, sign-up and sign-out now work only if the host serving the page is registered in Keycloak, both as a redirect URI and as a post-logout redirect URI. On master an unregistered host fell back to LOGIN_REDIRECT_URL, which is registered. The sign-out case is the one with a security cost: the user ends up still signed in to Keycloak.",
      "scenario": "Any host that serves this build but isn't registered: app.v2.<env> if DNS/ALB routing goes live before the Keycloak client change for that env, a self-hosted alias hostname, or one where LOGIN_REDIRECT_URL is the only registered value. Sign-in and sign-up send redirect_uri=<that origin>, and Keycloak stops on 'Invalid parameter: redirect_uri' (on master the user was sent to LOGIN_REDIRECT_URL, got 'Sign in to continue' and finished on the canonical host). Sign out is worse. signOutUser clears the local tokens (utils.js:803-806) and then goes to /users/logout/ with post_logout_redirect_uri=<that origin>. Keycloak's RP-initiated logout checks that URI before it ends the session. It shows 'Invalid redirect uri' and keeps the SSO session. The user thinks they've signed out, but on a shared machine the next person to open app.<env> or TBv3 is signed in again as them by silent SSO. Nothing an attacker can steer: the origin is the page's own, and the returnTo/next paths are unchanged.",
      "action": "Make the Keycloak registration a gate on the rollout. For every env, register app.v2.<env>/* as a redirect URI and as a post-logout redirect URI (or keep the post-logout setting at '+'), confirm a sign-in and a sign-out work, and only then point DNS at app.v2.<env>. Also say in the PR and the self-host notes that LOGIN_REDIRECT_URL no longer sets where users return to, so every host users load TBv2 from must be registered. Optional hardening: use window.location.origin only when toAppOrigin(origin) equals the origin of LOGIN_REDIRECT_URL, and otherwise keep the old LOGIN_REDIRECT_URL fallback.",
      "lines_delta": null,
      "confidence": "medium"
    },
    {
      "id": "A2",
      "severity": "P2",
      "tag": null,
      "file": "src/common/utils.js",
      "line": 1025,
      "claim": "Introduced: getSSOLogoutURL no longer depends on SSO being turned on. redirectURL is now always truthy, so any id_token left in localStorage sends a normal Sign out through <API>/users/logout/, even where LOGIN_REDIRECT_URL is unset. That contradicts the new comment 'LOGIN_REDIRECT_URL only turns SSO on'. On master, `'' && idToken` was falsy and the sign-out stayed local.",
      "scenario": "A non-SSO deployment (LOGIN_REDIRECT_URL unset, so the published image's process.env.LOGIN_REDIRECT_URL is '') whose origin still holds an id_token from an earlier SSO setup. Example: a developer on localhost:4000 switches between docker-compose.sso.yml and docker-compose.yml, or a self-hoster turns SSO off. Clicking Sign out clears the tokens and then navigates to <API>/users/logout/?id_token_hint=...; an oclapi2 with SSO off answers 405 (OIDCLogoutView). The user lands on an API error page instead of back in the app. A forced sign-out already behaved this way on master, because it always passed a returnTo.",
      "action": "Turn SSO sign-out back on only when SSO is on: `if(isSSOEnabled() && idToken)` in getSSOLogoutURL. That also covers the forced path.",
      "lines_delta": 0,
      "confidence": "medium"
    },
    {
      "id": "A3",
      "severity": "P1",
      "tag": null,
      "file": "src/components/app/App.jsx",
      "line": 116,
      "claim": "Pre-existing (on origin/master since d94aa320, 2025-09-25), not caused by this PR: forceLoginUser runs on every App mount. When SSO is off it calls getLoginURL(origin + '/#' + pathname) and navigates there. pathname is used before its `const` declaration, which Babel compiles to var, so its value is undefined. A non-SSO deployment therefore sends every fresh page load to the sign-in page.",
      "scenario": "This is the default non-SSO DPG stack: oclweb2 docker-compose.yml with no LOGIN_REDIRECT_URL or OIDC_RP_CLIENT_ID, so the published image bakes LOGIN_REDIRECT_URL='' and master's getLoginURL doesn't throw either. Opening any deep link anonymously (a shared concept URL, for example) lands on /#/accounts/login?returnTo=<origin>/#undefined. After a form sign-in the app sets hash '#<origin>/#undefined', which matches no route. The PR doesn't change this, except for builds where process.env.LOGIN_REDIRECT_URL is truly undefined (webpack run without --env): master's getLoginURL threw there, which hid the redirect, and the PR's doesn't throw. Mentioned because the review brief asked about self-hosted deployments.",
      "action": "Separate ticket: in forceLoginUser, replace the `if(!isSSOEnabled()) {...}` block with `if(!isSSOEnabled()) return` (or delete it), and move the props.location destructuring above any use of pathname.",
      "lines_delta": -2,
      "confidence": "high"
    }
  ]
}

[paynejd]

Codex check of commits 4827130..6cc36fa (codex-cli 0.166.0)

Verdict: clean. The A2 fix broke nothing. Review loop done.

findings.json

{
  "reviewer": "codex-cli 0.166.0",
  "perspective": "defects",
  "mode": "final",
  "commit": "6cc36fa7",
  "verdict": "clean",
  "prior": [
    {
      "id": "A2",
      "status": "resolved"
    }
  ],
  "findings": []
}