diff --git a/app/portainer/authorization-guard.test.ts b/app/portainer/authorization-guard.test.ts index 38d58937b1..a89199ce0f 100644 --- a/app/portainer/authorization-guard.test.ts +++ b/app/portainer/authorization-guard.test.ts @@ -4,6 +4,8 @@ import { Transition, } from '@uirouter/angularjs'; +import { get, keyBuilder } from '@/react/hooks/useLocalStorage'; + import { checkAuthorizations } from './authorization-guard'; import { IAuthenticationService } from './services/types'; @@ -47,6 +49,7 @@ describe('checkAuthorizations', () => { } as unknown as Transition; stateTo.data.access = 'restricted'; + localStorage.removeItem(keyBuilder('RETURN_URL')); }); afterEach(() => { @@ -75,6 +78,24 @@ describe('checkAuthorizations', () => { expect($state.target).toHaveBeenCalledWith('portainer.logout'); }); + it('should store the current URL in localStorage when the user is not authenticated', async () => { + authService.init.mockResolvedValue(false); + + await checkAuthorizations(transition); + + expect(get('RETURN_URL', null)).toBe( + window.location.pathname + window.location.search + window.location.hash + ); + }); + + it('should not store a returnUrl when the user is authenticated', async () => { + authService.init.mockResolvedValue(true); + + await checkAuthorizations(transition); + + expect(get('RETURN_URL', null)).toBeNull(); + }); + it('should return undefined if user is an admin and access is "admin"', async () => { authService.init.mockResolvedValue(true); authService.isPureAdmin.mockReturnValue(true); diff --git a/app/portainer/authorization-guard.ts b/app/portainer/authorization-guard.ts index 7b128abdd7..c942195cda 100644 --- a/app/portainer/authorization-guard.ts +++ b/app/portainer/authorization-guard.ts @@ -1,5 +1,7 @@ import { Transition, TransitionService } from '@uirouter/angularjs'; +import { storeReturnUrl } from '@/react/portainer/helpers/returnUrl'; + import { IAuthenticationService } from './services/types'; export enum AccessHeaders { @@ -40,6 +42,9 @@ export async function checkAuthorizations(transition: Transition) { 'User is not authenticated, redirecting to login, access:', access ); + const currentUrl = + window.location.pathname + window.location.search + window.location.hash; + storeReturnUrl(currentUrl); return $state.target('portainer.logout'); } diff --git a/app/portainer/helpers/url-utils.test.ts b/app/portainer/helpers/url-utils.test.ts new file mode 100644 index 0000000000..5d6143ec51 --- /dev/null +++ b/app/portainer/helpers/url-utils.test.ts @@ -0,0 +1,51 @@ +import { isValidReturnUrl } from './url-utils'; + +describe('isValidReturnUrl', () => { + const origin = 'http://localhost:9000'; + + it('returns false for empty string', () => { + expect(isValidReturnUrl('', origin)).toBe(false); + }); + + it('returns false for cross-origin URLs', () => { + expect(isValidReturnUrl('https://evil.com', origin)).toBe(false); + expect(isValidReturnUrl('http://evil.com/path', origin)).toBe(false); + expect(isValidReturnUrl('//evil.com', origin)).toBe(false); + expect(isValidReturnUrl('javascript:alert(1)', origin)).toBe(false); + }); + + it('rejects backslash / mixed slash-backslash authority tricks', () => { + // Browsers (WHATWG URL) treat backslashes as forward slashes in the + // authority, so these all resolve to evil.com and must be rejected. + expect(isValidReturnUrl('/\\evil.com', origin)).toBe(false); + expect(isValidReturnUrl('\\/evil.com', origin)).toBe(false); + expect(isValidReturnUrl('\\\\evil.com', origin)).toBe(false); + expect(isValidReturnUrl('/\\/evil.com', origin)).toBe(false); + expect(isValidReturnUrl('https:\\\\evil.com', origin)).toBe(false); + }); + + it('treats percent-encoded slashes and single backslash as same-origin paths (not open redirects)', () => { + // %2f stays encoded, so the origin remains the app origin — navigating to + // http://localhost:9000/%2f%2fevil.com, never to evil.com. + expect(isValidReturnUrl('%2f%2fevil.com', origin)).toBe(true); + expect(isValidReturnUrl('/%2f%2fevil.com', origin)).toBe(true); + // A single leading backslash resolves to /evil.com on the app origin. + expect(isValidReturnUrl('\\evil.com', origin)).toBe(true); + }); + + it('returns true for same-origin paths', () => { + expect(isValidReturnUrl('/home', origin)).toBe(true); + expect(isValidReturnUrl('/run/page?foo=bar', origin)).toBe(true); + expect(isValidReturnUrl('/run/page?foo=bar#section', origin)).toBe(true); + expect(isValidReturnUrl('/home?redirect=https://example.com', origin)).toBe( + true + ); + expect(isValidReturnUrl('http://localhost:9000/run/page', origin)).toBe( + true + ); + expect(isValidReturnUrl('#!/home', origin)).toBe(true); + expect(isValidReturnUrl('#!/environments/1/docker/dashboard', origin)).toBe( + true + ); + }); +}); diff --git a/app/portainer/helpers/url-utils.ts b/app/portainer/helpers/url-utils.ts new file mode 100644 index 0000000000..1e38132ce4 --- /dev/null +++ b/app/portainer/helpers/url-utils.ts @@ -0,0 +1,12 @@ +export function isValidReturnUrl( + url: string, + origin = window.location.origin +): boolean { + if (!url) return false; + try { + const parsed = new URL(url, origin); + return parsed.origin === origin; + } catch { + return false; + } +} diff --git a/app/portainer/views/auth/authController.js b/app/portainer/views/auth/authController.js index b2800ed425..0a84f44687 100644 --- a/app/portainer/views/auth/authController.js +++ b/app/portainer/views/auth/authController.js @@ -2,6 +2,8 @@ import angular from 'angular'; import uuidv4 from 'uuid/v4'; import { getEnvironments } from '@/react/portainer/environments/environment.service'; import { dispatchCacheRefreshEvent } from '@/portainer/services/http-request.helper'; +import { isValidReturnUrl } from '@/portainer/helpers/url-utils'; +import { storeReturnUrl, getReturnUrl, cleanReturnUrl } from '@/react/portainer/helpers/returnUrl'; class AuthenticationController { /* @ngInject */ @@ -123,9 +125,16 @@ class AuthenticationController { if (endpoints.value.length === 0 && isAdmin) { return this.$state.go('portainer.wizard'); - } else { - return this.$state.go('portainer.home'); } + + const returnUrl = getReturnUrl(); + cleanReturnUrl(); + if (returnUrl && isValidReturnUrl(returnUrl)) { + this.$window.location.href = returnUrl; + return; + } + + return this.$state.go('portainer.home'); } catch (err) { this.error(err, 'Unable to retrieve environments'); } @@ -220,6 +229,11 @@ class AuthenticationController { this.state.OAuthLoginURI = settings.OAuthLoginURI; this.state.OAuthProvider = this.determineOauthProvider(settings.OAuthLoginURI); + const returnUrl = new URLSearchParams(this.$window.location.search).get('returnUrl'); + if (returnUrl && isValidReturnUrl(returnUrl)) { + storeReturnUrl(returnUrl); + } + const code = this.URLHelper.getParameter('code'); const state = this.URLHelper.getParameter('state'); if (code && state) { diff --git a/app/portainer/views/logout/logoutController.js b/app/portainer/views/logout/logoutController.js index 5369121d6b..51b35fb07c 100644 --- a/app/portainer/views/logout/logoutController.js +++ b/app/portainer/views/logout/logoutController.js @@ -1,5 +1,6 @@ import angular from 'angular'; import { dispatchCacheRefreshEvent } from '@/portainer/services/http-request.helper'; +import { cleanReturnUrl } from '@/react/portainer/helpers/returnUrl'; class LogoutController { /* @ngInject */ @@ -34,6 +35,7 @@ class LogoutController { // always clear the kubernetes cache dispatchCacheRefreshEvent(); + cleanReturnUrl(); this.LocalStorage.storeLogoutReason(error); if (settings.OAuthLogoutURI && this.Authentication.getUserDetails().ID !== 1) { this.$window.location.href = settings.OAuthLogoutURI; diff --git a/app/react/portainer/helpers/returnUrl.ts b/app/react/portainer/helpers/returnUrl.ts new file mode 100644 index 0000000000..002af83241 --- /dev/null +++ b/app/react/portainer/helpers/returnUrl.ts @@ -0,0 +1,15 @@ +import { get, set, keyBuilder } from '@/react/hooks/useLocalStorage'; + +const KEY = 'RETURN_URL'; + +export function storeReturnUrl(url: string): void { + set(KEY, url); +} + +export function getReturnUrl(): string | null { + return get(KEY, null); +} + +export function cleanReturnUrl(): void { + localStorage.removeItem(keyBuilder(KEY)); +}