[sejal-gupta-ksolves]

Summary

NIFI-15393

When an unauthenticated user opens a shared NiFi URL, the login flow does not preserve it, and after logging in the user lands on the root canvas instead of the requested page.

This change captures the requested route when the user is redirected to the login page and navigates back to it after a successful login:

• The URL is captured in the navigateToLogIn$ effect. The auth interceptor already dispatches navigateToLogIn on a 401, so no change to authentication.guard.ts is needed.
• The URL is read from window.location.hash rather than router.url. With NavigationActionTiming.PostActivation, router.url does not reflect the pending navigation, and the application uses hash routing.
• /login and /logout-complete are not stored, so logging out and back in does not return the user to the logout-complete page.
• Session storage access goes through a new StorageService that wraps SessionStorageService, so the storage key is defined in one place.
• After login, AccessEffects navigates to the stored URL and removes it. If none is stored, it navigates to / as before.

Only values starting with / are stored, and the stored URL is passed to Router.navigateByUrl, so the redirect stays inside the application.

Verification

Run from nifi-frontend/src/main/frontend using Node 24 (the version in package.json):

• npx nx test nifi --testFile=storage.service.spec.ts
• npx nx lint nifi
• npx prettier --check on the changed files: passes
• npx nx test nifi
• npx nx test shared
• ./mvnw clean install -P contrib-check -pl nifi-frontend

[mcgilman]

Will review...

[mcgilman]

Thanks for adding support for returning users to their original route. Capturing the hash in navigateToLogIn$ is the right approach, and the additional startsWith('/') validation is useful.

I think the restore flow needs adjustment before merging: loginSuccess$ only covers username/password login, the public-route predicate currently allows several unauthenticated routes to be captured, and the new storage wrapper duplicates the existing SessionStorageService abstraction. The inline comments propose a structure that handles both form and external authentication.

[mcgilman]

loginSuccess$ only runs after the username/password form succeeds. External authentication redirects through loginUri and never dispatches AccessActions.loginSuccess; after returning from the identity provider, the authentication guard instead dispatches loadCurrentUserSuccess.

Could we leave this effect responsible only for navigating to the authenticated root route?

this.router.navigate(['/']);

The stored URL can then be restored from a loadCurrentUserSuccess effect, which covers both form and external authentication. It also avoids removing the URL before the subsequent current-user request has succeeded.

[sejal-gupta-ksolves]

loginSuccess$ is back to just this.router.navigate(['/']) (no diff against main for that file). The stored URL is now restored in a new restoreReturnUrl$ effect on loadCurrentUserSuccess, so it covers both form and external login. It removes the key before calling navigateByUrl, so later polling successes are no-ops.

[mcgilman]

This compares the entire hash and only excludes two routes. Consequently, values such as /login?returnTo=/settings, /error, and /404 can be stored and restored after login.

Could we centralize the exact public-route check while preserving the original URL, including its query string? For example, in state/index.ts:

export const RETURN_URL = 'return-url';

const UNAUTHENTICATED_ROUTES: ReadonlySet<string> = new Set(['/login', '/logout-complete', '/error', '/404']);

export function isAuthenticatedRoute(url: string): boolean {
    const pathname = url.split('?')[0];
    return !UNAUTHENTICATED_ROUTES.has(pathname);
}

Then capture it here, keeping the useful leading-slash validation:

const currentUrl = window.location.hash.substring(1);
if (currentUrl.startsWith('/') && isAuthenticatedRoute(currentUrl)) {
    this.sessionStorage.setItem<string>(RETURN_URL, currentUrl);
}

this.router.navigate(['/login']);

The corresponding root-level restore effect can handle both login methods and remove the key before navigation so later polling successes are no-ops:

restoreReturnUrl$ = createEffect(
    () =>
        this.actions$.pipe(
            ofType(UserActions.loadCurrentUserSuccess),
            tap(() => {
                const returnUrl = this.sessionStorage.getItem<string>(RETURN_URL);
                if (returnUrl) {
                    this.sessionStorage.removeItem(RETURN_URL);
                    this.router.navigateByUrl(returnUrl);
                }
            })
        ),
    { dispatch: false }
);

[sejal-gupta-ksolves]

Followed your suggestion: added RETURN_URL, UNAUTHENTICATED_ROUTES (/login, /logout-complete, /error, /404) and isAuthenticatedRoute() to state/index.ts. The check matches on the path only, so something like /login?returnTo=/settings is excluded, while an authenticated route such as /settings?x=1 is stored with its query string. navigateToLogIn$ keeps the startsWith('/') check and uses the shared predicate.

[mcgilman]

This class only forwards setItem, getItem, and removeItem to the existing SessionStorageService. Could we use SessionStorageService directly from CurrentUserEffects and remove this wrapper?

That would also let the tests focus on the behavior that matters: exact exclusion of all four public routes (including query strings), preservation of authenticated-route query strings, capture during navigateToLogIn$, restore and removal during loadCurrentUserSuccess, and no navigation when a return URL is absent.

[sejal-gupta-ksolves]

Thanks, agreed. Removed StorageService (and its spec) and CurrentUserEffects now uses SessionStorageService directly. The tests now cover the behavior you listed: exact exclusion of the four public routes (including with query strings), preservation of authenticated-route query strings, capture in navigateToLogIn$, restore and removal on loadCurrentUserSuccess, and no navigation when no return URL is stored.