From 18207bcfbd168a79211e317b9a5c3ab78edb8fd3 Mon Sep 17 00:00:00 2001 From: jasquat <2487833+jasquat@users.noreply.github.com> Date: Tue, 14 Nov 2023 14:54:16 -0500 Subject: [PATCH] bugfix/fix-refresh-token-error (#668) * when backend returns 401 also remove cookies and redirect in frontend if cookies are not set w/ burnettk * added a copule helpful comments w/ burnettk --------- Co-authored-by: jasquat --- .../routes/authentication_controller.py | 13 +++++++--- .../services/authentication_service.py | 6 +++++ spiffworkflow-frontend/src/routes/Login.tsx | 25 +++++++++++++------ .../src/services/HttpService.ts | 7 ++++++ 4 files changed, 39 insertions(+), 12 deletions(-) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/routes/authentication_controller.py b/spiffworkflow-backend/src/spiffworkflow_backend/routes/authentication_controller.py index 79ff9ef4..ce791ef0 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/authentication_controller.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/authentication_controller.py @@ -152,6 +152,7 @@ def login_return(code: str, state: str, session_state: str = "") -> Response | N tld.new_authentication_identifier = authentication_identifier return redirect(redirect_url) + # we normally clear cookies on 401, but there is a high chance you do not have any yet in this case raise ApiError( error_code="invalid_login", message="Login failed. Please try again", @@ -159,6 +160,7 @@ def login_return(code: str, state: str, session_state: str = "") -> Response | N ) else: + # we normally clear cookies on 401, but there is a high chance you do not have any yet in this case current_app.logger.error(f"id_token not found in payload from provider: {auth_token_object}") raise ApiError( error_code="invalid_token", @@ -208,8 +210,7 @@ def login_api_return(code: str, state: str, session_state: str) -> str: def logout(id_token: str, authentication_identifier: str, redirect_url: str | None) -> Response: if redirect_url is None: redirect_url = "" - tld = current_app.config["THREAD_LOCAL_DATA"] - tld.user_has_logged_out = True + AuthenticationService.set_user_has_logged_out() return AuthenticationService().logout( redirect_url=redirect_url, id_token=id_token, authentication_identifier=authentication_identifier ) @@ -296,8 +297,7 @@ def _force_logout_user_if_necessary(user_model: UserModel | None = None) -> bool and user_model.service_id == "spiff_guest_service_id" and not AuthorizationService.request_allows_guest_access() ): - tld = current_app.config["THREAD_LOCAL_DATA"] - tld.user_has_logged_out = True + AuthenticationService.set_user_has_logged_out() return True return False @@ -377,6 +377,7 @@ def _get_user_model_from_token(token: str) -> UserModel | None: } if user_info is None: + AuthenticationService.set_user_has_logged_out() raise ApiError( error_code="invalid_token", message="Your token is expired. Please Login", @@ -384,6 +385,7 @@ def _get_user_model_from_token(token: str) -> UserModel | None: ) from token_expired_error except Exception as e: + AuthenticationService.set_user_has_logged_out() raise ApiError( error_code="fail_get_user_info", message="Cannot get user info from token", @@ -396,6 +398,7 @@ def _get_user_model_from_token(token: str) -> UserModel | None: .first() ) if user_model is None: + AuthenticationService.set_user_has_logged_out() raise ApiError( error_code="invalid_user", message="Invalid user. Please log in.", @@ -403,6 +406,7 @@ def _get_user_model_from_token(token: str) -> UserModel | None: ) # no user_info else: + AuthenticationService.set_user_has_logged_out() raise ApiError( error_code="no_user_info", message="Cannot retrieve user info", @@ -411,6 +415,7 @@ def _get_user_model_from_token(token: str) -> UserModel | None: else: current_app.logger.debug("token_type not in decode_token in verify_token") + AuthenticationService.set_user_has_logged_out() raise ApiError( error_code="invalid_token", message="Invalid token. Please log in.", diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/authentication_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/authentication_service.py index 7be9ced7..7f04e474 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/authentication_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/authentication_service.py @@ -230,6 +230,7 @@ class AuthenticationService: valid = False if valid and now > decoded_token["exp"]: + AuthenticationService.set_user_has_logged_out() raise TokenExpiredError("Your token is expired. Please Login") elif not valid: current_app.logger.error( @@ -353,3 +354,8 @@ class AuthenticationService: tld.new_access_token = g.token tld.new_id_token = g.token tld.new_authentication_identifier = authentication_identifier + + @classmethod + def set_user_has_logged_out(cls) -> None: + tld = current_app.config["THREAD_LOCAL_DATA"] + tld.user_has_logged_out = True diff --git a/spiffworkflow-frontend/src/routes/Login.tsx b/spiffworkflow-frontend/src/routes/Login.tsx index 8d0ca3a0..0a1e2a61 100644 --- a/spiffworkflow-frontend/src/routes/Login.tsx +++ b/spiffworkflow-frontend/src/routes/Login.tsx @@ -1,12 +1,13 @@ import { ArrowRight } from '@carbon/icons-react'; import { useEffect, useState } from 'react'; import { Loading, Button, Grid, Column } from '@carbon/react'; -import { useSearchParams } from 'react-router-dom'; +import { useNavigate, useSearchParams } from 'react-router-dom'; import { AuthenticationOption } from '../interfaces'; import HttpService from '../services/HttpService'; import UserService from '../services/UserService'; export default function Login() { + const navigate = useNavigate(); const [searchParams] = useSearchParams(); const [authenticationOptions, setAuthenticationOptions] = useState< AuthenticationOption[] | null @@ -18,6 +19,14 @@ export default function Login() { }); }, []); + const getOriginalUrl = () => { + const originalUrl = searchParams.get('original_url'); + if (originalUrl === '/login') { + return '/'; + } + return originalUrl; + }; + const authenticationOptionButtons = () => { if (!authenticationOptions) { return null; @@ -30,9 +39,7 @@ export default function Login() { size="lg" className="login-button" renderIcon={ArrowRight} - onClick={() => - UserService.doLogin(option, searchParams.get('original_url')) - } + onClick={() => UserService.doLogin(option, getOriginalUrl())} > {option.label} @@ -46,10 +53,7 @@ export default function Login() { return; } if (authenticationOptions.length === 1) { - UserService.doLogin( - authenticationOptions[0], - searchParams.get('original_url') - ); + UserService.doLogin(authenticationOptions[0], getOriginalUrl()); } }; @@ -89,6 +93,11 @@ export default function Login() { ); }; + if (UserService.isLoggedIn()) { + navigate('/'); + return null; + } + if (authenticationOptions === null || authenticationOptions.length < 2) { doLoginIfAppropriate(); return ( diff --git a/spiffworkflow-frontend/src/services/HttpService.ts b/spiffworkflow-frontend/src/services/HttpService.ts index e3e3292b..e553ac63 100644 --- a/spiffworkflow-frontend/src/services/HttpService.ts +++ b/spiffworkflow-frontend/src/services/HttpService.ts @@ -75,6 +75,8 @@ backendCallProps) => { const updatedPath = path.replace(/^\/v1\.0/, ''); let isSuccessful = true; + // this fancy 403 handling is like this because we want to get the response body. + // otherwise, we would just throw an exception. let is403 = false; fetch(`${BACKEND_BASE_URL}${updatedPath}`, httpArgs) .then((response) => { @@ -120,6 +122,11 @@ backendCallProps) => { } else { console.error(error.message); } + } else if ( + !UserService.isLoggedIn() && + window.location.pathname !== '/login' + ) { + window.location.href = `/login?original_url=${UserService.getCurrentLocation()}`; } }); };