From fefb053d144d4f08dc6b9d51613fb17cf09e01e8 Mon Sep 17 00:00:00 2001 From: Tan Nhu Date: Thu, 7 Sep 2023 19:45:24 +0000 Subject: [PATCH] Incorporate Cookies based Auth / Clean up (#410) --- web/src/App.tsx | 6 +-- web/src/AppContext.tsx | 11 ++-- web/src/components/Changes/Changes.tsx | 2 +- .../CommitRangeDropdown.tsx | 6 +-- .../CloneCredentialDialog.tsx | 26 +++++---- .../MarkdownViewer/MarkdownViewer.tsx | 2 +- .../SpaceSelector/SpaceSelector.tsx | 4 +- web/src/hooks/useAPIToken.ts | 17 ------ web/src/hooks/useModalHook.tsx | 2 +- web/src/pages/ExecutionList/ExecutionList.tsx | 2 +- web/src/pages/PipelineList/PipelineList.tsx | 2 +- web/src/pages/SignIn/SignIn.tsx | 35 ++++++------ web/src/pages/SignUp/SignUp.tsx | 8 +-- web/src/pages/UserProfile/UserProfile.tsx | 7 --- web/src/pages/UsersListing/UsersListing.tsx | 54 +++++++++---------- 15 files changed, 79 insertions(+), 105 deletions(-) delete mode 100644 web/src/hooks/useAPIToken.ts diff --git a/web/src/App.tsx b/web/src/App.tsx index 4aae0d320..476f57d35 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -9,7 +9,6 @@ import { AppContextProvider, defaultCurrentUser } from 'AppContext' import type { AppProps } from 'AppProps' import { buildResfulReactRequestOptions, handle401 } from 'AppUtils' import { RouteDestinations } from 'RouteDestinations' -import { useAPIToken } from 'hooks/useAPIToken' import { routes as _routes } from 'RouteDefinitions' import { getConfig } from 'services/config' import { ModalProvider } from 'hooks/useModalHook' @@ -33,10 +32,9 @@ const App: React.FC = React.memo(function App({ currentUserProfileURL = '' }: AppProps) { const [strings, setStrings] = useState() - const [token] = useAPIToken() const getRequestOptions = useCallback( - (): Partial => buildResfulReactRequestOptions(hooks?.useGetToken?.() || token), - [token, hooks] + (): Partial => buildResfulReactRequestOptions(hooks?.useGetToken?.() || ''), + [hooks] ) const routingId = useMemo(() => (standalone ? '' : space.split('/').shift() || ''), [standalone, space]) const queryParams = useMemo(() => (!standalone ? { routingId } : {}), [standalone, routingId]) diff --git a/web/src/AppContext.tsx b/web/src/AppContext.tsx index e07a1250d..9cb19591d 100644 --- a/web/src/AppContext.tsx +++ b/web/src/AppContext.tsx @@ -1,10 +1,10 @@ import React, { useState, useContext, useEffect } from 'react' +import { useHistory } from 'react-router-dom' import { noop } from 'lodash-es' import { useGet } from 'restful-react' import type { AppProps } from 'AppProps' import { routes } from 'RouteDefinitions' import type { TypesUser } from 'services/code' -import { useAPIToken } from 'hooks/useAPIToken' interface AppContextProps extends AppProps { setAppContext: (value: Partial) => void @@ -34,10 +34,9 @@ export const AppContextProvider: React.FC<{ value: AppProps }> = React.memo(func value: initialValue, children }) { - const [token, setToken] = useAPIToken() + const history = useHistory() const { data: currentUser = defaultCurrentUser, error } = useGet({ - path: '/api/v1/user', - lazy: initialValue.standalone && !token + path: '/api/v1/user' }) const [appStates, setAppStates] = useState(initialValue) @@ -49,9 +48,9 @@ export const AppContextProvider: React.FC<{ value: AppProps }> = React.memo(func useEffect(() => { if (initialValue.standalone && error) { - setToken('') + history.push(routes.toSignIn()) } - }, [initialValue.standalone, error, setToken]) + }, [initialValue.standalone, error, history]) return ( = ({ }) ) } - }, [commitRange]) + }, [commitRange, history, routes, repoMetadata.path, pullRequestMetadata?.number]) const { data: rawDiff, diff --git a/web/src/components/Changes/CommitRangeDropdown/CommitRangeDropdown.tsx b/web/src/components/Changes/CommitRangeDropdown/CommitRangeDropdown.tsx index 56dfc4b5f..e0330789a 100644 --- a/web/src/components/Changes/CommitRangeDropdown/CommitRangeDropdown.tsx +++ b/web/src/components/Changes/CommitRangeDropdown/CommitRangeDropdown.tsx @@ -1,4 +1,4 @@ -import React, { useEffect } from 'react' +import React, { useEffect, useMemo } from 'react' import { Divider, PopoverInteractionKind, Position } from '@blueprintjs/core' import { Checkbox, Container, FlexExpander, Layout, Popover, Text } from '@harnessio/uicore' import { Color, FontVariation } from '@harnessio/design-system' @@ -50,13 +50,13 @@ const CommitRangeDropdown: React.FC = ({ setSelectedCommits }) => { const { getString } = useStrings() - const allCommitsSHA = allCommits.map(commit => commit.sha as string) + const allCommitsSHA = useMemo(() => allCommits.map(commit => commit.sha as string), [allCommits]) useEffect(() => { if (selectedCommits.length && allCommitsSHA.length) { setSelectedCommits(prevVal => getCommitRange(prevVal, allCommitsSHA)) } - }, [JSON.stringify(allCommitsSHA)]) + }, [allCommitsSHA, setSelectedCommits, selectedCommits.length]) const handleCheckboxClick = ( event: React.MouseEvent, diff --git a/web/src/components/CloneCredentialDialog/CloneCredentialDialog.tsx b/web/src/components/CloneCredentialDialog/CloneCredentialDialog.tsx index 0e8fd60ef..51a63792f 100644 --- a/web/src/components/CloneCredentialDialog/CloneCredentialDialog.tsx +++ b/web/src/components/CloneCredentialDialog/CloneCredentialDialog.tsx @@ -1,4 +1,4 @@ -import React, { useEffect, useState } from 'react' +import React, { useCallback, useEffect, useState } from 'react' import { Button, ButtonVariation, Container, Dialog, FlexExpander, Layout, Text, useToaster } from '@harnessio/uicore' import { FontVariation } from '@harnessio/design-system' import { useMutate } from 'restful-react' @@ -24,16 +24,20 @@ const CloneCredentialDialog = (props: CloneCredentialDialogProps) => { const { showError } = useToaster() const hash = generateAlphaNumericHash(6) const { mutate } = useMutate({ path: '/api/v1/user/tokens', verb: 'POST' }) - const genToken = async (_props: { uid: string }) => { - const res = await mutate({ uid: _props.uid }) - try { - setToken(res?.access_token) - } catch { - showError(res?.data?.message || res?.message) - } - return res - } + const genToken = useCallback( + async (_props: { uid: string }) => { + const res = await mutate({ uid: _props.uid }) + try { + setToken(res?.access_token) + } catch { + showError(res?.data?.message || res?.message) + } + return res + }, + [mutate, showError] + ) const tokenData = standalone ? false : hooks?.useGenerateToken?.(hash, currentUser.uid, flag) + useEffect(() => { if (tokenData) { if (tokenData && tokenData?.status !== 400) { @@ -44,7 +48,7 @@ const CloneCredentialDialog = (props: CloneCredentialDialogProps) => { } else if (!tokenData && standalone && flag) { genToken({ uid: `code_token_${hash}` }) } - }, [flag, tokenData, showError]) + }, [flag, tokenData, showError]) // eslint-disable-line react-hooks/exhaustive-deps return ( = ({ onSelect }) => { selectSpace(data, false) refetch() } - }, [data]) + }, [data, refetch, selectSpace, space]) useEffect(() => { if (space && !selectedSpace && data) { @@ -81,7 +81,7 @@ export const SpaceSelector: React.FC = ({ onSelect }) => { if (response?.status === 403) { history.push(routes.toSignIn()) } - }, [response, history]) + }, [response, history, routes]) useShowRequestError(error) const NewSpaceButton = ( diff --git a/web/src/hooks/useAPIToken.ts b/web/src/hooks/useAPIToken.ts deleted file mode 100644 index c8fc1f83e..000000000 --- a/web/src/hooks/useAPIToken.ts +++ /dev/null @@ -1,17 +0,0 @@ -import { useLocalStorage } from 'hooks/useLocalStorage' - -const API_TOKEN_KEY = 'HARNESS_CODE_MODULE_STANDALONE_APP_API_TOKEN' - -/** - * Get and Set API token to use in Restful React calls. - * - * This function is called to inject token to Restful React calls only when - * application is run under standalone mode. In embedded mode, token is passed - * from parent app. - * - * @param initialToken initial API token. - * @returns [token, setToken]. - */ -export function useAPIToken(initialToken = ''): [string, React.Dispatch>] { - return useLocalStorage(API_TOKEN_KEY, initialToken) -} diff --git a/web/src/hooks/useModalHook.tsx b/web/src/hooks/useModalHook.tsx index d45f57d6b..6ddcda116 100644 --- a/web/src/hooks/useModalHook.tsx +++ b/web/src/hooks/useModalHook.tsx @@ -32,7 +32,7 @@ const ModalRoot: Unknown = memo(({ modals, container, component: RootComponent = useEffect(() => { setMountNode(container || document.body) - }, []) + }, [container]) return mountNode ? ReactDOM.createPortal( diff --git a/web/src/pages/ExecutionList/ExecutionList.tsx b/web/src/pages/ExecutionList/ExecutionList.tsx index 33737d66e..2370305d2 100644 --- a/web/src/pages/ExecutionList/ExecutionList.tsx +++ b/web/src/pages/ExecutionList/ExecutionList.tsx @@ -130,7 +130,7 @@ const ExecutionList = () => { disableSortBy: true } ], - [getString] + [getString, repoMetadata?.path, routes] ) return ( diff --git a/web/src/pages/PipelineList/PipelineList.tsx b/web/src/pages/PipelineList/PipelineList.tsx index a6641acb9..2d6cebd0e 100644 --- a/web/src/pages/PipelineList/PipelineList.tsx +++ b/web/src/pages/PipelineList/PipelineList.tsx @@ -168,7 +168,7 @@ const PipelineList = () => { disableSortBy: true } ], - [getString, searchTerm] + [getString, repoMetadata?.path, routes, searchTerm] ) return ( diff --git a/web/src/pages/SignIn/SignIn.tsx b/web/src/pages/SignIn/SignIn.tsx index 52a3b001c..92ac01f4b 100644 --- a/web/src/pages/SignIn/SignIn.tsx +++ b/web/src/pages/SignIn/SignIn.tsx @@ -7,42 +7,44 @@ import { useStrings } from 'framework/strings' import { useOnLogin } from 'services/code' import AuthLayout from 'components/AuthLayout/AuthLayout' import { useAppContext } from 'AppContext' -import { useAPIToken } from 'hooks/useAPIToken' import { getErrorMessage, type LoginForm } from 'utils/Utils' import css from './SignIn.module.scss' export const SignIn: React.FC = () => { const { routes } = useAppContext() const { getString } = useStrings() - const [, setToken] = useAPIToken() - const { mutate } = useOnLogin({}) + const { mutate } = useOnLogin({ + queryParams: { + include_cookie: true + } + }) const { showError } = useToaster() - const onLogin = useCallback( - (data: LoginForm) => { + ({ username, password }: LoginForm) => { mutate( - { login_identifier: data.username, password: data.password }, + { login_identifier: username, password }, { headers: { Authorization: '' } } ) - .then(result => { - setToken(result.access_token as string) + .then(() => { window.location.replace(window.location.origin + routes.toCODEHome()) }) - .catch(error => { showError(getErrorMessage(error)) }) }, - [mutate, setToken, showError] + [mutate, showError, routes] + ) + const onSubmit = useCallback( + (data: LoginForm): void => { + if (data.username && data.password) { + onLogin(data) + } + }, + [onLogin] ) - const handleSubmit = (data: LoginForm): void => { - if (data.username && data.password) { - onLogin(data) - } - } return ( @@ -54,10 +56,9 @@ export const SignIn: React.FC = () => { initialValues={{ username: '', password: '' }} formName="loginPageForm" - onSubmit={handleSubmit} + onSubmit={onSubmit} validationSchema={Yup.object().shape({ username: Yup.string().required(getString('userNameRequired')), - password: Yup.string().required(getString('passwordRequired')) })}> diff --git a/web/src/pages/SignUp/SignUp.tsx b/web/src/pages/SignUp/SignUp.tsx index 982974c83..cdeebfc61 100644 --- a/web/src/pages/SignUp/SignUp.tsx +++ b/web/src/pages/SignUp/SignUp.tsx @@ -18,15 +18,12 @@ import AuthLayout from 'components/AuthLayout/AuthLayout' import { useAppContext } from 'AppContext' import { getErrorMessage, type RegisterForm } from 'utils/Utils' import { useOnRegister } from 'services/code' -import { useAPIToken } from 'hooks/useAPIToken' import css from './SignUp.module.scss' -// Renders the Register page. export const SignUp: React.FC = () => { const { routes } = useAppContext() const { getString } = useStrings() const { showError, showSuccess } = useToaster() - const [, setToken] = useAPIToken() const { mutate } = useOnRegister({}) const onRegister = useCallback( @@ -42,8 +39,7 @@ export const SignUp: React.FC = () => { headers: { Authorization: '' } } ) - .then(result => { - setToken(result.access_token as string) + .then(() => { showSuccess(getString('userCreated')) window.location.replace(window.location.origin + routes.toCODEHome()) }) @@ -51,7 +47,7 @@ export const SignUp: React.FC = () => { showError(getErrorMessage(error)) }) }, - [mutate, setToken, showSuccess, showError, getString] + [mutate, showSuccess, showError, getString, routes] ) const handleSubmit = (data: RegisterForm): void => { diff --git a/web/src/pages/UserProfile/UserProfile.tsx b/web/src/pages/UserProfile/UserProfile.tsx index 6fb3585cd..a9bc86fa4 100644 --- a/web/src/pages/UserProfile/UserProfile.tsx +++ b/web/src/pages/UserProfile/UserProfile.tsx @@ -18,19 +18,15 @@ import { useGet, useMutate } from 'restful-react' import type { CellProps, Column } from 'react-table' import ReactTimeago from 'react-timeago' import moment from 'moment' - import { useStrings } from 'framework/strings' -import { useAPIToken } from 'hooks/useAPIToken' import { TypesToken, TypesUser, useGetUser, useOpLogout, useUpdateUser } from 'services/code' import { ButtonRoleProps, getErrorMessage } from 'utils/Utils' import { useConfirmAct } from 'hooks/useConfirmAction' import { useAppContext } from 'AppContext' import { LoadingSpinner } from 'components/LoadingSpinner/LoadingSpinner' import { OptionsMenuButton } from 'components/OptionsMenuButton/OptionsMenuButton' - import useNewToken from './NewToken/NewToken' import EditableTextField from './EditableTextField' - import css from './UserProfile.module.scss' const USER_TOKENS_API_PATH = '/api/v1/user/tokens' @@ -48,11 +44,8 @@ const UserProfile = () => { const { data: userTokens, loading: tokensLoading, refetch: refetchTokens } = useGet({ path: USER_TOKENS_API_PATH }) const { mutate: deleteToken } = useMutate({ path: USER_TOKENS_API_PATH, verb: 'DELETE' }) - const [, setToken] = useAPIToken() - const onLogout = async () => { await logoutUser() - setToken('') history.push(routes.toSignIn()) } diff --git a/web/src/pages/UsersListing/UsersListing.tsx b/web/src/pages/UsersListing/UsersListing.tsx index 32a060bb2..074ca819c 100644 --- a/web/src/pages/UsersListing/UsersListing.tsx +++ b/web/src/pages/UsersListing/UsersListing.tsx @@ -1,4 +1,4 @@ -import React, { useMemo } from 'react' +import React, { useCallback, useMemo } from 'react' import { Avatar, Button, @@ -40,34 +40,34 @@ const UsersListing = () => { } }) const { mutate: deleteUser } = useAdminDeleteUser({}) - const { openModal } = useAddUserModal({ onClose: refetch }) const { openModal: openResetPasswordModal } = useResetPasswordModal() - const onConfirmAct = useConfirmAct() - - const handleDeleteUser = async (userId: string, displayName?: string) => - await onConfirmAct({ - action: async () => { - try { - await deleteUser(userId) - showSuccess(getString('newUserModal.userDeleted', { name: displayName })) - refetch() - } catch (error) { - showError(getErrorMessage(error)) - } - }, - message: ( - - }} - /> - - ), - intent: 'danger', - title: getString('userManagement.deleteUser') - }) + const handleDeleteUser = useCallback( + async (userId: string, displayName?: string) => + await onConfirmAct({ + action: async () => { + try { + await deleteUser(userId) + showSuccess(getString('newUserModal.userDeleted', { name: displayName })) + refetch() + } catch (error) { + showError(getErrorMessage(error)) + } + }, + message: ( + + }} + /> + + ), + intent: 'danger', + title: getString('userManagement.deleteUser') + }), + [deleteUser, getString, onConfirmAct, refetch, showError, showSuccess] + ) const columns: Column[] = useMemo( () => [ @@ -178,7 +178,7 @@ const UsersListing = () => { } } ], - [] + [getString, handleDeleteUser, onConfirmAct, openModal, openResetPasswordModal, refetch, showError, showSuccess] ) return (