From 13b4fca9dd4c0f7e6a036288077ee7fc065ea33b Mon Sep 17 00:00:00 2001 From: Wojtek Bazant Date: Fri, 22 Nov 2024 16:49:41 +0000 Subject: [PATCH] Changes to auth code --- src/components/auth/ConfirmationPage.js | 7 +- src/components/auth/ConfirmationResendPage.js | 8 +- src/components/auth/LoginPage.js | 5 - src/components/auth/PasswordResetPage.js | 8 +- src/components/auth/PasswordSetPage.js | 8 +- src/components/auth/SignupPage.js | 5 +- src/components/form/ReportModal.js | 4 +- src/components/map/ConnectGeolocation.js | 2 +- src/redux/authSlice.js | 126 +++++++++--------- src/redux/geolocationSlice.js | 2 +- src/utils/api.ts | 31 +++-- src/utils/authStore.js | 38 +++++- 12 files changed, 136 insertions(+), 108 deletions(-) diff --git a/src/components/auth/ConfirmationPage.js b/src/components/auth/ConfirmationPage.js index 01eb5e7c6..92eca328b 100644 --- a/src/components/auth/ConfirmationPage.js +++ b/src/components/auth/ConfirmationPage.js @@ -21,8 +21,11 @@ const ConfirmationPage = () => { const { email } = await confirmUser(token) toast.success(t('devise.confirmations.confirmed')) history.push({ pathname: '/users/sign_in', state: { email } }) - } catch (e) { - toast.error(e.response?.data.error, { autoClose: 5000 }) + } catch (error) { + toast.error( + `Account confirmation failed: ${error.message || 'Unknown error'}`, + { autoClose: 5000 }, + ) history.push('/users/confirmation/new') } } diff --git a/src/components/auth/ConfirmationResendPage.js b/src/components/auth/ConfirmationResendPage.js index d0a848274..27ba0b0ea 100644 --- a/src/components/auth/ConfirmationResendPage.js +++ b/src/components/auth/ConfirmationResendPage.js @@ -30,10 +30,10 @@ const ConfirmationResendPage = () => { autoClose: 5000, }) history.push('/users/sign_in') - } catch (e) { - // Should not happen since API silently accepts any email - toast.error(e.response?.data.error) - console.error(e.response) + } catch (error) { + toast.error( + `Resending confirmation failed: ${error.message || 'Unknown error'}`, + ) recaptchaRef.current.reset() } } diff --git a/src/components/auth/LoginPage.js b/src/components/auth/LoginPage.js index 54f9e3b90..d5da0c63f 100644 --- a/src/components/auth/LoginPage.js +++ b/src/components/auth/LoginPage.js @@ -12,7 +12,6 @@ import Button from '../ui/Button' import LabeledRow from '../ui/LabeledRow' import { Column, - ErrorMessage, FormButtonWrapper, FormCheckboxWrapper, FormInputWrapper, @@ -20,7 +19,6 @@ import { const LoginPage = () => { const { user, isLoading } = useSelector((state) => state.auth) - const error = useSelector((state) => state.auth.error) const { state, search } = useLocation() const { t } = useTranslation() const params = new URLSearchParams(search) @@ -62,9 +60,6 @@ const LoginPage = () => { label={t('glossary.password')} /> - {dirty && error && ( - {error.response.data.error} - )} { autoClose: 5000, }) history.push('/users/sign_in') - } catch (e) { - // Should not happen since API silently accepts any email - toast.error(e.response?.data?.error) - console.error(e.response) + } catch (error) { + toast.error( + `Resetting password failed: ${error.message || 'Unknown error'}`, + ) recaptchaRef.current.reset() } } diff --git a/src/components/auth/PasswordSetPage.js b/src/components/auth/PasswordSetPage.js index 2a2c19ba6..1936d346c 100644 --- a/src/components/auth/PasswordSetPage.js +++ b/src/components/auth/PasswordSetPage.js @@ -29,7 +29,6 @@ const PasswordSetPage = () => { useEffect(() => { if (!getResetToken()) { - // TODO: display this (among other toasts) on login page in permanent red error text rather than toast toast.error(t('devise.passwords.no_token'), { autoClose: 5000 }) history.push('/users/sign_in') } @@ -49,9 +48,10 @@ const PasswordSetPage = () => { autoClose: 5000, }) history.push({ pathname: '/users/sign_in', state: { email } }) - } catch (e) { - toast.error(e.response?.data.error) - console.error(e.response) + } catch (error) { + toast.error( + `Setting new password failed: ${error.message || 'Unknown error'}`, + ) history.push('/users/sign_in') } } diff --git a/src/components/auth/SignupPage.js b/src/components/auth/SignupPage.js index 47c38f734..1a11b6780 100644 --- a/src/components/auth/SignupPage.js +++ b/src/components/auth/SignupPage.js @@ -46,9 +46,8 @@ const SignupPage = () => { }, ) history.push('/map') - } catch (e) { - toast.error(e.response?.data?.error) - console.error(e.response) + } catch (error) { + toast.error(`Sign up failed: ${error.message || 'Unknown error'}`) recaptchaRef.current.reset() } } diff --git a/src/components/form/ReportModal.js b/src/components/form/ReportModal.js index fdb5fc2c1..b68087cff 100644 --- a/src/components/form/ReportModal.js +++ b/src/components/form/ReportModal.js @@ -29,8 +29,8 @@ const ReportModal = ({ locationId, name, onDismiss, ...props }) => { try { response = await addReport(reportValues) toast.success('Report submitted successfully!') - } catch (e) { - toast.error(`Report submission failed: ${e.message}`) + } catch (error) { + toast.error(`Report submission failed: ${error.message}`) } if (response && !response.error) { diff --git a/src/components/map/ConnectGeolocation.js b/src/components/map/ConnectGeolocation.js index 719c4af8f..577a7649b 100644 --- a/src/components/map/ConnectGeolocation.js +++ b/src/components/map/ConnectGeolocation.js @@ -73,7 +73,7 @@ export const ConnectGeolocation = () => { } else if (geolocation.error) { dispatch(geolocationError(geolocation.error)) } else if (!geolocation.latitude || !geolocation.longitude) { - dispatch(geolocationError({ message: 'Unknown Error' })) + dispatch(geolocationError({ message: 'Unknown error' })) } else if (isMapMoving) { // Do nothing } else { diff --git a/src/redux/authSlice.js b/src/redux/authSlice.js index 0d9401377..faee4d41b 100644 --- a/src/redux/authSlice.js +++ b/src/redux/authSlice.js @@ -1,88 +1,91 @@ -import { createAction, createAsyncThunk, createSlice } from '@reduxjs/toolkit' +import { createAsyncThunk, createSlice } from '@reduxjs/toolkit' import i18next from 'i18next' import { toast } from 'react-toastify' -import { editUser, getUser, getUserToken } from '../utils/api' +import { editUser, getUser, getUserToken, refreshUserToken } from '../utils/api' import authStore from '../utils/authStore' -export const checkAuth = createAsyncThunk( - 'auth/checkAuth', - async (_data, { rejectWithValue }) => { - let token - try { - token = authStore.getToken() - } catch (err) { - return rejectWithValue(err) - } +export const checkAuth = createAsyncThunk('auth/checkAuth', async (_data) => { + const token = authStore.initFromStorage() + + if (!token?.access_token || !token?.refresh_token) { + return null + } - if (token?.access_token) { - return await getUser() - } else { - return null + try { + const user = await getUser(token.access_token) + authStore.setToken(token) + return user + } catch (error) { + if ( + error.response?.status === 401 && + error.response?.data?.error === 'Expired access token' + ) { + let newToken + try { + newToken = await refreshUserToken(token.refresh_token) + } catch (refreshError) { + authStore.removeToken() + throw refreshError + } + const user = await getUser(newToken.access_token) + authStore.setToken(newToken) + return user + } else if (error.response?.status === 401) { + // We failed to log in with our token but can't fix the error based on response + authStore.removeToken() } - }, -) + throw error + } +}) export const login = createAsyncThunk( 'auth/login', - async ({ email, password, rememberMe }, { rejectWithValue }) => { - let token - try { - token = await getUserToken(email, password) - } catch (err) { - return rejectWithValue(err) - } - authStore.setToken(token, rememberMe) - - return await getUser() + async ({ email, password, rememberMe }) => { + const token = await getUserToken(email, password) + const user = await getUser(token.access_token) + authStore.setRememberMe(rememberMe) + authStore.setToken(token) + return user }, ) export const editProfile = createAsyncThunk( 'auth/editProfile', - async (userData, { rejectWithValue, getState }) => { - try { - const currentUser = getState().auth.user - const isEmailChanged = userData.email !== currentUser.email - const response = await editUser({ ...userData, range: currentUser.range }) - return { response, isEmailChanged } - } catch (err) { - return rejectWithValue(err.response?.data?.error || err.message) - } + async (userData, { getState }) => { + const currentUser = getState().auth.user + const isEmailChanged = userData.email !== currentUser.email + const response = await editUser({ ...userData, range: currentUser.range }) + return { response, isEmailChanged } }, ) -export const logout = createAction('auth/logout', () => { - authStore.removeToken() - return {} -}) - -const initialState = { - user: null, - error: null, - isLoading: true, - token: null, -} - export const authSlice = createSlice({ name: 'auth', + initialState: { + user: null, + isLoading: true, + }, reducers: { - setToken: (state, action) => { - state.token = action.payload + logout: (state) => { + authStore.removeToken() + state.user = null }, }, - initialState, extraReducers: { [checkAuth.pending]: (state) => { state.isLoading = true }, [checkAuth.fulfilled]: (state, action) => { state.user = action.payload - state.error = null state.isLoading = false }, [checkAuth.rejected]: (state, action) => { - state.error = action.payload + toast.error( + `Authentication check failed: ${ + action.error.message || 'Unknown error' + }`, + ) state.isLoading = false }, @@ -91,25 +94,22 @@ export const authSlice = createSlice({ }, [login.fulfilled]: (state, action) => { state.user = action.payload - state.error = null state.isLoading = false }, [login.rejected]: (state, action) => { - state.error = action.payload + toast.error( + `Sign in failed: ${action.error.message || 'Unknown error'}`, + { autoClose: 5000 }, + ) state.isLoading = false }, - [logout]: (state) => { - state.user = null - }, - [editProfile.pending]: (state) => { state.isLoading = true }, [editProfile.fulfilled]: (state, action) => { const { response, isEmailChanged } = action.payload state.user = response - state.error = null state.isLoading = false if (isEmailChanged && response.unconfirmed_email) { @@ -121,11 +121,13 @@ export const authSlice = createSlice({ } }, [editProfile.rejected]: (state, action) => { - state.error = action.payload + toast.error( + `Profile update failed: ${action.error.message || 'Unknown error'}`, + ) state.isLoading = false }, }, }) -export const { setToken } = authSlice.actions +export const { logout } = authSlice.actions export default authSlice.reducer diff --git a/src/redux/geolocationSlice.js b/src/redux/geolocationSlice.js index deb1be5c6..4f55a9c09 100644 --- a/src/redux/geolocationSlice.js +++ b/src/redux/geolocationSlice.js @@ -70,7 +70,7 @@ export const geolocationSlice = createSlice({ // @see src/components/map/ConnectedGeolocation.js state.geolocationState = GeolocationState.INITIAL toast.error( - `Geolocation failed: ${action.payload.message || 'Unknown Error'}`, + `Geolocation failed: ${action.payload.message || 'Unknown error'}`, ) break } diff --git a/src/utils/api.ts b/src/utils/api.ts index 44c3c3d68..0745f0bbd 100644 --- a/src/utils/api.ts +++ b/src/utils/api.ts @@ -1,5 +1,3 @@ -/* eslint-disable no-console */ - import axios from 'axios' import { matchPath } from 'react-router' @@ -36,9 +34,11 @@ instance.interceptors.request.use((config) => { config.method === 'get' && config.url && matchPath(config.url, { path: anonymousGetUrls }) - const token = authStore.getToken() - if (token && !isAnonymous) { - config.headers.Authorization = `Bearer ${token.access_token}` + + const accessToken = authStore.getAccessToken() + + if (accessToken && !isAnonymous) { + config.headers.Authorization = `Bearer ${accessToken}` } return config @@ -54,19 +54,22 @@ instance.interceptors.response.use( error.response.data.error === 'Expired access token' && !originalRequest._retry ) { - const token: any = authStore.getToken() + const refreshToken = authStore.getRefreshToken() - if (token) { + if (refreshToken) { originalRequest._retry = true - const newToken = await refreshUserToken(token.refresh_token) - authStore.setToken(newToken, token.rememberMe) + const newToken = await refreshUserToken(refreshToken) + authStore.setToken(newToken) return instance(originalRequest) } } - - throw error + if (error?.response?.data?.error) { + throw { ...error, message: error.response.data.error } + } else { + throw error + } }, ) @@ -78,7 +81,10 @@ export const editUser = ( data: paths['/user']['put']['requestBody']['content']['application/json'], ) => instance.put('/user', data) -export const getUser = () => instance.get('/user') +export const getUser = (accessToken: string) => + instance.get('/user', { + headers: { Authorization: `Bearer ${accessToken}` }, + }) export const deleteUser = () => instance.delete('/user') @@ -104,7 +110,6 @@ export const getUserToken = (username: string, password: string) => { export const refreshUserToken = (refreshToken: string) => { const formData = new FormData() formData.append('refresh_token', refreshToken) - return instance.post('/user/token/refresh', formData) } diff --git a/src/utils/authStore.js b/src/utils/authStore.js index c49a2e111..150e8944c 100644 --- a/src/utils/authStore.js +++ b/src/utils/authStore.js @@ -1,37 +1,61 @@ const ACCESS_TOKEN_KEY = 'authToken' const REFRESH_TOKEN_KEY = 'refreshToken' +let accessToken = null +let refreshToken = null +let rememberMe = null + const authStore = { - getToken: () => { + initFromStorage: () => { if (localStorage.getItem(ACCESS_TOKEN_KEY)) { + rememberMe = true return { access_token: localStorage.getItem(ACCESS_TOKEN_KEY), refresh_token: localStorage.getItem(REFRESH_TOKEN_KEY), - rememberMe: true, } } else if (sessionStorage.getItem(ACCESS_TOKEN_KEY)) { + rememberMe = false return { access_token: sessionStorage.getItem(ACCESS_TOKEN_KEY), refresh_token: sessionStorage.getItem(REFRESH_TOKEN_KEY), - rememberMe: false, } } else { return null } }, - setToken: (token, rememberMe) => { + getAccessToken: () => accessToken, + + getRefreshToken: () => refreshToken, + + setRememberMe: (remember) => { + rememberMe = remember + }, + + setToken: (token) => { const storage = rememberMe ? localStorage : sessionStorage + accessToken = token.access_token + refreshToken = token.refresh_token + storage.setItem(ACCESS_TOKEN_KEY, token.access_token) storage.setItem(REFRESH_TOKEN_KEY, token.refresh_token) }, removeToken: () => { - for (const storage of [localStorage, sessionStorage]) { - storage.removeItem(ACCESS_TOKEN_KEY) - storage.removeItem(REFRESH_TOKEN_KEY) + if (rememberMe) { + localStorage.removeItem(ACCESS_TOKEN_KEY) + localStorage.removeItem(REFRESH_TOKEN_KEY) + sessionStorage.removeItem(ACCESS_TOKEN_KEY) + sessionStorage.removeItem(REFRESH_TOKEN_KEY) + } else { + sessionStorage.removeItem(ACCESS_TOKEN_KEY) + sessionStorage.removeItem(REFRESH_TOKEN_KEY) } + + accessToken = null + refreshToken = null + rememberMe = null }, }