From a70fb1be1477f803a3c53bfc7c0e08640fc2b092 Mon Sep 17 00:00:00 2001 From: Roman Donchenko Date: Fri, 9 Aug 2024 18:47:11 +0300 Subject: [PATCH] WIP Don't use tokens in UI --- cvat-core/src/api-implementation.ts | 4 --- cvat-core/src/api.ts | 4 --- cvat-core/src/index.ts | 1 - cvat-core/src/server-proxy.ts | 31 +++-------------- cvat/apps/iam/authentication.py | 15 -------- cvat/settings/base.py | 2 +- .../pr_5331_missing_authentication_data.js | 34 ------------------- 7 files changed, 5 insertions(+), 86 deletions(-) delete mode 100644 tests/cypress/e2e/issues_prs/pr_5331_missing_authentication_data.js diff --git a/cvat-core/src/api-implementation.ts b/cvat-core/src/api-implementation.ts index 1e7bfb5164c7..d274027bdc1a 100644 --- a/cvat-core/src/api-implementation.ts +++ b/cvat-core/src/api-implementation.ts @@ -137,10 +137,6 @@ export default function implementAPI(cvat: CVATCore): CVATCore { const result = await serverProxy.server.setAuthData(response); return result; }); - implementationMixin(cvat.server.removeAuthData, async () => { - const result = await serverProxy.server.removeAuthData(); - return result; - }); implementationMixin(cvat.server.installedApps, async () => { const result = await serverProxy.server.installedApps(); return result; diff --git a/cvat-core/src/api.ts b/cvat-core/src/api.ts index 00a65ac3a84b..ba7191bbc05d 100644 --- a/cvat-core/src/api.ts +++ b/cvat-core/src/api.ts @@ -122,10 +122,6 @@ function build(): CVATCore { const result = await PluginRegistry.apiWrapper(cvat.server.setAuthData, response); return result; }, - async removeAuthData() { - const result = await PluginRegistry.apiWrapper(cvat.server.removeAuthData); - return result; - }, async installedApps() { const result = await PluginRegistry.apiWrapper(cvat.server.installedApps); return result; diff --git a/cvat-core/src/index.ts b/cvat-core/src/index.ts index efd069e919d6..89f5d98f4b9d 100644 --- a/cvat-core/src/index.ts +++ b/cvat-core/src/index.ts @@ -71,7 +71,6 @@ export default interface CVATCore { healthCheck: any; request: any; setAuthData: any; - removeAuthData: any; installedApps: any; apiSchema: typeof serverProxy.server.apiSchema; }; diff --git a/cvat-core/src/server-proxy.ts b/cvat-core/src/server-proxy.ts index 1400b1e3db3f..91dc52a71821 100644 --- a/cvat-core/src/server-proxy.ts +++ b/cvat-core/src/server-proxy.ts @@ -358,10 +358,10 @@ Axios.interceptors.response.use((response) => { return response; }); -let token = store.get('token'); -if (token) { - Axios.defaults.headers.common.Authorization = `Token ${token}`; -} +// Previously, we used to store an additional authentication token in local storage. +// Now we don't, and if the user still has one stored, we'll remove it to prevent +// unnecessary credential exposure. +store.remove('token'); function setAuthData(response: AxiosResponse): void { if (response.headers['set-cookie']) { @@ -370,18 +370,6 @@ function setAuthData(response: AxiosResponse): void { const cookies = response.headers['set-cookie'].join(';'); Axios.defaults.headers.common.Cookie = cookies; } - - if (response.data.key) { - token = response.data.key; - store.set('token', token); - Axios.defaults.headers.common.Authorization = `Token ${token}`; - } -} - -function removeAuthData(): void { - Axios.defaults.headers.common.Authorization = ''; - store.remove('token'); - token = null; } async function about(): Promise { @@ -474,7 +462,6 @@ async function register( } async function login(credential: string, password: string): Promise { - removeAuthData(); let authenticationResponse = null; try { authenticationResponse = await Axios.post(`${config.backendAPI}/auth/login`, { @@ -491,7 +478,6 @@ async function login(credential: string, password: string): Promise { async function logout(): Promise { try { await Axios.post(`${config.backendAPI}/auth/logout`); - removeAuthData(); } catch (errorData) { throw generateError(errorData); } @@ -570,17 +556,9 @@ async function getSelf(): Promise { async function authenticated(): Promise { try { - // In CVAT app we use two types of authentication - // At first we check if authentication token is present - // Request in getSelf will provide correct authentication cookies - if (!store.get('token')) { - removeAuthData(); - return false; - } await getSelf(); } catch (serverError) { if (serverError.code === 401) { - removeAuthData(); return false; } @@ -2345,7 +2323,6 @@ async function calculateAnalyticsReport( export default Object.freeze({ server: Object.freeze({ setAuthData, - removeAuthData, about, share, formats, diff --git a/cvat/apps/iam/authentication.py b/cvat/apps/iam/authentication.py index 6bffc8366b6e..060b840efc51 100644 --- a/cvat/apps/iam/authentication.py +++ b/cvat/apps/iam/authentication.py @@ -52,21 +52,6 @@ def unsign(self, signature, url): except User.DoesNotExist: raise signing.BadSignature() -# Even with token authentication it is very important to have a valid session id -# in cookies because in some cases we cannot use token authentication (e.g. when -# we redirect to the server in UI using just URL). To overkill that we override -# the class to call `login` method which restores the session id in cookies. -class TokenAuthenticationEx(TokenAuthentication): - def authenticate(self, request): - auth = super().authenticate(request) - # drf_spectacular uses mock requests without session field - session = getattr(request, 'session', None) - if (auth is not None and - session is not None and - (session.session_key is None or (not session.modified and not session.load()))): - login(request, auth[0], 'django.contrib.auth.backends.ModelBackend') - return auth - class SignatureAuthentication(BaseAuthentication): """ Authentication backend for signed URLs. diff --git a/cvat/settings/base.py b/cvat/settings/base.py index 6b1158690b21..bfd7914eba7a 100644 --- a/cvat/settings/base.py +++ b/cvat/settings/base.py @@ -134,7 +134,7 @@ def generate_secret_key(): 'cvat.apps.iam.permissions.PolicyEnforcer', ], 'DEFAULT_AUTHENTICATION_CLASSES': [ - 'cvat.apps.iam.authentication.TokenAuthenticationEx', + 'rest_framework.authentication.TokenAuthentication', 'cvat.apps.iam.authentication.SignatureAuthentication', 'rest_framework.authentication.SessionAuthentication', 'rest_framework.authentication.BasicAuthentication' diff --git a/tests/cypress/e2e/issues_prs/pr_5331_missing_authentication_data.js b/tests/cypress/e2e/issues_prs/pr_5331_missing_authentication_data.js deleted file mode 100644 index 6821234cba87..000000000000 --- a/tests/cypress/e2e/issues_prs/pr_5331_missing_authentication_data.js +++ /dev/null @@ -1,34 +0,0 @@ -// Copyright (C) 2023 CVAT.ai Corporation -// -// SPDX-License-Identifier: MIT - -/// - -context('Check behavior in case of missing authentification data', () => { - const prId = '5331'; - - before(() => { - cy.visit('auth/login'); - }); - - describe(`Testing pr "${prId}"`, () => { - it('Auto logout if authentication token is missing', () => { - cy.login(); - cy.clearLocalStorage('token'); - cy.reload(); - cy.get('.cvat-login-form-wrapper').should('exist'); - }); - - it('Cookies are set correctly if only token is present', () => { - cy.login(); - cy.get('.cvat-tasks-page').should('exist'); - cy.clearCookies(); - cy.getCookies() - .should('have.length', 0) - .then(() => { - cy.reload(); - cy.get('.cvat-tasks-page').should('exist'); - }); - }); - }); -});