DSpace9/Redirect back to originating page after DiscoJuice local login - #1472
Merged
milanmajchrak merged 3 commits intoAug 21, 2026
Merged
Conversation
#876) On the standalone login page the post-login redirect ignored the `redirectUrl` query param that the DiscoJuice local-auth flow (src/aai/aai.js) appends as the absolute page URL, so signing in from e.g. the search page always landed on the home page. `LogInPasswordComponent.submit()` now reads that query param, reduces it to an app-relative path (the same format `HardRedirectService.getCurrentRoute()` produces, which `reloadGuard` already consumes), and uses it as the redirect target; it prefers a nested `redirectUrl` so login is never the target, and keeps the previous `setRedirectUrlIfNotSet('/')` fallback when no param is present. Fixes the dspace-ui-tests LINDAT-013 scenario (loginPage.spec.ts "login from search page should redirect back to search page"). Mirrors the dtq-dev fix 9dff6af, adapted to the refactored v9 component. Refs #876 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Shorten the multiline comments/JSDoc added for the redirect-from-search fix to one-liners; the fuller rationale now lives in the PR description. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes CLARIN DSpace v9 standalone DiscoJuice local-auth login flow so users are redirected back to the originating page (via redirectUrl query param) instead of always landing on the home page, unblocking dspace-ui-tests scenario LINDAT-013.
Changes:
- Read
redirectUrlfrom the standalone login page query params and store it as the post-login redirect target. - Normalize absolute URLs to app-relative paths and prefer nested
redirectUrlto avoid redirecting back to/login. - Add unit tests covering the new redirect behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/app/shared/log-in/methods/password/log-in-password.component.ts | Read and normalize redirectUrl query param for standalone login redirects. |
| src/app/shared/log-in/methods/password/log-in-password.component.spec.ts | Adds unit tests validating standalone-login redirect behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- getRedirectUrlFromQueryParams() now returns `string | null` and guards against non-string (e.g. repeated `string[]`) query params, so login submission falls back to the default redirect instead of throwing. - Normalize the redirect with a string `.replace(/^https?:\/\/[^/]+/i, '')` (v7-style, can't throw) instead of `new URL(...)`. - Add unit tests for an already-relative redirectUrl and a non-string value. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On CLARIN DSpace v9, logging in from a non-home page through the DiscoJuice local authentication flow always redirected the user to the home page instead of back to the page they started from. The
dspace-ui-testsscenario LINDAT-013 (loginPage.spec.ts→ login from search page should redirect back to search page) failed because of this.Root cause
The DiscoJuice local-auth handler (
src/aai/aai.js) sends the user to/login?redirectUrl=<absolute page URL>. On the standalone login page,LogInPasswordComponent.submit()ignored that query param and always calledsetRedirectUrlIfNotSet('/'), so the stored post-login target was the home page. (The value travels through the URL on purpose — it is lost from the auth store while passing through DiscoJuice.)Fix
On the standalone login page,
LogInPasswordComponentnow reads theredirectUrlquery param and:/repository/search) — the same formatHardRedirectService.getCurrentRoute()produces and that the existingreloadGuardalready consumes;redirectUrl(login initiated from the login page) so we never redirect back to/login;setRedirectUrlIfNotSet('/')when noredirectUrlis present.Mirrors the verified dtq-dev fix
9dff6af543, adapted to the refactored v9 component (v9 usesHardRedirectServiceinstead of the v7baseUrl/config lookup).Implementation notes
Kept the code comments terse; the detail lives here.
getRedirectUrlFromQueryParams()readsroute.snapshot.queryParams.redirectUrl.aai.jsappends it as the absolute page URL (e.g.http://host/repository/search) — the value is dropped from the auth store while passing through DiscoJuice, so it has to be read back from the URL.?redirectUrl=…(login started while already on/login), the inner target is preferred so we never redirect back to/login.toRelativePath()drops the scheme+host with a string.replace(/^https?:\/\/[^/]+/i, '')(http://host/repository/search→/repository/search); already-relative values pass through unchanged. This is the same string-replace technique v7 uses (redirectUrl.replace(baseUrl, '')) — but host-agnostic, so it cannot throw and is unaffected by a misconfiguredui.baseUrl. The result matches the formatHardRedirectService.getCurrentRoute()produces, whichreloadGuardthen strips to the app route.getRedirectUrlFromQueryParams()returnsstring | nulland falls back to the default redirect for a missing or non-string (string[])redirectUrl, so login submission can never break on a malformed value.Scope
Covers LINDAT-013 (login initiated from a public page, where
aai.jsputs the target in the URL). It does not port dtq-dev'sngOnInit → setUpRedirectUrl()re-navigation, which additionally covers the guard-initiated flow (e.g. a restricted collection bounces to/loginwith the target only in the store) through DiscoJuice. No test covers that flow; can be added on request.Tests
Added unit tests to
log-in-password.component.spec.tscovering the relative-path reduction, query-string preservation, nested-redirectUrlhandling, and the no-param fallback.ng testfor this spec passes.🤖 Generated with Claude Code