From 9e60723e4de388f81bc05d0b2e3c619f00068457 Mon Sep 17 00:00:00 2001 From: LP B Date: Fri, 27 Oct 2023 14:44:05 +0200 Subject: [PATCH] fix(app/logout): always perform API logout + make API logout route public [EE-6198] (#10448) * feat(api/logout): make logout route public * feat(app/logout): always perform API logout on /logout redirect * fix(app): send a logout event to AngularJS when axios hits a 401 --- api/http/handler/auth/handler.go | 2 +- api/http/handler/auth/logout.go | 13 ++++++---- api/http/security/bouncer.go | 18 +++++++------- app/portainer/__module.js | 24 +++++++++++++++---- app/portainer/services/authentication.js | 8 +++---- app/portainer/services/axios.ts | 12 ++++++++++ .../views/logout/logoutController.js | 3 +-- app/react/components/PageHeader/UserMenu.tsx | 1 - 8 files changed, 54 insertions(+), 27 deletions(-) diff --git a/api/http/handler/auth/handler.go b/api/http/handler/auth/handler.go index 877b55ad9..8d7d577dd 100644 --- a/api/http/handler/auth/handler.go +++ b/api/http/handler/auth/handler.go @@ -38,7 +38,7 @@ func NewHandler(bouncer security.BouncerService, rateLimiter *security.RateLimit h.Handle("/auth", rateLimiter.LimitAccess(bouncer.PublicAccess(httperror.LoggerHandler(h.authenticate)))).Methods(http.MethodPost) h.Handle("/auth/logout", - bouncer.AuthenticatedAccess(httperror.LoggerHandler(h.logout))).Methods(http.MethodPost) + bouncer.PublicAccess(httperror.LoggerHandler(h.logout))).Methods(http.MethodPost) return h } diff --git a/api/http/handler/auth/logout.go b/api/http/handler/auth/logout.go index dc4088259..87099eadf 100644 --- a/api/http/handler/auth/logout.go +++ b/api/http/handler/auth/logout.go @@ -7,26 +7,29 @@ import ( "github.com/portainer/portainer/api/internal/logoutcontext" httperror "github.com/portainer/portainer/pkg/libhttp/error" "github.com/portainer/portainer/pkg/libhttp/response" + "github.com/rs/zerolog/log" ) // @id Logout // @summary Logout -// @description **Access policy**: authenticated +// @description **Access policy**: public // @security ApiKeyAuth // @security jwt // @tags auth // @success 204 "Success" // @failure 500 "Server error" // @router /auth/logout [post] + func (handler *Handler) logout(w http.ResponseWriter, r *http.Request) *httperror.HandlerError { tokenData, err := security.RetrieveTokenData(r) if err != nil { - return httperror.InternalServerError("Unable to retrieve user details from authentication token", err) + log.Warn().Err(err).Msg("unable to retrieve user details from authentication token") } - handler.KubernetesTokenCacheManager.RemoveUserFromCache(tokenData.ID) - - logoutcontext.Cancel(tokenData.Token) + if tokenData != nil { + handler.KubernetesTokenCacheManager.RemoveUserFromCache(tokenData.ID) + logoutcontext.Cancel(tokenData.Token) + } return response.Empty(w) } diff --git a/api/http/security/bouncer.go b/api/http/security/bouncer.go index 85ef5f68f..36c8eeada 100644 --- a/api/http/security/bouncer.go +++ b/api/http/security/bouncer.go @@ -60,15 +60,15 @@ func NewRequestBouncer(dataStore dataservices.DataStore, jwtService dataservices } } -// PublicAccess defines a security check for public API environments(endpoints). -// No authentication is required to access these environments(endpoints). +// PublicAccess defines a security check for public API endpoints. +// No authentication is required to access these endpoints. func (bouncer *RequestBouncer) PublicAccess(h http.Handler) http.Handler { return mwSecureHeaders(h) } -// AdminAccess defines a security check for API environments(endpoints) that require an authorization check. -// Authentication is required to access these environments(endpoints). -// The administrator role is required to use these environments(endpoints). +// AdminAccess defines a security check for API endpoints that require an authorization check. +// Authentication is required to access these endpoints. +// The administrator role is required to use these endpoints. // The request context will be enhanced with a RestrictedRequestContext object // that might be used later to inside the API operation for extra authorization validation // and resource filtering. @@ -79,8 +79,8 @@ func (bouncer *RequestBouncer) AdminAccess(h http.Handler) http.Handler { return h } -// RestrictedAccess defines a security check for restricted API environments(endpoints). -// Authentication is required to access these environments(endpoints). +// RestrictedAccess defines a security check for restricted API endpoints. +// Authentication is required to access these endpoints. // The request context will be enhanced with a RestrictedRequestContext object // that might be used later to inside the API operation for extra authorization validation // and resource filtering. @@ -104,8 +104,8 @@ func (bouncer *RequestBouncer) TeamLeaderAccess(h http.Handler) http.Handler { return h } -// AuthenticatedAccess defines a security check for restricted API environments(endpoints). -// Authentication is required to access these environments(endpoints). +// AuthenticatedAccess defines a security check for restricted API endpoints. +// Authentication is required to access these endpoints. // The request context will be enhanced with a RestrictedRequestContext object // that might be used later to inside the API operation for extra authorization validation // and resource filtering. diff --git a/app/portainer/__module.js b/app/portainer/__module.js index 5a69c84b1..a00d3e84e 100644 --- a/app/portainer/__module.js +++ b/app/portainer/__module.js @@ -12,18 +12,33 @@ import { reactModule } from './react'; import { sidebarModule } from './react/views/sidebar'; import environmentsModule from './environments'; import { helpersModule } from './helpers'; +import { AXIOS_UNAUTHENTICATED } from './services/axios'; async function initAuthentication(authManager, Authentication, $rootScope, $state) { authManager.checkAuthOnRefresh(); + + function handleUnauthenticated(data, performReload) { + if (!_.includes(data.config.url, '/v2/') && !_.includes(data.config.url, '/api/v4/') && isTransitionRequiresAuthentication($state.transition)) { + $state.go('portainer.logout', { error: 'Your session has expired' }); + if (performReload) { + window.location.reload(); + } + } + } + // The unauthenticated event is broadcasted by the jwtInterceptor when // hitting a 401. We're using this instead of the usual combination of // authManager.redirectWhenUnauthenticated() + unauthenticatedRedirector // to have more controls on which URL should trigger the unauthenticated state. $rootScope.$on('unauthenticated', function (event, data) { - if (!_.includes(data.config.url, '/v2/') && !_.includes(data.config.url, '/api/v4/') && isTransitionRequiresAuthentication($state.transition)) { - $state.go('portainer.logout', { error: 'Your session has expired' }); - window.location.reload(); - } + handleUnauthenticated(data, true); + }); + + // the AXIOS_UNAUTHENTICATED event is emitted by axios when a request returns with a 401 code + // the event contains the entire AxiosError in detail.err + window.addEventListener(AXIOS_UNAUTHENTICATED, (event) => { + const data = event.detail.err; + handleUnauthenticated(data); }); return await Authentication.init(); @@ -157,7 +172,6 @@ angular url: '/logout', params: { error: '', - performApiLogout: true, }, views: { 'content@': { diff --git a/app/portainer/services/authentication.js b/app/portainer/services/authentication.js index 9a182f84d..df7d990e0 100644 --- a/app/portainer/services/authentication.js +++ b/app/portainer/services/authentication.js @@ -40,8 +40,8 @@ angular.module('portainer.app').factory('Authentication', [ } } - async function logoutAsync(performApiLogout) { - if (performApiLogout && isAuthenticated()) { + async function logoutAsync() { + if (isAuthenticated()) { await Auth.logout().$promise; } @@ -53,8 +53,8 @@ angular.module('portainer.app').factory('Authentication', [ tryAutoLoginExtension(); } - function logout(performApiLogout) { - return $async(logoutAsync, performApiLogout); + function logout() { + return $async(logoutAsync); } function init() { diff --git a/app/portainer/services/axios.ts b/app/portainer/services/axios.ts index 0f4cbee36..ae4ac3696 100644 --- a/app/portainer/services/axios.ts +++ b/app/portainer/services/axios.ts @@ -49,6 +49,8 @@ export function agentInterceptor(config: AxiosRequestConfig) { axios.interceptors.request.use(agentInterceptor); +export const AXIOS_UNAUTHENTICATED = '__axios__unauthenticated__'; + /** * Parses an Axios error and returns a PortainerError. * @param err The original error. @@ -72,6 +74,16 @@ export function parseAxiosError( } else { resultMsg = msg || details; } + // dispatch an event for unauthorized errors that AngularJS can catch + if (err.response?.status === 401) { + dispatchEvent( + new CustomEvent(AXIOS_UNAUTHENTICATED, { + detail: { + err, + }, + }) + ); + } } return new PortainerError(resultMsg, resultErr); diff --git a/app/portainer/views/logout/logoutController.js b/app/portainer/views/logout/logoutController.js index 816ab87b7..244081c07 100644 --- a/app/portainer/views/logout/logoutController.js +++ b/app/portainer/views/logout/logoutController.js @@ -25,11 +25,10 @@ class LogoutController { */ async logoutAsync() { const error = this.$transition$.params().error; - const performApiLogout = this.$transition$.params().performApiLogout; const settings = await this.SettingsService.publicSettings(); try { - await this.Authentication.logout(performApiLogout); + await this.Authentication.logout(); } finally { this.LocalStorage.storeLogoutReason(error); if (settings.OAuthLogoutURI && this.Authentication.getUserDetails().ID !== 1) { diff --git a/app/react/components/PageHeader/UserMenu.tsx b/app/react/components/PageHeader/UserMenu.tsx index 0e7fe48ef..2a814cb2e 100644 --- a/app/react/components/PageHeader/UserMenu.tsx +++ b/app/react/components/PageHeader/UserMenu.tsx @@ -56,7 +56,6 @@ export function UserMenu() { to="portainer.logout" label="Log out" data-cy="userMenu-logOut" - params={{ performApiLogout: true }} />