From 1b26df45d11a56db663617500b73ccd0ee0e5be1 Mon Sep 17 00:00:00 2001 From: Alberto Chamorro Date: Wed, 1 Feb 2023 07:30:31 +0100 Subject: [PATCH 01/19] feat: add Google Login endpoint --- src/shared/api/endpoints.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/shared/api/endpoints.ts b/src/shared/api/endpoints.ts index ebc529bd0..d463cd4a2 100644 --- a/src/shared/api/endpoints.ts +++ b/src/shared/api/endpoints.ts @@ -15,7 +15,8 @@ const endpoints = { vacationsDays: `${BASE_URL}/api/vacations/days`, vacationsDetails: `${BASE_URL}/api/vacations/details`, workingTime: `${BASE_URL}/api/working-time`, - version: `${BASE_URL}/api/version` + version: `${BASE_URL}/api/version`, + googleLogin: `${BASE_URL}/oauth/login/google` } export default endpoints From f716e9fb1861f17990b10be1a44b5eb73fb0f738 Mon Sep 17 00:00:00 2001 From: Alberto Chamorro Date: Wed, 1 Feb 2023 07:31:44 +0100 Subject: [PATCH 02/19] feat: add 'withCredentials' option to HttpClient and remove HttpOauthInterceptor --- src/shared/AppRoutes.tsx | 20 +--- .../data-access/http-client/http-client.ts | 3 +- .../http-client/http-oauth-interceptor.ts | 96 ------------------- 3 files changed, 4 insertions(+), 115 deletions(-) delete mode 100644 src/shared/data-access/http-client/http-oauth-interceptor.ts diff --git a/src/shared/AppRoutes.tsx b/src/shared/AppRoutes.tsx index 759c489ac..a63a00ef2 100644 --- a/src/shared/AppRoutes.tsx +++ b/src/shared/AppRoutes.tsx @@ -1,20 +1,16 @@ import type { FC } from 'react' -import { lazy, Suspense, useEffect } from 'react' +import { lazy, Suspense } from 'react' import { ActivityFormScreen } from 'modules/binnacle/page/BinnacleMobile/ActivityFormScreen' import { LazyBinnaclePage } from 'modules/binnacle/page/BinnaclePage.lazy' import { LazyLoginPage } from 'modules/login/page/LoginPage.lazy' import { LazySettingsPage } from 'modules/settings/page/SettingsPage.lazy' import { LazyVacationsPage } from 'modules/vacations/page/VacationsPage.lazy' -import { Route, Routes, useNavigate } from 'react-router-dom' +import { Route, Routes } from 'react-router-dom' import FullPageLoadingSpinner from 'shared/components/FullPageLoadingSpinner' import { Navbar } from 'shared/components/Navbar/Navbar' import { useIsMobile } from 'shared/hooks' import { rawPaths } from './router/paths' import { RequireAuth } from 'shared/router/RequireAuth' -import { useAction } from 'shared/arch/hooks/use-action' -import { LogoutAction } from 'modules/login/data-access/actions/logout-action' -import { container } from 'tsyringe' -import { HttpOAuthInterceptor } from 'shared/data-access/http-client/http-oauth-interceptor' const LazyBinnacleDesktop = lazy( () => @@ -33,18 +29,6 @@ const LazyBinnacleMobile = lazy( export const AppRoutes: FC = () => { const isMobile = useIsMobile() - const logout = useAction(LogoutAction) - const navigate = useNavigate() - - useEffect(() => { - const redirectToLogin = async () => { - await logout() - navigate('/') - } - - container.resolve(HttpOAuthInterceptor).initInterceptor(redirectToLogin) - }, [logout, navigate]) - return ( <> diff --git a/src/shared/data-access/http-client/http-client.ts b/src/shared/data-access/http-client/http-client.ts index e11baa6e5..d44c9739d 100644 --- a/src/shared/data-access/http-client/http-client.ts +++ b/src/shared/data-access/http-client/http-client.ts @@ -8,7 +8,8 @@ export class HttpClient { constructor() { this.httpInstance = axios.create({ - timeout: 10_000 + timeout: 10_000, + withCredentials: true }) } diff --git a/src/shared/data-access/http-client/http-oauth-interceptor.ts b/src/shared/data-access/http-client/http-oauth-interceptor.ts deleted file mode 100644 index 71ae48f71..000000000 --- a/src/shared/data-access/http-client/http-oauth-interceptor.ts +++ /dev/null @@ -1,96 +0,0 @@ -import { inject, singleton } from 'tsyringe' -import axios, { AxiosError, AxiosRequestConfig } from 'axios' -import endpoints from 'shared/api/endpoints' -import { HttpClient } from 'shared/data-access/http-client/http-client' -import { OAuthRepository } from 'shared/api/oauth/oauth-repository' -import type { TokenStorage } from 'shared/api/oauth/token-storage/token-storage' -import type { OAuth } from 'shared/api/oauth/OAuth' - -@singleton() -export class HttpOAuthInterceptor { - refreshTokenPromise?: Promise = undefined - sessionExpiredCb: () => Promise - - constructor( - private httpClient: HttpClient, - private authRepository: OAuthRepository, - @inject('TokenStorage') private tokenStorage: TokenStorage - ) {} - - initInterceptor = (sessionExpiredCb: () => Promise) => { - this.httpClient.httpInstance.interceptors.request.use(this.interceptRequest, (error) => { - Promise.reject(error) - }) - - this.httpClient.httpInstance.interceptors.response.use( - (response) => response, - this.interceptResponseError - ) - - this.sessionExpiredCb = sessionExpiredCb - } - - interceptRequest = async (config: AxiosRequestConfig) => { - const isNotOAuthURL = config.url !== endpoints.auth - - const accessToken = this.tokenStorage.getAccessToken() - - if (accessToken && isNotOAuthURL) { - config.headers = { - ...config.headers, - Authorization: `Bearer ${accessToken}` - } - } - - return config - } - - // https://github.com/waltergar/react-CA/blob/5fb4bd64a8e5f2c276d14a89fe317db0b743983c/src/utils/api/axios.js - interceptResponseError = async (error: AxiosError) => { - const originalRequest: AxiosRequestConfig & { _retry?: boolean } = error.config - const isAccessTokenExpired = - /access token expired/gi.test(error.response?.data.error_description) || - error.response?.status === 403 - - if (error.response && isAccessTokenExpired && !originalRequest._retry) { - try { - originalRequest._retry = true - const response = await this.refreshToken() - axios.defaults.headers.common['Authorization'] = 'Bearer ' + response.access_token - return this.httpClient.httpInstance(originalRequest) - } catch (e) { - return Promise.reject(e) - } - } - return Promise.reject(error) - } - - async refreshToken() { - if (this.refreshTokenPromise !== undefined) { - return this.refreshTokenPromise - } else { - const refreshToken = await this.tokenStorage.getRefreshToken() - this.refreshTokenPromise = this.authRepository - .renewOAuthByRefreshToken(refreshToken!) - .then(async (response) => { - await this.tokenStorage.setRefreshToken(response.refresh_token) - this.tokenStorage.setAccessToken(response.access_token) - this.refreshTokenPromise = undefined - return response - }) - .catch((e) => { - this.refreshTokenPromise = undefined - const isRefreshTokenExpired = /refresh token \(expired\)/gi.test( - e.response?.data.error_description - ) - if (isRefreshTokenExpired) { - this.sessionExpiredCb() - return Promise.reject(e) - } - throw e - }) - - return this.refreshTokenPromise - } - } -} From bb92a2d0bbd474f7a445f832709488eed7c6b14d Mon Sep 17 00:00:00 2001 From: Alberto Chamorro Date: Wed, 1 Feb 2023 07:32:44 +0100 Subject: [PATCH 03/19] feat: add SignInWithGoogle button and change welcome message from LoginPage --- .../SignInWithGoogle/GoogleIcon.tsx | 10 ++++++++ .../SignInWithGoogleButton.tsx | 24 +++++++++++++++++++ src/shared/i18n/en.json | 5 ++-- src/shared/i18n/es.json | 5 ++-- 4 files changed, 38 insertions(+), 6 deletions(-) create mode 100644 src/modules/login/components/SignInWithGoogle/GoogleIcon.tsx create mode 100644 src/modules/login/components/SignInWithGoogle/SignInWithGoogleButton.tsx diff --git a/src/modules/login/components/SignInWithGoogle/GoogleIcon.tsx b/src/modules/login/components/SignInWithGoogle/GoogleIcon.tsx new file mode 100644 index 000000000..c943b5816 --- /dev/null +++ b/src/modules/login/components/SignInWithGoogle/GoogleIcon.tsx @@ -0,0 +1,10 @@ +import { IconProps } from '@chakra-ui/icons' +import { Icon } from '@chakra-ui/react' + +export const GoogleIcon: React.FC = (props) => { + return ( + + + + ) +} diff --git a/src/modules/login/components/SignInWithGoogle/SignInWithGoogleButton.tsx b/src/modules/login/components/SignInWithGoogle/SignInWithGoogleButton.tsx new file mode 100644 index 000000000..3e7b5c182 --- /dev/null +++ b/src/modules/login/components/SignInWithGoogle/SignInWithGoogleButton.tsx @@ -0,0 +1,24 @@ +import { Button } from '@chakra-ui/react' +import { useTranslation } from 'react-i18next' +import endpoints from 'shared/api/endpoints' +import { GoogleIcon } from './GoogleIcon' + +export const SignInWithGoogleButton: React.FC = () => { + const { t } = useTranslation() + + const onClick = () => { + window.location.href = endpoints.googleLogin + } + + return ( + + ) +} diff --git a/src/shared/i18n/en.json b/src/shared/i18n/en.json index 5cc2b1814..764825f4c 100644 --- a/src/shared/i18n/en.json +++ b/src/shared/i18n/en.json @@ -32,10 +32,9 @@ "matches": "The date is not valid" }, "login_page": { - "username_field": "Username", - "password_field": "Password", "welcome_title": "Welcome", - "welcome_message": "Enter in your TNT account" + "welcome_message": "Sign in with your company account", + "sign_in_with_google": "Sign in with Google" }, "activity_form": { "start_time": "Start time", diff --git a/src/shared/i18n/es.json b/src/shared/i18n/es.json index c2425b6e6..60aba1b4a 100644 --- a/src/shared/i18n/es.json +++ b/src/shared/i18n/es.json @@ -32,10 +32,9 @@ "matches": "La fecha no es valida" }, "login_page": { - "username_field": "Usuario", - "password_field": "Password", "welcome_title": "Bienvenido", - "welcome_message": "Entra en tu cuenta TNT" + "welcome_message": "Inicia sesión con tu cuenta de empresa", + "sign_in_with_google": "Iniciar sesión con Google" }, "activity_form": { "start_time": "Hora inicio", From 66bf52bed4fcff3ced370191fb82b7c4c131709b Mon Sep 17 00:00:00 2001 From: Alberto Chamorro Date: Wed, 1 Feb 2023 07:33:33 +0100 Subject: [PATCH 04/19] feat: now AutoLoginAction only try to get the user logged from API --- .../data-access/actions/auto-login-action.ts | 21 ++++--------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/src/modules/login/data-access/actions/auto-login-action.ts b/src/modules/login/data-access/actions/auto-login-action.ts index 80c792628..63ae67561 100644 --- a/src/modules/login/data-access/actions/auto-login-action.ts +++ b/src/modules/login/data-access/actions/auto-login-action.ts @@ -1,33 +1,20 @@ import { action, makeObservable, runInAction } from 'mobx' -import { inject, singleton } from 'tsyringe' +import { singleton } from 'tsyringe' import type { IAction } from 'shared/arch/interfaces/IAction' -import type { TokenStorage } from 'shared/api/oauth/token-storage/token-storage' -import { OAuthRepository } from 'shared/api/oauth/oauth-repository' import { AppState } from 'shared/data-access/state/app-state' import { UserRepository } from 'modules/login/data-access/repositories/user-repository' @singleton() export class AutoLoginAction implements IAction<{ username: string; password: string }> { - constructor( - @inject('TokenStorage') private tokenStorage: TokenStorage, - private oauthRepository: OAuthRepository, - private appState: AppState, - private userRepository: UserRepository - ) { + constructor(private appState: AppState, private userRepository: UserRepository) { makeObservable(this) } @action async execute(): Promise { - const refreshToken = await this.tokenStorage.getRefreshToken() - - if (refreshToken) { - const oauthResponse = await this.oauthRepository.renewOAuthByRefreshToken(refreshToken) - this.tokenStorage.setAccessToken(oauthResponse.access_token) - await this.tokenStorage.setRefreshToken(oauthResponse.refresh_token) - - const user = await this.userRepository.getUser() + const user = await this.userRepository.getUser() + if (user) { runInAction(() => { this.appState.isAuthenticated = true this.appState.loggedUser = user From e2a095343cd53f67af83812ce1be96ce39d648db Mon Sep 17 00:00:00 2001 From: Alberto Chamorro Date: Wed, 1 Feb 2023 07:34:27 +0100 Subject: [PATCH 05/19] feat: add SignInWithGoogle button to LoginForm and remove PasswordField and useLoginForm elements --- .../login/components/LoginForm/LoginForm.tsx | 38 ++++------------ .../components/LoginForm/useLoginForm.ts | 43 ------------------- .../PasswordField/PasswordField.tsx | 31 ------------- .../PasswordField/PasswordInput.test.tsx | 27 ------------ .../PasswordField/PasswordInput.tsx | 39 ----------------- 5 files changed, 9 insertions(+), 169 deletions(-) delete mode 100644 src/modules/login/components/LoginForm/useLoginForm.ts delete mode 100644 src/modules/login/components/PasswordField/PasswordField.tsx delete mode 100644 src/modules/login/components/PasswordField/PasswordInput.test.tsx delete mode 100644 src/modules/login/components/PasswordField/PasswordInput.tsx diff --git a/src/modules/login/components/LoginForm/LoginForm.tsx b/src/modules/login/components/LoginForm/LoginForm.tsx index 9b071ca65..d8b0455ba 100644 --- a/src/modules/login/components/LoginForm/LoginForm.tsx +++ b/src/modules/login/components/LoginForm/LoginForm.tsx @@ -1,15 +1,11 @@ -import { Flex, Heading, Stack, useColorModeValue } from '@chakra-ui/react' +import { Flex, Heading, useColorModeValue } from '@chakra-ui/react' import { AppVersion } from 'modules/login/components/AppVersion' -import { useLoginForm } from 'modules/login/components/LoginForm/useLoginForm' -import { PasswordField } from 'modules/login/components/PasswordField/PasswordField' import { useTranslation } from 'react-i18next' -import SubmitButton from 'shared/components/FormFields/SubmitButton' -import { TextField } from 'shared/components/FormFields/TextField' import { LogoAutentia } from 'shared/components/LogoAutentia' +import { SignInWithGoogleButton } from '../SignInWithGoogle/SignInWithGoogleButton' export const LoginForm = () => { const { t } = useTranslation() - const form = useLoginForm() const bgColor = useColorModeValue('white', undefined) @@ -17,29 +13,13 @@ export const LoginForm = () => { -
- - {t('login_page.welcome_title')} - - - {t('login_page.welcome_message')} - - - - - LOGIN - -
+ + {t('login_page.welcome_title')} + + + {t('login_page.welcome_message')} + + diff --git a/src/modules/login/components/LoginForm/useLoginForm.ts b/src/modules/login/components/LoginForm/useLoginForm.ts deleted file mode 100644 index 27a04976c..000000000 --- a/src/modules/login/components/LoginForm/useLoginForm.ts +++ /dev/null @@ -1,43 +0,0 @@ -import { LoginAction } from 'modules/login/data-access/actions/login-action' -import { useForm } from 'react-hook-form' -import { useAction } from 'shared/arch/hooks/use-action' - -type LoginForm = { - username: string - password: string -} - -export function useLoginForm() { - const login = useAction(LoginAction) - - const { - register, - handleSubmit, - reset, - formState: { errors, isSubmitting }, - setFocus, - clearErrors - } = useForm({ - mode: 'onSubmit' - }) - - const onSubmit = handleSubmit(async (data) => { - try { - await login({ username: data.username, password: data.password }) - } catch (e) { - // @ts-ignore - if (e.response && e.response.status === 401) { - clearErrors() - reset({ username: '', password: '' }) - setFocus('username') - } - } - }) - - return { - register, - errors, - onSubmit, - isLoading: isSubmitting - } -} diff --git a/src/modules/login/components/PasswordField/PasswordField.tsx b/src/modules/login/components/PasswordField/PasswordField.tsx deleted file mode 100644 index be0acf97e..000000000 --- a/src/modules/login/components/PasswordField/PasswordField.tsx +++ /dev/null @@ -1,31 +0,0 @@ -import { FormControl, FormErrorMessage } from '@chakra-ui/react' -import { PasswordInput } from 'modules/login/components/PasswordField/PasswordInput' -import type { FC } from 'react' -import { forwardRef } from 'react' - -interface Props { - name: string - label: string - error?: string -} - -export const PasswordField: FC = forwardRef( - ({ label, error, ...props }, ref) => { - const id = props.name + '_field' - - return ( - - - {error} - - ) - } -) - -PasswordField.displayName = 'PasswordField' diff --git a/src/modules/login/components/PasswordField/PasswordInput.test.tsx b/src/modules/login/components/PasswordField/PasswordInput.test.tsx deleted file mode 100644 index ed506a644..000000000 --- a/src/modules/login/components/PasswordField/PasswordInput.test.tsx +++ /dev/null @@ -1,27 +0,0 @@ -import { FormControl } from '@chakra-ui/react' -import { PasswordInput } from 'modules/login/components/PasswordField/PasswordInput' -import { render, screen, userEvent, waitFor } from 'test-utils/app-test-utils' - -it('should show and hide password', async () => { - setup() - - userEvent.type(screen.getByTestId('password'), 's3cr3t') - - userEvent.click(screen.getByLabelText('actions.show')) - expect(screen.getByTestId('password')).toHaveAttribute('type', 'text') - - userEvent.click(screen.getByLabelText('actions.hide')) - expect(screen.getByTestId('password')).toHaveAttribute('type', 'password') - - await waitFor(() => { - expect(screen.getByTestId('password')).toHaveValue('s3cr3t') - }) -}) - -function setup() { - render( - - - - ) -} diff --git a/src/modules/login/components/PasswordField/PasswordInput.tsx b/src/modules/login/components/PasswordField/PasswordInput.tsx deleted file mode 100644 index 8bbda8693..000000000 --- a/src/modules/login/components/PasswordField/PasswordInput.tsx +++ /dev/null @@ -1,39 +0,0 @@ -import { Icon, IconButton, InputGroup, InputRightElement } from '@chakra-ui/react' -import { EyeIcon, EyeOffIcon } from '@heroicons/react/outline' -import type { InputHTMLAttributes } from 'react' -import { forwardRef, useState } from 'react' -import { useTranslation } from 'react-i18next' -import { FloatingLabelInput } from 'shared/components/FloatingLabelInput' - -interface Props extends InputHTMLAttributes> { - label: string -} - -export const PasswordInput = forwardRef((props, ref) => { - const { t } = useTranslation() - const [show, setShow] = useState(false) - const handleShowPassword = () => setShow((prevState) => !prevState) - - return ( - - - - : } - onClick={handleShowPassword} - variant="ghost" - size="sm" - data-testid="password_visibility_button" - /> - - - ) -}) - -PasswordInput.displayName = 'PasswordInput' From e5bdce014434dca6707821f69de0b6c286c9f695 Mon Sep 17 00:00:00 2001 From: Alberto Chamorro Date: Wed, 1 Feb 2023 15:39:04 +0100 Subject: [PATCH 06/19] feat: remove TokenStorage, OAuthRepository and LoginAction --- .../data-access/actions/login-action.test.ts | 44 ----------------- .../login/data-access/actions/login-action.ts | 36 -------------- src/modules/login/page/LoginPage.test.tsx | 26 +--------- src/shared/api/endpoints.ts | 1 - src/shared/api/oauth/OAuth.ts | 8 ---- src/shared/api/oauth/oauth-repository.ts | 38 --------------- .../token-storage/memory-token-source.test.ts | 36 -------------- .../token-storage/memory-token-storage.ts | 29 ------------ .../session-token-storage.test.ts | 47 ------------------- .../token-storage/session-token-storage.ts | 35 -------------- .../api/oauth/token-storage/token-storage.ts | 10 ---- .../ioc-container/ioc-container.ts | 7 --- 12 files changed, 1 insertion(+), 316 deletions(-) delete mode 100644 src/modules/login/data-access/actions/login-action.test.ts delete mode 100644 src/modules/login/data-access/actions/login-action.ts delete mode 100644 src/shared/api/oauth/OAuth.ts delete mode 100644 src/shared/api/oauth/oauth-repository.ts delete mode 100644 src/shared/api/oauth/token-storage/memory-token-source.test.ts delete mode 100644 src/shared/api/oauth/token-storage/memory-token-storage.ts delete mode 100644 src/shared/api/oauth/token-storage/session-token-storage.test.ts delete mode 100644 src/shared/api/oauth/token-storage/session-token-storage.ts delete mode 100644 src/shared/api/oauth/token-storage/token-storage.ts diff --git a/src/modules/login/data-access/actions/login-action.test.ts b/src/modules/login/data-access/actions/login-action.test.ts deleted file mode 100644 index 93d43b36a..000000000 --- a/src/modules/login/data-access/actions/login-action.test.ts +++ /dev/null @@ -1,44 +0,0 @@ -import { LoginAction } from 'modules/login/data-access/actions/login-action' -import { UserRepository } from 'modules/login/data-access/repositories/user-repository' -import type { User } from 'shared/api/users/User' -import { AppState } from 'shared/data-access/state/app-state' -import { container } from 'tsyringe' -import { mock } from 'jest-mock-extended' -import { OAuthRepository } from 'shared/api/oauth/oauth-repository' -import { TokenStorage } from 'shared/api/oauth/token-storage/token-storage' -import { buildOAuthResource } from 'test-utils/generateTestMocks' - -describe('LoginAction', () => { - test('should login', async () => { - const user: User = { foo: '' } as any - const { userRepository, oauthRepository, tokenStorage, appState, loginAction } = setup() - - const oauthResponse = buildOAuthResource() - oauthRepository.getOAuthByUserCredentials.mockResolvedValue(oauthResponse) - userRepository.getUser.mockResolvedValue(user) - - await loginAction.execute({ username: 'username', password: 'password' }) - - expect(oauthRepository.getOAuthByUserCredentials).toHaveBeenCalledWith('username', 'password') - expect(tokenStorage.setAccessToken).toHaveBeenCalledWith(oauthResponse.access_token) - expect(tokenStorage.setRefreshToken).toHaveBeenCalledWith(oauthResponse.refresh_token) - expect(userRepository.getUser).toHaveBeenCalledWith() - expect(appState.isAuthenticated).toEqual(true) - expect(appState.loggedUser).toEqual(user) - }) -}) - -function setup() { - const userRepository = mock() - const oauthRepository = mock() - const tokenStorage = mock() - const appState = container.resolve(AppState) - - return { - userRepository, - oauthRepository, - tokenStorage, - appState, - loginAction: new LoginAction(userRepository, oauthRepository, appState, tokenStorage) - } -} diff --git a/src/modules/login/data-access/actions/login-action.ts b/src/modules/login/data-access/actions/login-action.ts deleted file mode 100644 index af35fe2a4..000000000 --- a/src/modules/login/data-access/actions/login-action.ts +++ /dev/null @@ -1,36 +0,0 @@ -import { action, makeObservable, runInAction } from 'mobx' -import { UserRepository } from 'modules/login/data-access/repositories/user-repository' -import { AppState } from 'shared/data-access/state/app-state' -import { inject, singleton } from 'tsyringe' -import type { IAction } from 'shared/arch/interfaces/IAction' -import { OAuthRepository } from 'shared/api/oauth/oauth-repository' -import type { TokenStorage } from 'shared/api/oauth/token-storage/token-storage' - -@singleton() -export class LoginAction implements IAction<{ username: string; password: string }> { - constructor( - private userRepository: UserRepository, - private oauthRepository: OAuthRepository, - private appState: AppState, - @inject('TokenStorage') private tokenStorage: TokenStorage - ) { - makeObservable(this) - } - - @action - async execute(param: { username: string; password: string }): Promise { - const oauthResponse = await this.oauthRepository.getOAuthByUserCredentials( - param.username, - param.password - ) - this.tokenStorage.setAccessToken(oauthResponse.access_token) - await this.tokenStorage.setRefreshToken(oauthResponse.refresh_token) - - const user = await this.userRepository.getUser() - - runInAction(() => { - this.appState.isAuthenticated = true - this.appState.loggedUser = user - }) - } -} diff --git a/src/modules/login/page/LoginPage.test.tsx b/src/modules/login/page/LoginPage.test.tsx index 1e7acec4f..720574789 100644 --- a/src/modules/login/page/LoginPage.test.tsx +++ b/src/modules/login/page/LoginPage.test.tsx @@ -2,13 +2,10 @@ import { mock } from 'jest-mock-extended' import LoginPage from 'modules/login/page/LoginPage' import { MemoryRouter, Route, Routes } from 'react-router-dom' import { rawPaths } from 'shared/router/paths' -import { createAxiosError, render, screen, userEvent, waitFor } from 'test-utils/app-test-utils' +import { render, screen, userEvent, waitFor } from 'test-utils/app-test-utils' import { container } from 'tsyringe' import { AutoLoginAction } from 'modules/login/data-access/actions/auto-login-action' -import { LoginAction } from 'modules/login/data-access/actions/login-action' import { UserRepository } from 'modules/login/data-access/repositories/user-repository' -import { OAuthRepository } from 'shared/api/oauth/oauth-repository' -import { MemoryTokenStorage } from 'shared/api/oauth/token-storage/memory-token-storage' import { GetApiVersionAction } from '../data-access/actions/get-api-version-action' describe('LoginPage', () => { @@ -58,17 +55,7 @@ describe('LoginPage', () => { }) it('should redirect to binnacle page on login success', async () => { - const authRepository = mock() - const tokenStorage = mock() const userRepository = mock() - container.registerInstance(OAuthRepository, authRepository) - authRepository.getOAuthByUserCredentials.mockResolvedValue({ - access_token: 'accessToken', - refresh_token: 'refreshToken' - } as any) - container.registerInstance(MemoryTokenStorage, tokenStorage) - tokenStorage.setAccessToken.mockImplementation() - tokenStorage.setRefreshToken.mockImplementation() container.registerInstance(UserRepository, userRepository) userRepository.getUser.mockResolvedValue({} as any) @@ -83,16 +70,11 @@ describe('LoginPage', () => { await waitFor(() => { expect(screen.getByText(/Binnacle Page/i)).toBeInTheDocument() - expect(authRepository.getOAuthByUserCredentials).toHaveBeenCalledWith('johndoe', 's3cr3t') expect(userRepository.getUser).toHaveBeenCalled() }) }) it('should reset form values and focus username on login http 401 error', async () => { - const loginAction = mock() - container.registerInstance(LoginAction, loginAction) - loginAction.execute.mockRejectedValue(createAxiosError(401)) - setup() await waitFor(() => { expect(screen.getByLabelText('login_page.username_field')).toBeInTheDocument() @@ -103,7 +85,6 @@ describe('LoginPage', () => { userEvent.click(screen.getByRole('button', { name: /login/i })) await waitFor(() => { - expect(loginAction.execute).toHaveBeenCalledWith({ password: 's3cr3t', username: 'johndoe' }) expect(screen.getByTestId('login-form')).toHaveFormValues({ username: '', password: '' @@ -113,10 +94,6 @@ describe('LoginPage', () => { }) it('should keep form values and show notification on login http error', async () => { - const loginAction = mock() - container.registerInstance(LoginAction, loginAction) - loginAction.execute.mockRejectedValue(createAxiosError(500)) - setup() await waitFor(() => { expect(screen.getByLabelText('login_page.username_field')).toBeInTheDocument() @@ -132,7 +109,6 @@ describe('LoginPage', () => { await waitFor(() => { expect(screen.getByText('api_errors.server_error')).toBeInTheDocument() - expect(loginAction.execute).toHaveBeenCalledWith({ password: 's3cr3t', username: 'johndoe' }) }) expect(screen.getByRole('button', { name: /login/i })).not.toBeDisabled() diff --git a/src/shared/api/endpoints.ts b/src/shared/api/endpoints.ts index d463cd4a2..ad619382d 100644 --- a/src/shared/api/endpoints.ts +++ b/src/shared/api/endpoints.ts @@ -3,7 +3,6 @@ const BASE_URL = (process.env.REACT_APP_API_SUBDIRECTORY_PATH || '').slice(0, -1) const endpoints = { - auth: `${BASE_URL}/oauth/token`, user: `${BASE_URL}/api/user`, activities: `${BASE_URL}/api/activities`, holidays: `${BASE_URL}/api/holidays`, diff --git a/src/shared/api/oauth/OAuth.ts b/src/shared/api/oauth/OAuth.ts deleted file mode 100644 index ca94108ee..000000000 --- a/src/shared/api/oauth/OAuth.ts +++ /dev/null @@ -1,8 +0,0 @@ -export interface OAuth { - access_token: string - token_type: 'bearer' - refresh_token: string - expires_in: number - scope: string - jti: string -} diff --git a/src/shared/api/oauth/oauth-repository.ts b/src/shared/api/oauth/oauth-repository.ts deleted file mode 100644 index 9f30278dc..000000000 --- a/src/shared/api/oauth/oauth-repository.ts +++ /dev/null @@ -1,38 +0,0 @@ -import { singleton } from 'tsyringe' -import { HttpClient } from 'shared/data-access/http-client/http-client' -import endpoints from 'shared/api/endpoints' -import { OAuth } from 'shared/types/OAuth' - -@singleton() -export class OAuthRepository { - private clientName: string = process.env.REACT_APP_CLIENT_NAME! - private clientSecret: string = process.env.REACT_APP_CLIENT_SECRET! - private base64Credentials = btoa(this.clientName + ':' + this.clientSecret) - - constructor(private httpClient: HttpClient) {} - - async getOAuthByUserCredentials(username: string, password: string): Promise { - const searchParams = new URLSearchParams() - searchParams.set('grant_type', 'password') - searchParams.set('username', username) - searchParams.set('password', password) - - return await this.httpClient.post(endpoints.auth, searchParams, { - headers: { - Authorization: `Basic ${this.base64Credentials}` - } - }) - } - - async renewOAuthByRefreshToken(refresh_token: string): Promise { - return await this.httpClient.post(endpoints.auth, null, { - headers: { - Authorization: `Basic ${this.base64Credentials}` - }, - params: { - grant_type: 'refresh_token', - refresh_token: refresh_token - } - }) - } -} diff --git a/src/shared/api/oauth/token-storage/memory-token-source.test.ts b/src/shared/api/oauth/token-storage/memory-token-source.test.ts deleted file mode 100644 index c838f065d..000000000 --- a/src/shared/api/oauth/token-storage/memory-token-source.test.ts +++ /dev/null @@ -1,36 +0,0 @@ -import { MemoryTokenStorage } from 'shared/api/oauth/token-storage/memory-token-storage' -import { container } from 'tsyringe' - -describe('MemoryTokenStorage', () => { - it('should set & get access token', () => { - const { memoryTokenStorage } = setup() - - memoryTokenStorage.setAccessToken('accessToken') - expect(memoryTokenStorage.getAccessToken()).toEqual('accessToken') - }) - - it('should set & get refresh token', async () => { - const { memoryTokenStorage } = setup() - - await memoryTokenStorage.setRefreshToken('refreshToken') - expect(await memoryTokenStorage.getRefreshToken()).toEqual('refreshToken') - }) - - it('should clear tokens', async () => { - const { memoryTokenStorage } = setup() - - memoryTokenStorage.setAccessToken('accessToken') - await memoryTokenStorage.setRefreshToken('refreshToken') - - memoryTokenStorage.clearTokens() - - expect(memoryTokenStorage.getAccessToken()).toEqual(undefined) - expect(await memoryTokenStorage.getRefreshToken()).toEqual(undefined) - }) -}) - -function setup() { - return { - memoryTokenStorage: container.resolve(MemoryTokenStorage) - } -} diff --git a/src/shared/api/oauth/token-storage/memory-token-storage.ts b/src/shared/api/oauth/token-storage/memory-token-storage.ts deleted file mode 100644 index 9b691b85f..000000000 --- a/src/shared/api/oauth/token-storage/memory-token-storage.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { singleton } from 'tsyringe' -import type { TokenStorage } from 'shared/api/oauth/token-storage/token-storage' - -@singleton() -export class MemoryTokenStorage implements TokenStorage { - private accessToken: string | undefined = undefined - private refreshToken: string | undefined = undefined - - setAccessToken(accessToken: string) { - this.accessToken = accessToken - } - - getAccessToken(): string | undefined { - return this.accessToken - } - - async setRefreshToken(refreshToken: string) { - this.refreshToken = refreshToken - } - - async getRefreshToken(): Promise { - return this.refreshToken - } - - clearTokens = async () => { - this.accessToken = undefined - this.refreshToken = undefined - } -} diff --git a/src/shared/api/oauth/token-storage/session-token-storage.test.ts b/src/shared/api/oauth/token-storage/session-token-storage.test.ts deleted file mode 100644 index efd69f2eb..000000000 --- a/src/shared/api/oauth/token-storage/session-token-storage.test.ts +++ /dev/null @@ -1,47 +0,0 @@ -import { mock } from 'jest-mock-extended' - -import { SessionTokenStorage } from 'shared/api/oauth/token-storage/session-token-storage' - -describe('SessionTokenStorage', () => { - it('should set & get access token', async () => { - const { sessionTokenStorage } = setup() - - sessionTokenStorage.setAccessToken('accessToken') - expect(sessionTokenStorage.getAccessToken()).toEqual('accessToken') - }) - - it('should set & get refresh token', async () => { - const { sessionTokenStorage, storage } = setup() - storage.getItem.mockReturnValue('Foo') - - expect(await sessionTokenStorage.getRefreshToken()).toBe('Foo') - expect(storage.getItem).toHaveBeenCalledWith(SessionTokenStorage.REFRESH_TOKEN_KEY) - - await sessionTokenStorage.setRefreshToken('refreshToken') - expect(storage.setItem).toHaveBeenCalledWith( - SessionTokenStorage.REFRESH_TOKEN_KEY, - 'refreshToken' - ) - }) - - it('should clear tokens', async () => { - const { sessionTokenStorage, storage } = setup() - - sessionTokenStorage.setAccessToken('accessToken') - await sessionTokenStorage.setRefreshToken('refreshToken') - - await sessionTokenStorage.clearTokens() - - expect(sessionTokenStorage.getAccessToken()).toEqual(null) - expect(storage.removeItem).toHaveBeenCalledWith(SessionTokenStorage.REFRESH_TOKEN_KEY) - }) -}) - -function setup() { - const storage = mock() - - return { - storage, - sessionTokenStorage: new SessionTokenStorage(storage) - } -} diff --git a/src/shared/api/oauth/token-storage/session-token-storage.ts b/src/shared/api/oauth/token-storage/session-token-storage.ts deleted file mode 100644 index 9ec264050..000000000 --- a/src/shared/api/oauth/token-storage/session-token-storage.ts +++ /dev/null @@ -1,35 +0,0 @@ -import type { TokenStorage } from 'shared/api/oauth/token-storage/token-storage' -import { Nullable } from 'shared/types/Nullable' -import { singleton } from 'tsyringe' - -@singleton() -export class SessionTokenStorage implements TokenStorage { - static REFRESH_TOKEN_KEY = 'binnacle_token' - private ACCESS_TOKEN_KEY = 'binnacle_access_token' - - private accessToken: Nullable = null - - constructor(private storage: Storage) {} - - setAccessToken(accessToken: string) { - this.accessToken = accessToken - } - - getAccessToken(): Nullable { - return this.accessToken - } - - async setRefreshToken(refreshToken: string) { - this.storage.setItem(SessionTokenStorage.REFRESH_TOKEN_KEY, refreshToken) - } - - async getRefreshToken(): Promise { - const refreshToken = this.storage.getItem(SessionTokenStorage.REFRESH_TOKEN_KEY) - return refreshToken !== null ? refreshToken : undefined - } - - clearTokens = async () => { - this.storage.removeItem(SessionTokenStorage.REFRESH_TOKEN_KEY) - this.accessToken = null - } -} diff --git a/src/shared/api/oauth/token-storage/token-storage.ts b/src/shared/api/oauth/token-storage/token-storage.ts deleted file mode 100644 index 11782add4..000000000 --- a/src/shared/api/oauth/token-storage/token-storage.ts +++ /dev/null @@ -1,10 +0,0 @@ -import { Nullable } from 'shared/types/Nullable' - -export interface TokenStorage { - setAccessToken: (value: string) => void - getAccessToken: () => Nullable - - setRefreshToken: (value: string) => Promise - getRefreshToken: () => Promise> - clearTokens: () => Promise -} diff --git a/src/shared/data-access/ioc-container/ioc-container.ts b/src/shared/data-access/ioc-container/ioc-container.ts index e6777bd75..407d92549 100644 --- a/src/shared/data-access/ioc-container/ioc-container.ts +++ b/src/shared/data-access/ioc-container/ioc-container.ts @@ -1,6 +1,4 @@ import { createStandaloneToast } from '@chakra-ui/react' -import { SessionTokenStorage } from 'shared/api/oauth/token-storage/session-token-storage' -import type { TokenStorage } from 'shared/api/oauth/token-storage/token-storage' import { STORAGE, TOAST } from 'shared/data-access/ioc-container/ioc-container.types' import { container } from 'tsyringe' @@ -10,11 +8,6 @@ export type ToastType = typeof toast export function registerValueProviders() { container.register(STORAGE, { useValue: localStorage }) container.register(TOAST, { useValue: toast }) - - // https://github.com/microsoft/tsyringe#instancecachingfactory - container.register('TokenStorage', { - useValue: new SessionTokenStorage(sessionStorage) - }) } registerValueProviders() From 4c91a291a099f6876f6883040d6a3ac9064085f7 Mon Sep 17 00:00:00 2001 From: Alberto Chamorro Date: Thu, 2 Feb 2023 07:20:19 +0100 Subject: [PATCH 07/19] test: update LoginPage tests with new login process --- .../SignInWithGoogleButton.tsx | 2 +- src/modules/login/page/LoginPage.test.tsx | 91 ++++--------------- 2 files changed, 18 insertions(+), 75 deletions(-) diff --git a/src/modules/login/components/SignInWithGoogle/SignInWithGoogleButton.tsx b/src/modules/login/components/SignInWithGoogle/SignInWithGoogleButton.tsx index 3e7b5c182..5b899348a 100644 --- a/src/modules/login/components/SignInWithGoogle/SignInWithGoogleButton.tsx +++ b/src/modules/login/components/SignInWithGoogle/SignInWithGoogleButton.tsx @@ -7,7 +7,7 @@ export const SignInWithGoogleButton: React.FC = () => { const { t } = useTranslation() const onClick = () => { - window.location.href = endpoints.googleLogin + window.location.assign(endpoints.googleLogin) } return ( diff --git a/src/modules/login/page/LoginPage.test.tsx b/src/modules/login/page/LoginPage.test.tsx index 720574789..5c785d4b5 100644 --- a/src/modules/login/page/LoginPage.test.tsx +++ b/src/modules/login/page/LoginPage.test.tsx @@ -5,113 +5,56 @@ import { rawPaths } from 'shared/router/paths' import { render, screen, userEvent, waitFor } from 'test-utils/app-test-utils' import { container } from 'tsyringe' import { AutoLoginAction } from 'modules/login/data-access/actions/auto-login-action' -import { UserRepository } from 'modules/login/data-access/repositories/user-repository' import { GetApiVersionAction } from '../data-access/actions/get-api-version-action' +import endpoints from 'shared/api/endpoints' +import { AppState } from 'shared/data-access/state/app-state' describe('LoginPage', () => { it('should update document title', async function () { setup() await waitFor(() => { - expect(screen.getByLabelText('login_page.username_field')).toBeInTheDocument() + expect(screen.getByText('login_page.sign_in_with_google')).toBeInTheDocument() }) expect(document.title).toEqual('Login') }) - it('should autofocus username on mount', async function () { + it('should show the sign in with Google button', async function () { setup() await waitFor(() => { - expect(screen.getByLabelText('login_page.username_field')).toBeInTheDocument() + expect(screen.getByText('login_page.sign_in_with_google')).toBeInTheDocument() }) - - expect(screen.getByLabelText('login_page.username_field')).toHaveFocus() }) - it('should have empty values by default', async function () { + it('should change the window location by googleLogin url', async () => { + const assignSpy = jest.fn() + window.location = { + assign: assignSpy + } as unknown as Location setup() - await waitFor(() => { - expect(screen.getByLabelText('login_page.username_field')).toBeInTheDocument() - }) - expect(screen.getByTestId('login-form')).toHaveFormValues({ - username: '', - password: '' - }) - }) - - it('should require fields on submit', async () => { - setup() await waitFor(() => { - expect(screen.getByLabelText('login_page.username_field')).toBeInTheDocument() + expect(screen.getByText('login_page.sign_in_with_google')).toBeInTheDocument() }) - userEvent.click(screen.getByRole('button', { name: /login/i })) + userEvent.click(screen.getByText('login_page.sign_in_with_google')) - await waitFor(() => { - expect(screen.getAllByText('form_errors.field_required')).toHaveLength(2) + waitFor(() => { + expect(assignSpy).toHaveBeenCalledWith(endpoints.googleLogin) }) }) - it('should redirect to binnacle page on login success', async () => { - const userRepository = mock() - container.registerInstance(UserRepository, userRepository) - userRepository.getUser.mockResolvedValue({} as any) + it('should redirect to binnacle page when user is authenticated', async () => { + const appState = container.resolve(AppState) + appState.isAuthenticated = true setup() - await waitFor(() => { - expect(screen.getByLabelText('login_page.username_field')).toBeInTheDocument() - }) - - userEvent.type(screen.getByLabelText('login_page.username_field'), 'johndoe') - userEvent.type(screen.getByLabelText('login_page.password_field'), 's3cr3t') - userEvent.click(screen.getByRole('button', { name: /login/i })) await waitFor(() => { expect(screen.getByText(/Binnacle Page/i)).toBeInTheDocument() - expect(userRepository.getUser).toHaveBeenCalled() - }) - }) - - it('should reset form values and focus username on login http 401 error', async () => { - setup() - await waitFor(() => { - expect(screen.getByLabelText('login_page.username_field')).toBeInTheDocument() }) - - userEvent.type(screen.getByLabelText('login_page.username_field'), 'johndoe') - userEvent.type(screen.getByLabelText('login_page.password_field'), 's3cr3t') - userEvent.click(screen.getByRole('button', { name: /login/i })) - - await waitFor(() => { - expect(screen.getByTestId('login-form')).toHaveFormValues({ - username: '', - password: '' - }) - expect(screen.getByLabelText('login_page.username_field')).toHaveFocus() - }) - }) - - it('should keep form values and show notification on login http error', async () => { - setup() - await waitFor(() => { - expect(screen.getByLabelText('login_page.username_field')).toBeInTheDocument() - }) - - userEvent.type(screen.getByLabelText('login_page.username_field'), 'johndoe') - userEvent.type(screen.getByLabelText('login_page.password_field'), 's3cr3t') - userEvent.click(screen.getByRole('button', { name: /login/i })) - - await waitFor(() => { - expect(screen.getByRole('button', { name: /login/i })).toBeDisabled() - }) - - await waitFor(() => { - expect(screen.getByText('api_errors.server_error')).toBeInTheDocument() - }) - - expect(screen.getByRole('button', { name: /login/i })).not.toBeDisabled() }) }) From 0c73a0a934fd472d06dac88cb593d843c9f38d67 Mon Sep 17 00:00:00 2001 From: Alberto Chamorro Date: Thu, 2 Feb 2023 15:21:07 +0100 Subject: [PATCH 08/19] feat: remove tokenStorage from LogoutAction --- .../login/data-access/actions/logout-action.test.ts | 9 ++------- src/modules/login/data-access/actions/logout-action.ts | 9 ++------- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/src/modules/login/data-access/actions/logout-action.test.ts b/src/modules/login/data-access/actions/logout-action.test.ts index 042a8a5ed..1a3d8d7a8 100644 --- a/src/modules/login/data-access/actions/logout-action.test.ts +++ b/src/modules/login/data-access/actions/logout-action.test.ts @@ -1,12 +1,10 @@ -import { mock } from 'jest-mock-extended' import { LogoutAction } from 'modules/login/data-access/actions/logout-action' -import { MemoryTokenStorage } from 'shared/api/oauth/token-storage/memory-token-storage' import { AppState } from 'shared/data-access/state/app-state' import { container } from 'tsyringe' describe('LogoutAction', () => { test('should logout', async () => { - const { tokenStorage, appState, logoutAction } = setup() + const { appState, logoutAction } = setup() appState.isAuthenticated = true expect(appState.isAuthenticated).toBe(true) @@ -14,17 +12,14 @@ describe('LogoutAction', () => { await logoutAction.execute() expect(appState.isAuthenticated).toBe(false) - expect(tokenStorage.clearTokens).toHaveBeenCalled() }) }) function setup() { - const tokenStorage = mock() const appState = container.resolve(AppState) return { - tokenStorage, appState, - logoutAction: new LogoutAction(appState, tokenStorage) + logoutAction: new LogoutAction(appState) } } diff --git a/src/modules/login/data-access/actions/logout-action.ts b/src/modules/login/data-access/actions/logout-action.ts index 49106c383..e89cbffc7 100644 --- a/src/modules/login/data-access/actions/logout-action.ts +++ b/src/modules/login/data-access/actions/logout-action.ts @@ -1,21 +1,16 @@ import { action, makeObservable, runInAction } from 'mobx' -import type { TokenStorage } from 'shared/api/oauth/token-storage/token-storage' import { AppState } from 'shared/data-access/state/app-state' -import { inject, singleton } from 'tsyringe' +import { singleton } from 'tsyringe' import type { IAction } from 'shared/arch/interfaces/IAction' @singleton() export class LogoutAction implements IAction { - constructor( - private appState: AppState, - @inject('TokenStorage') private tokenStorage: TokenStorage - ) { + constructor(private appState: AppState) { makeObservable(this) } @action async execute(): Promise { - await this.tokenStorage.clearTokens() runInAction(() => { this.appState.isAuthenticated = false }) From df0098b17be71bb44ec3279c71ff697fb2e99d56 Mon Sep 17 00:00:00 2001 From: Alberto Chamorro Date: Fri, 3 Feb 2023 07:57:09 +0100 Subject: [PATCH 09/19] feat: add logout with API --- .../login/data-access/actions/logout-action.test.ts | 9 +++++++-- src/modules/login/data-access/actions/logout-action.ts | 5 ++++- src/shared/api/endpoints.ts | 3 ++- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/modules/login/data-access/actions/logout-action.test.ts b/src/modules/login/data-access/actions/logout-action.test.ts index 1a3d8d7a8..ca5fe93f3 100644 --- a/src/modules/login/data-access/actions/logout-action.test.ts +++ b/src/modules/login/data-access/actions/logout-action.test.ts @@ -1,10 +1,12 @@ +import { mock } from 'jest-mock-extended' import { LogoutAction } from 'modules/login/data-access/actions/logout-action' import { AppState } from 'shared/data-access/state/app-state' import { container } from 'tsyringe' +import { UserRepository } from '../repositories/user-repository' describe('LogoutAction', () => { test('should logout', async () => { - const { appState, logoutAction } = setup() + const { appState, logoutAction, userRepository } = setup() appState.isAuthenticated = true expect(appState.isAuthenticated).toBe(true) @@ -12,14 +14,17 @@ describe('LogoutAction', () => { await logoutAction.execute() expect(appState.isAuthenticated).toBe(false) + expect(userRepository.logout).toHaveBeenCalled() }) }) function setup() { const appState = container.resolve(AppState) + const userRepository = mock() return { appState, - logoutAction: new LogoutAction(appState) + userRepository, + logoutAction: new LogoutAction(appState, userRepository) } } diff --git a/src/modules/login/data-access/actions/logout-action.ts b/src/modules/login/data-access/actions/logout-action.ts index e89cbffc7..acdcd823a 100644 --- a/src/modules/login/data-access/actions/logout-action.ts +++ b/src/modules/login/data-access/actions/logout-action.ts @@ -2,15 +2,18 @@ import { action, makeObservable, runInAction } from 'mobx' import { AppState } from 'shared/data-access/state/app-state' import { singleton } from 'tsyringe' import type { IAction } from 'shared/arch/interfaces/IAction' +import { UserRepository } from '../repositories/user-repository' @singleton() export class LogoutAction implements IAction { - constructor(private appState: AppState) { + constructor(private appState: AppState, private userRepository: UserRepository) { makeObservable(this) } @action async execute(): Promise { + await this.userRepository.logout() + runInAction(() => { this.appState.isAuthenticated = false }) diff --git a/src/shared/api/endpoints.ts b/src/shared/api/endpoints.ts index ad619382d..963859009 100644 --- a/src/shared/api/endpoints.ts +++ b/src/shared/api/endpoints.ts @@ -15,7 +15,8 @@ const endpoints = { vacationsDetails: `${BASE_URL}/api/vacations/details`, workingTime: `${BASE_URL}/api/working-time`, version: `${BASE_URL}/api/version`, - googleLogin: `${BASE_URL}/oauth/login/google` + googleLogin: `${BASE_URL}/oauth/login/google`, + logout: `${BASE_URL}/oauth/logout/google` } export default endpoints From ce2e0c3a2ebf7f7d4d70d54f491001c014303696 Mon Sep 17 00:00:00 2001 From: Alberto Chamorro Date: Fri, 3 Feb 2023 07:58:17 +0100 Subject: [PATCH 10/19] feat: now getUser can throw an AnonymousUserError --- .../errors/anonymous-user-error.ts | 1 + .../repositories/user-repository.test.ts | 36 +++++++++++++++---- .../repositories/user-repository.ts | 16 ++++++++- 3 files changed, 45 insertions(+), 8 deletions(-) create mode 100644 src/modules/login/data-access/errors/anonymous-user-error.ts diff --git a/src/modules/login/data-access/errors/anonymous-user-error.ts b/src/modules/login/data-access/errors/anonymous-user-error.ts new file mode 100644 index 000000000..e69be6a75 --- /dev/null +++ b/src/modules/login/data-access/errors/anonymous-user-error.ts @@ -0,0 +1 @@ +export class AnonymousUserError extends Error {} diff --git a/src/modules/login/data-access/repositories/user-repository.test.ts b/src/modules/login/data-access/repositories/user-repository.test.ts index 7b191f7cf..b2397aa4e 100644 --- a/src/modules/login/data-access/repositories/user-repository.test.ts +++ b/src/modules/login/data-access/repositories/user-repository.test.ts @@ -1,20 +1,42 @@ import { mock } from 'jest-mock-extended' import { UserRepository } from 'modules/login/data-access/repositories/user-repository' import endpoints from 'shared/api/endpoints' -import type { User } from 'shared/api/users/User' import { HttpClient } from 'shared/data-access/http-client/http-client' +import { buildUser } from 'test-utils/generateTestMocks' +import { AnonymousUserError } from '../errors/anonymous-user-error' describe('UserRepository', () => { test('should get user', async () => { - const user: User = { foo: '' } as any - const { httpClient, userService } = setup() + const { httpClient, userRepository } = setup() - httpClient.get.mockResolvedValue(user) + httpClient.get.mockResolvedValue(buildUser()) - const result = await userService.getUser() + const result = await userRepository.getUser() expect(httpClient.get).toHaveBeenCalledWith(endpoints.user) - expect(result).toEqual(user) + expect(result).toEqual(buildUser()) + }) + + test('should throw AnonymousUserError when httpClient returns 401 error', async () => { + const { httpClient, userRepository } = setup() + const error = { + response: { + status: 401 + } + } + + httpClient.get.mockRejectedValue(error) + + expect(userRepository.getUser()).rejects.toThrowError(new AnonymousUserError()) + }) + + test('should throw the httpClient error when error is not 401', async () => { + const { httpClient, userRepository } = setup() + const error = new Error() + + httpClient.get.mockRejectedValue(error) + + expect(userRepository.getUser()).rejects.toThrowError(error) }) }) @@ -23,6 +45,6 @@ function setup() { return { httpClient, - userService: new UserRepository(httpClient) + userRepository: new UserRepository(httpClient) } } diff --git a/src/modules/login/data-access/repositories/user-repository.ts b/src/modules/login/data-access/repositories/user-repository.ts index 8634f1a65..10471677c 100644 --- a/src/modules/login/data-access/repositories/user-repository.ts +++ b/src/modules/login/data-access/repositories/user-repository.ts @@ -2,12 +2,26 @@ import endpoints from 'shared/api/endpoints' import type { User } from 'shared/api/users/User' import { HttpClient } from 'shared/data-access/http-client/http-client' import { singleton } from 'tsyringe' +import { AnonymousUserError } from '../errors/anonymous-user-error' @singleton() export class UserRepository { constructor(private httpClient: HttpClient) {} async getUser(): Promise { - return await this.httpClient.get(endpoints.user) + try { + const user = await this.httpClient.get(endpoints.user) + return user + } catch (error) { + if (error.response?.status === 401) { + throw new AnonymousUserError() + } + + throw error + } + } + + async logout(): Promise { + return this.httpClient.get(endpoints.logout) } } From e6af57be31f096a7440b2a32315b15cfe41bf901 Mon Sep 17 00:00:00 2001 From: Alberto Chamorro Date: Fri, 3 Feb 2023 07:59:24 +0100 Subject: [PATCH 11/19] feat: now LoginPage can show an alert error --- src/modules/login/page/LoginPage.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/login/page/LoginPage.tsx b/src/modules/login/page/LoginPage.tsx index a1c05317a..889362333 100644 --- a/src/modules/login/page/LoginPage.tsx +++ b/src/modules/login/page/LoginPage.tsx @@ -14,7 +14,7 @@ import { useActionLoadable } from 'shared/arch/hooks/use-action-loadable' const useAutoLogin = () => { const [autoLogging, loading] = useActionLoadable(AutoLoginAction, { initialLoading: true, - showAlertError: false + showAlertError: true }) useEffect(() => { From 1fae79c04974ca84ba42aa74037c1ad5731b9132 Mon Sep 17 00:00:00 2001 From: Alberto Chamorro Date: Fri, 3 Feb 2023 08:00:02 +0100 Subject: [PATCH 12/19] feat: ignore AnonymousUserError for AutoLoginAction --- .../data-access/actions/auto-login-action.ts | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/modules/login/data-access/actions/auto-login-action.ts b/src/modules/login/data-access/actions/auto-login-action.ts index 63ae67561..ba3dd4cd9 100644 --- a/src/modules/login/data-access/actions/auto-login-action.ts +++ b/src/modules/login/data-access/actions/auto-login-action.ts @@ -3,6 +3,7 @@ import { singleton } from 'tsyringe' import type { IAction } from 'shared/arch/interfaces/IAction' import { AppState } from 'shared/data-access/state/app-state' import { UserRepository } from 'modules/login/data-access/repositories/user-repository' +import { AnonymousUserError } from '../errors/anonymous-user-error' @singleton() export class AutoLoginAction implements IAction<{ username: string; password: string }> { @@ -12,13 +13,20 @@ export class AutoLoginAction implements IAction<{ username: string; password: st @action async execute(): Promise { - const user = await this.userRepository.getUser() + try { + const user = await this.userRepository.getUser() - if (user) { - runInAction(() => { - this.appState.isAuthenticated = true - this.appState.loggedUser = user - }) + if (user) { + runInAction(() => { + this.appState.isAuthenticated = true + this.appState.loggedUser = user + }) + } + } catch (error) { + const isAnonymousUser = error instanceof AnonymousUserError + if (!isAnonymousUser) { + throw error + } } } } From 48a96319a6501377882cf9bb69d8733b11633598 Mon Sep 17 00:00:00 2001 From: Alberto Chamorro Date: Fri, 3 Feb 2023 08:08:50 +0100 Subject: [PATCH 13/19] feat: remove sessionExpired custom error; session is on the back side now --- .../components/Notifications/HttpStatusCodeMessage.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/shared/components/Notifications/HttpStatusCodeMessage.ts b/src/shared/components/Notifications/HttpStatusCodeMessage.ts index 736c90125..5eb090532 100644 --- a/src/shared/components/Notifications/HttpStatusCodeMessage.ts +++ b/src/shared/components/Notifications/HttpStatusCodeMessage.ts @@ -39,16 +39,10 @@ export const statusCodeMap: ICustomStatusMessages = { unknown: { title: i18n.t('api_errors.unknown'), description: i18n.t('api_errors.general_description') - }, - sessionExpired: { - title: i18n.t('api_errors.session_expired'), - description: i18n.t('api_errors.session_expired_description') } } const getTimeoutOrUnknownStatusCode = (error: any) => { - const isRefreshTokenError = - error.response?.status === 401 && error.config?.params?.grant_type === 'refresh_token' if (!navigator.onLine) { return 'offline' } else if (error.name === 'TimeoutError') { @@ -56,8 +50,6 @@ const getTimeoutOrUnknownStatusCode = (error: any) => { } else if (!error.response) { // Network error -> Server is down return 503 - } else if (isRefreshTokenError) { - return 'sessionExpired' } else if (error.response.status !== null) { return error.response.status } else { From 4c9032f9bd54cc029b22877321a550edb6bfbdff Mon Sep 17 00:00:00 2001 From: Alberto Chamorro Date: Fri, 3 Feb 2023 08:12:42 +0100 Subject: [PATCH 14/19] fix: logout endpoint and HTTP method --- src/modules/login/data-access/repositories/user-repository.ts | 2 +- src/shared/api/endpoints.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/modules/login/data-access/repositories/user-repository.ts b/src/modules/login/data-access/repositories/user-repository.ts index 10471677c..2d7e6f143 100644 --- a/src/modules/login/data-access/repositories/user-repository.ts +++ b/src/modules/login/data-access/repositories/user-repository.ts @@ -22,6 +22,6 @@ export class UserRepository { } async logout(): Promise { - return this.httpClient.get(endpoints.logout) + return this.httpClient.post(endpoints.logout) } } diff --git a/src/shared/api/endpoints.ts b/src/shared/api/endpoints.ts index 963859009..2c0c21b17 100644 --- a/src/shared/api/endpoints.ts +++ b/src/shared/api/endpoints.ts @@ -16,7 +16,7 @@ const endpoints = { workingTime: `${BASE_URL}/api/working-time`, version: `${BASE_URL}/api/version`, googleLogin: `${BASE_URL}/oauth/login/google`, - logout: `${BASE_URL}/oauth/logout/google` + logout: `${BASE_URL}/logout` } export default endpoints From 7dedfb6a7c6429884fbf417f6c876c2187db8437 Mon Sep 17 00:00:00 2001 From: Alberto Chamorro Date: Fri, 3 Feb 2023 12:18:49 +0100 Subject: [PATCH 15/19] test: add test for AutoLoginAction --- .../actions/auto-login-action.test.ts | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 src/modules/login/data-access/actions/auto-login-action.test.ts diff --git a/src/modules/login/data-access/actions/auto-login-action.test.ts b/src/modules/login/data-access/actions/auto-login-action.test.ts new file mode 100644 index 000000000..b2ad32343 --- /dev/null +++ b/src/modules/login/data-access/actions/auto-login-action.test.ts @@ -0,0 +1,54 @@ +import { AppState } from 'shared/data-access/state/app-state' +import { container } from 'tsyringe' +import { mock } from 'jest-mock-extended' +import { UserRepository } from '../repositories/user-repository' +import { AutoLoginAction } from './auto-login-action' +import { buildUser } from 'test-utils/generateTestMocks' +import { AnonymousUserError } from '../errors/anonymous-user-error' + +describe('AutoLoginAction', () => { + test('should set loggedUser state when repository return the user', async () => { + const { appState, userRepository, autoLoginAction } = setup() + userRepository.getUser.mockResolvedValue(buildUser()) + appState.loggedUser = undefined + + await autoLoginAction.execute() + + expect(appState.loggedUser).toEqual(buildUser()) + }) + + test('should set isAuthenticated state when repository return the user', async () => { + const { appState, userRepository, autoLoginAction } = setup() + userRepository.getUser.mockResolvedValue(buildUser()) + appState.isAuthenticated = false + + await autoLoginAction.execute() + + expect(appState.isAuthenticated).toBeTruthy() + }) + + test('should throw error when error is not AnonymousUserError', async () => { + const { userRepository, autoLoginAction } = setup() + userRepository.getUser.mockRejectedValue(new Error()) + + expect(autoLoginAction.execute()).rejects.toThrowError() + }) + + test('should not throw error when error is AnonymousUserError', async () => { + const { userRepository, autoLoginAction } = setup() + userRepository.getUser.mockRejectedValue(new AnonymousUserError()) + + expect(autoLoginAction.execute()).resolves.not.toThrowError() + }) +}) + +function setup() { + const userRepository = mock() + const appState = container.resolve(AppState) + + return { + userRepository, + appState, + autoLoginAction: new AutoLoginAction(appState, userRepository) + } +} From bc0a1d5003543f4846f01aa5fedffc490a515cf4 Mon Sep 17 00:00:00 2001 From: Alberto Chamorro Date: Tue, 7 Feb 2023 16:01:11 +0100 Subject: [PATCH 16/19] feat: add HttpSessionInterceptor to validate HTTP errors and redirect to login --- src/shared/AppRoutes.tsx | 20 +++++++++++-- .../http-client/http-session-interceptor.ts | 30 +++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) create mode 100644 src/shared/data-access/http-client/http-session-interceptor.ts diff --git a/src/shared/AppRoutes.tsx b/src/shared/AppRoutes.tsx index a63a00ef2..849921ef5 100644 --- a/src/shared/AppRoutes.tsx +++ b/src/shared/AppRoutes.tsx @@ -1,16 +1,20 @@ import type { FC } from 'react' -import { lazy, Suspense } from 'react' +import { lazy, Suspense, useEffect } from 'react' import { ActivityFormScreen } from 'modules/binnacle/page/BinnacleMobile/ActivityFormScreen' import { LazyBinnaclePage } from 'modules/binnacle/page/BinnaclePage.lazy' import { LazyLoginPage } from 'modules/login/page/LoginPage.lazy' import { LazySettingsPage } from 'modules/settings/page/SettingsPage.lazy' import { LazyVacationsPage } from 'modules/vacations/page/VacationsPage.lazy' -import { Route, Routes } from 'react-router-dom' +import { Route, Routes, useNavigate } from 'react-router-dom' import FullPageLoadingSpinner from 'shared/components/FullPageLoadingSpinner' import { Navbar } from 'shared/components/Navbar/Navbar' import { useIsMobile } from 'shared/hooks' import { rawPaths } from './router/paths' import { RequireAuth } from 'shared/router/RequireAuth' +import { useAction } from 'shared/arch/hooks/use-action' +import { LogoutAction } from 'modules/login/data-access/actions/logout-action' +import { container } from 'tsyringe' +import { HttpSessionInterceptor } from './data-access/http-client/http-session-interceptor' const LazyBinnacleDesktop = lazy( () => @@ -29,6 +33,18 @@ const LazyBinnacleMobile = lazy( export const AppRoutes: FC = () => { const isMobile = useIsMobile() + const logout = useAction(LogoutAction) + const navigate = useNavigate() + + useEffect(() => { + const redirectToLogin = async () => { + await logout() + navigate('/') + } + + container.resolve(HttpSessionInterceptor).initInterceptor(redirectToLogin) + }, [logout, navigate]) + return ( <> diff --git a/src/shared/data-access/http-client/http-session-interceptor.ts b/src/shared/data-access/http-client/http-session-interceptor.ts new file mode 100644 index 000000000..d84ba58e3 --- /dev/null +++ b/src/shared/data-access/http-client/http-session-interceptor.ts @@ -0,0 +1,30 @@ +import { singleton } from 'tsyringe' +import { AxiosError } from 'axios' +import { HttpClient } from 'shared/data-access/http-client/http-client' + +@singleton() +export class HttpSessionInterceptor { + sessionExpiredCb: () => Promise + + constructor(private httpClient: HttpClient) {} + + initInterceptor = (sessionExpiredCb: () => Promise) => { + this.httpClient.httpInstance.interceptors.response.use( + (response) => response, + this.interceptResponseError + ) + + this.sessionExpiredCb = sessionExpiredCb + } + + // https://github.com/waltergar/react-CA/blob/5fb4bd64a8e5f2c276d14a89fe317db0b743983c/src/utils/api/axios.js + interceptResponseError = async (error: AxiosError) => { + const isSessionExpired = error.response?.status === 401 + + if (error.response && isSessionExpired) { + this.sessionExpiredCb() + } + + return Promise.reject(error) + } +} From 89c27c160826b7685c18f0b2646cef37dc7de1dc Mon Sep 17 00:00:00 2001 From: Alberto Chamorro Date: Tue, 7 Feb 2023 16:01:28 +0100 Subject: [PATCH 17/19] feat: change 401 HTTP error with session expired message --- .../Notifications/HttpStatusCodeMessage.test.ts | 16 ++++++++-------- .../Notifications/HttpStatusCodeMessage.ts | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/shared/components/Notifications/HttpStatusCodeMessage.test.ts b/src/shared/components/Notifications/HttpStatusCodeMessage.test.ts index c257d23ec..ff56ca3da 100644 --- a/src/shared/components/Notifications/HttpStatusCodeMessage.test.ts +++ b/src/shared/components/Notifications/HttpStatusCodeMessage.test.ts @@ -10,14 +10,14 @@ const mockPromiseError = (status?: number, name: string | undefined = undefined) } as any) test.each` - status | title | description - ${401} | ${'api_errors.unauthorized'} | ${'api_errors.unauthorized_description'} - ${403} | ${'api_errors.forbidden'} | ${'api_errors.forbidden_description'} - ${404} | ${'api_errors.not_found'} | ${'api_errors.general_description'} - ${408} | ${'api_errors.timeout'} | ${'api_errors.general_description'} - ${500} | ${'api_errors.server_error'} | ${'api_errors.general_description'} - ${503} | ${'api_errors.server_down'} | ${'api_errors.general_description'} - ${'unknown'} | ${'api_errors.unknown'} | ${'api_errors.general_description'} + status | title | description + ${401} | ${'api_errors.session_expired'} | ${'api_errors.session_expired_description'} + ${403} | ${'api_errors.forbidden'} | ${'api_errors.forbidden_description'} + ${404} | ${'api_errors.not_found'} | ${'api_errors.general_description'} + ${408} | ${'api_errors.timeout'} | ${'api_errors.general_description'} + ${500} | ${'api_errors.server_error'} | ${'api_errors.general_description'} + ${503} | ${'api_errors.server_down'} | ${'api_errors.general_description'} + ${'unknown'} | ${'api_errors.unknown'} | ${'api_errors.general_description'} `( 'when status is $status returns a message with title $title and description $description', ({ status, title, description }) => { diff --git a/src/shared/components/Notifications/HttpStatusCodeMessage.ts b/src/shared/components/Notifications/HttpStatusCodeMessage.ts index 5eb090532..fa990b0c2 100644 --- a/src/shared/components/Notifications/HttpStatusCodeMessage.ts +++ b/src/shared/components/Notifications/HttpStatusCodeMessage.ts @@ -9,8 +9,8 @@ export interface ICustomStatusMessages { export const statusCodeMap: ICustomStatusMessages = { '401': { - title: i18n.t('api_errors.unauthorized'), - description: i18n.t('api_errors.unauthorized_description') + title: i18n.t('api_errors.session_expired'), + description: i18n.t('api_errors.session_expired_description') }, '403': { title: i18n.t('api_errors.forbidden'), From eee6e936b4299de3c4a52573f6a8cc5e3714b4ae Mon Sep 17 00:00:00 2001 From: Alberto Chamorro Date: Tue, 7 Feb 2023 16:16:17 +0100 Subject: [PATCH 18/19] fix: mobile background color for today date --- src/shared/styles/misc.css | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/shared/styles/misc.css b/src/shared/styles/misc.css index 6d65c85af..271346717 100644 --- a/src/shared/styles/misc.css +++ b/src/shared/styles/misc.css @@ -2,6 +2,10 @@ --primary-color: #3939df; } +.chakra-ui-light { + --primary-color: var(--chakra-colors-brand-500); +} + /* Hide focus on mouse interaction */ From f76370a2a897514dceef031e9df50958d1a4c001 Mon Sep 17 00:00:00 2001 From: Alberto Chamorro Date: Thu, 9 Feb 2023 18:18:31 +0100 Subject: [PATCH 19/19] test: fix now date for isPastMonth test --- src/shared/utils/isPastMonth.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/utils/isPastMonth.test.ts b/src/shared/utils/isPastMonth.test.ts index 6103c4e2e..c6ea99665 100644 --- a/src/shared/utils/isPastMonth.test.ts +++ b/src/shared/utils/isPastMonth.test.ts @@ -29,7 +29,7 @@ describe('isPastMonth', () => { }) it('Should return false if year is equal and month is greater', () => { - const now = new Date('2022-12-01') + const now = new Date() const date = new Date(now.getFullYear(), now.getMonth() + 1, now.getDate()) const result = isPastMonth(date) expect(result).toBe(false)