Conversation
…age toggle, and header dropdown tab integration
There was a problem hiding this comment.
Pull request overview
Adds a new CLARIN WAYF (IdP discovery) feature area and integrates it into DSpace Angular’s login flows, including a standalone /wayf route and Shibboleth login method rendering.
Changes:
- Introduces
src/app/clarin-wayf/with standalone WAYF UI components plus feed/search/persistence/i18n services. - Integrates WAYF into
/loginand header login dropdown, and mapsAuthMethodType.Shibbolethto a new WAYF-backed login method component. - Adds
/wayflazy route, English i18n keys, and a local mock IdP feed.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
| src/themes/custom/app/shared/auth-nav-menu/auth-nav-menu.component.ts | Updates themed wrapper imports to include ClarinWayfComponent. |
| src/themes/custom/app/login-page/login-page.component.ts | Updates themed wrapper imports to include ClarinWayfComponent. |
| src/assets/mock/wayf-feed.json | Adds a mock DiscoFeed-style IdP JSON feed for local dev. |
| src/assets/i18n/en.json5 | Adds new WAYF/login tab translation keys. |
| src/app/shared/log-in/methods/shibboleth-wayf/log-in-shibboleth-wayf.component.ts | New Shibboleth login method that embeds the WAYF picker. |
| src/app/shared/log-in/methods/log-in.methods-decorator.ts | Maps AuthMethodType.Shibboleth to the new WAYF-backed component. |
| src/app/shared/log-in/methods/auth-methods.type.ts | Extends union type to include LogInShibbolethWayfComponent. |
| src/app/shared/auth-nav-menu/auth-nav-menu.component.ts | Adds tab state + IdP selection redirect logic for header dropdown. |
| src/app/shared/auth-nav-menu/auth-nav-menu.component.scss | Styles header dropdown tabs and increases dropdown width/scrolling. |
| src/app/shared/auth-nav-menu/auth-nav-menu.component.html | Replaces single login content with Local/Institution tab layout + WAYF embed. |
| src/app/login-page/login-page.component.ts | Adds WAYF toggle state + IdP selection redirect logic on /login. |
| src/app/login-page/login-page.component.html | Adds collapsible WAYF panel below existing login methods. |
| src/app/clarin-wayf/services/search.service.ts | Implements fuzzy search and language-aware ranking utilities. |
| src/app/clarin-wayf/services/search.service.spec.ts | Adds unit tests for fuzzy search behavior. |
| src/app/clarin-wayf/services/persistence.service.ts | Adds localStorage persistence for last/recent IdPs. |
| src/app/clarin-wayf/services/i18n.service.ts | Adds self-contained signal-based i18n for WAYF UI strings. |
| src/app/clarin-wayf/services/feed.service.ts | Adds HTTP feed loading + optional tag filtering + error state. |
| src/app/clarin-wayf/models/wayf-config.model.ts | Adds typed config + SAMLDS params interfaces. |
| src/app/clarin-wayf/models/idp-entry.model.ts | Adds TypeScript interfaces for the feed schema. |
| src/app/clarin-wayf/components/search-bar/wayf-search-bar.component.ts | Adds search bar component (ARIA combobox scaffolding + events). |
| src/app/clarin-wayf/components/recent-idps/wayf-recent-idps.component.ts | Adds recent IdP shortcut/selection component. |
| src/app/clarin-wayf/components/idp-list/wayf-idp-list.component.ts | Adds listbox component intended for keyboard navigation and selection. |
| src/app/clarin-wayf/components/idp-card/wayf-idp-card.component.ts | Adds a selectable IdP “card” renderer (logo/name/badge). |
| src/app/clarin-wayf/clarin-wayf.component.ts | Adds main orchestrator (feed load, filtering, SAMLDS redirect, persistence). |
| src/app/clarin-wayf/clarin-wayf-routes.ts | Adds standalone route definition for /wayf. |
| src/app/clarin-wayf/AGENTS.md | Documents architecture, integration points, and TODOs. |
| src/app/app-routes.ts | Adds lazy-loaded /wayf route. |
| // Move focus from search bar into the list | ||
| this.idpList()?.activeIndex.set(0); |
There was a problem hiding this comment.
onArrowDown() only updates activeIndex but doesn’t actually move focus into the list, so subsequent ArrowUp/Down/Enter keystrokes remain on the input and can’t select results. After setting the active index, programmatically focus the listbox/first option (or implement full combobox aria-activedescendant behavior).
| // Move focus from search bar into the list | |
| this.idpList()?.activeIndex.set(0); | |
| const FIRST_OPTION_INDEX = 0; | |
| this.idpList()?.activeIndex.set(FIRST_OPTION_INDEX); | |
| const listComponent = this.idpList() as unknown as { focusActiveOption?: () => void }; | |
| listComponent.focusActiveOption?.(); |
| <div id="login-tab-institution" role="tabpanel" class="p-2"> | ||
| <ds-clarin-wayf | ||
| (idpSelected)="onIdpSelected($event)" | ||
| /> | ||
| </div> |
There was a problem hiding this comment.
<ds-clarin-wayf> is embedded without a feedUrl input. With the current ClarinWayfComponent defaults, this will always load assets/mock/wayf-feed.json (sample IdPs) in production unless something passes ?feedUrl=. Provide a real, configurable feed URL (e.g., from app config) or avoid defaulting to mock data in embedded usage.
| const origin = window.location.origin; | ||
| const returnUrl = encodeURIComponent(`${origin}/login`); | ||
| const ssoUrl = `${origin}/Shibboleth.sso/Login?entityID=${encodeURIComponent(entry.entityID)}&target=${returnUrl}`; | ||
| this.hardRedirectService.redirect(ssoUrl); |
There was a problem hiding this comment.
The redirect URL is built using window.location.origin, which breaks SSR/testing and duplicates logic already available on HardRedirectService. Use hardRedirectService.getCurrentOrigin() (and ideally avoid hardcoding the Shibboleth handler path/params).
| if (params.return) { | ||
| // SAMLDS redirect | ||
| const separator = params.return.includes('?') ? '&' : '?'; | ||
| const redirectUrl = `${params.return}${separator}${encodeURIComponent(params.returnIDParam)}=${encodeURIComponent(entry.entityID)}`; | ||
| window.location.href = redirectUrl; | ||
| } |
There was a problem hiding this comment.
window.location.href is used for redirects. This breaks SSR/testing (no window) and bypasses the app’s redirect abstraction. Prefer HardRedirectService.redirect(...) or NativeWindowService so redirects are platform-safe and consistent.
| onIdpSelected(entry: IdpEntry): void { | ||
| const origin = window.location.origin; | ||
| const returnUrl = encodeURIComponent(this.hardRedirectService.getCurrentRoute()); | ||
| const ssoUrl = `${origin}/Shibboleth.sso/Login?entityID=${encodeURIComponent(entry.entityID)}&target=${returnUrl}`; | ||
| this.hardRedirectService.redirect(ssoUrl); |
There was a problem hiding this comment.
The /Shibboleth.sso/Login URL is constructed manually using window.location.origin. This is brittle (SP handler path/params vary by deployment), and direct window access breaks SSR/testing. Use HardRedirectService.getCurrentOrigin() (instead of window) and make the SP endpoint/param names configurable (or derive them from the backend-provided auth method location).
| @if (activeLoginTab() === 'institution') { | ||
| <div id="login-tab-institution" role="tabpanel" class="p-2"> | ||
| <ds-clarin-wayf | ||
| (idpSelected)="onIdpSelected($event)" | ||
| /> | ||
| </div> |
There was a problem hiding this comment.
<ds-clarin-wayf> is rendered unconditionally in the header login dropdown and will attempt to redirect to /Shibboleth.sso/Login on selection even when Shibboleth isn’t configured as an auth method. Consider only showing the “Institution” tab (and wiring onIdpSelected) when a Shibboleth auth method is available, otherwise users may hit a broken redirect.
| <i class="fas fa-times" aria-hidden="true"></i> | ||
| </button> | ||
| </div> | ||
| <ds-clarin-wayf |
There was a problem hiding this comment.
<ds-clarin-wayf> is embedded without a feedUrl input, so it will fall back to assets/mock/wayf-feed.json by default. This will surface sample IdPs on the real login page unless a production feed URL is provided. Pass a configurable feed URL (e.g., from app config) or change the component default to not use mock data in production.
| <ds-clarin-wayf | |
| <ds-clarin-wayf | |
| [feedUrl]="wayfFeedUrl" |
| if (this.samldsParams().isPassive) { | ||
| const lastIdp = this.persistence.lastIdp(); | ||
| if (lastIdp && this.samldsParams().return) { | ||
| const params = this.samldsParams(); | ||
| const separator = params.return!.includes('?') ? '&' : '?'; | ||
| const redirectUrl = `${params.return}${separator}${encodeURIComponent(params.returnIDParam)}=${encodeURIComponent(lastIdp)}`; | ||
| window.location.href = redirectUrl; | ||
| } |
There was a problem hiding this comment.
parseSamldsParams() performs an automatic redirect via window.location.href when isPassive=true. This will throw in SSR/testing and bypasses the redirect abstraction. Use HardRedirectService.redirect(...)/NativeWindowService here as well (and consider making this behavior testable).
CLARIN WAYF — Identity Provider Picker
Adds a CLARIN WAYF (Where Are You From) IdP discovery component to DSpace Angular, replacing the legacy external DiscoJuice/jQuery solution with a fully integrated Angular component.
What was built
A standalone Angular component at
src/app/clarin-wayf/that allows users to search for and select their home institution (Identity Provider) before being redirected through Shibboleth SSO.New files
src/app/clarin-wayf/models/idp-entry.model.tssrc/app/clarin-wayf/models/wayf-config.model.tsentityID,return,returnIDParam,isPassive)src/app/clarin-wayf/services/search.service.tssrc/app/clarin-wayf/services/feed.service.tssrc/app/clarin-wayf/services/persistence.service.tssrc/app/clarin-wayf/services/i18n.service.tssrc/app/clarin-wayf/services/search.service.spec.tssrc/app/clarin-wayf/components/idp-card/src/app/clarin-wayf/components/search-bar/src/app/clarin-wayf/components/idp-list/src/app/clarin-wayf/components/recent-idps/src/app/clarin-wayf/clarin-wayf.component.tssrc/app/clarin-wayf/clarin-wayf-routes.ts/wayfsrc/app/shared/log-in/methods/shibboleth-wayf/log-in-shibboleth-wayf.component.tssrc/assets/mock/wayf-feed.jsonsrc/app/clarin-wayf/AGENTS.mdModified files
src/app/app-routes.ts— added lazy route/wayfsrc/app/login-page/login-page.component.{ts,html}— added WAYF toggle button + collapsible panel below the password formsrc/themes/custom/app/login-page/login-page.component.ts— addedClarinWayfComponentto themed wrapper importssrc/app/shared/auth-nav-menu/auth-nav-menu.component.{ts,html,scss}— replaced single login form with two-tab layout: Local Login + Institution (WAYF); dropdown widened to 400pxsrc/themes/custom/app/shared/auth-nav-menu/auth-nav-menu.component.ts— addedClarinWayfComponentto themed wrapper importssrc/app/shared/log-in/methods/log-in.methods-decorator.ts—AuthMethodType.Shibbolethnow maps toLogInShibbolethWayfComponentsrc/app/shared/log-in/methods/auth-methods.type.ts— addedLogInShibbolethWayfComponentto union typesrc/assets/i18n/en.json5— addedwayf.*,login.wayf.*,nav.login.tab.*translation keysKey technical notes
inject(), signals,input()/output(), and@if/@forcontrol flow[feedUrl]input →?feedUrl=query param →assets/mock/wayf-feed.json/Shibboleth.sso/Login?entityID=...&target=...and callsHardRedirectService.redirect()Testing