fix: header sign in honors configured path and preserves callbackurl (#961)

[frano-m]

Summary

• Widens HeaderProps.authenticationEnabled from boolean to boolean | string. Pass true (current default) for the legacy /login destination, or pass a path string to tell the Sign In button where to navigate (e.g. when NextAuth's pages.signIn is configured elsewhere).
• The Sign In button now appends the current asPath as a ?callbackUrl= query param, so paired with LoginView's query-callbackUrl preference (feat: LoginView should honor NextAuth callbackUrl from the query string (middleware-friendly) #955) the user lands back where they were after authenticating.
• New getSignInPath helper colocated in Authentication/utils.ts keeps the prop-decoding logic out of the component body.

Closes #961

Why

Today the Sign In button does Router.push("/login") unconditionally, ignoring both the consumer's NextAuth pages.signIn config and the user's current location. Repro for a consumer with pages.signIn = "/":

1. User on /help (public).
2. Click Sign In → Router.push("/login").
3. Middleware finds no token at /login → redirects to /?callbackUrl=/login.
4. User signs in. Post-login they're sent to the literal /login path (callbackUrl preserved via feat: LoginView should honor NextAuth callbackUrl from the query string (middleware-friendly) #955), which doesn't exist in the consumer app → they end up wherever the consumer's catch-all routes 404s to.

End result: the user wanted to be back on /help but lands somewhere unrelated.

Back-compat

• authenticationEnabled?: boolean → authenticationEnabled?: boolean | string is purely additive. Existing consumers passing true/false/undefined see no behavior change for the destination (still /login).
• The ?callbackUrl= is appended unconditionally — but with no consumer of that query param, it's a no-op cosmetic addition. Consumers running feat: LoginView should honor NextAuth callbackUrl from the query string (middleware-friendly) #955-aware LoginView start honoring it.
• useHeaderVisibility.isActionsIn got a Boolean(...) coercion so the union widening doesn't leak into the visibility return type.

Test plan

• tests/getSignInPath.test.ts (2 cases) — pure-helper coverage.
• tests/authentication.test.tsx (4 cases) — covers: not rendered when disabled; default /login destination when true; custom path when string; closeMenu called after navigation. All assertions check that Router.push receives { pathname, query: { callbackUrl: <asPath> } }.
• npm run lint, npm run check-format, npx tsc, full npm test (475 tests) all clean.
• Verified end-to-end in a consumer (hca-atlas-tracker) via local tarball install: Sign In from /requesting-elevated-permissions now navigates to /?callbackUrl=/requesting-elevated-permissions, and after Google OAuth the user lands back on /requesting-elevated-permissions (no detour through /atlases).

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR updates the library Header authentication UX to (1) honor a consumer-configured sign-in path and (2) preserve the user’s current location via a callbackUrl query parameter, improving compatibility with NextAuth pages.signIn configurations and middleware-driven flows.

Changes:

• Widened HeaderProps.authenticationEnabled (and related props) from boolean to boolean | string, where a string represents a custom sign-in path.
• Updated the header Sign In navigation to push { pathname, query: { callbackUrl: asPath } } and introduced a getSignInPath helper to keep decoding logic out of the component.
• Added unit/component tests covering default and custom destinations, callbackUrl propagation, and basic rendering behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
tests/getSignInPath.test.ts	Adds unit coverage for getSignInPath default vs custom path behavior.
tests/authentication.test.tsx	Adds component-level coverage for Sign In button rendering and Router navigation payload (including callbackUrl).
src/components/Layout/components/Header/hooks/useHeaderVisibility.ts	Coerces isActionsIn to boolean to avoid union type leakage after widening authenticationEnabled.
src/components/Layout/components/Header/header.tsx	Updates HeaderProps typing + documentation for authenticationEnabled behavior.
src/components/Layout/components/Header/components/Content/components/Actions/components/Menu/components/Toolbar/toolbar.tsx	Prop type widened to match HeaderProps change so menu toolbar passes through custom sign-in path.
src/components/Layout/components/Header/components/Content/components/Actions/components/Authentication/utils.ts	Introduces getSignInPath helper encapsulating default vs configured sign-in path selection.
src/components/Layout/components/Header/components/Content/components/Actions/components/Authentication/authentication.tsx	Updates Sign In click handler to navigate to resolved path and include callbackUrl from asPath.

[NoopDog]

Code review findings

Targeted, correct fix with good test coverage and full back-compat — no blocking issues. A few notes for consideration:

1. renderButton active-state still keys off ROUTE.LOGIN (minor, cosmetic). When a consumer configures a custom path (e.g. authenticationEnabled="/"), the Sign In button's activeNav highlight is computed via isNavigationLinkSelected(pathname, [ROUTE.LOGIN]) — so on the custom sign-in page the button won't render as active. renderButton isn't wired to the prop, so threading the custom path through is arguably out of scope, but worth a follow-up since the rest of the PR is about honoring the configured path.

2. boolean | string overloads one prop with two concerns (design nit, not blocking). Encoding the path into the enable-flag is a slight smell vs. a separate signInPath?: string prop. The union keeps the surface minimal and is purely additive/back-compat, so it's a defensible tradeoff — just flagging it as the one debatable API choice.

3. ?callbackUrl= appended unconditionally, including the legacy /login path. For true consumers the destination becomes /login?callbackUrl=<asPath>. This is a no-op for non-#955 consumers and an improvement for #955-aware ones — just confirming it's intended for the default path too (reads as intended).

4. Mixed Router.push (singleton) + useRouter() for asPath (trivial). Functionally identical (Router.push === useRouter().push) and consistent with existing code, but using the push from the useRouter() destructure would be marginally more idiomatic. Optional.

Verified: rules-of-hooks respected (useProfile/useRouter both unconditional before early returns), the Boolean(...) coercion in useHeaderVisibility correctly contains the union widening, no open-redirect risk (signInPath is consumer config, asPath is in-app), and the #955 pairing checks out (useNextAuthService prefers getQueryCallbackUrl(query.callbackUrl)). CI green. Approve.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.