Stop using token authentication in the UI (#8289)

Currently, the UI authenticates with the server using two parallel
methods:

* A cookie set by the `/api/auth/login` endpoint.
* A token returned by the same endpoint.

This is redundant and confusing, and also causes several usability &
security issues:

* If a user creates 2 or more concurrent sessions (e.g. logs in on two
  computers), and then logs out of one of them, it will effectively log
  them out of all other sessions too. This happens because:

  1. The same token is shared between all sessions.
  2. Logging out destroys the token in the DB.
  3. The server tries to authenticate the browser using the token first,
      so if a browser presents a token that's no longer present in the DB, the
      server responds with a 401 (even if the cookie is still valid).

* When a user changes their password, Django invalidates all of that
  user's other sessions... except that doesn't work, because the user's
  token remains valid. This is bad, because if an attacker steals a user's
  password and logs in, the most obvious recourse (changing the password)
  will not work - the attacker will stay logged in.

* Sessions effectively last forever, because, while Django's session
  data expires after `SESSION_COOKIE_AGE`, the token never expires.

* The token is stored in local storage, so it could be stolen in an XSS
  attack. The session cookie is not susceptible to this, because it's
  marked `HttpOnly`.

The common theme in all these problems is the token, so by ceasing to
use it we can fix them all.

Note that this patch does not remove the server-side token creation &
authentication logic, or remove the token from the output of the
`/api/auth/login` endpoint. This is because that would break the
`Client.login` method in the SDK. However, I believe that in the future
we should get rid of the whole "generate token on login" logic, and let
users create API tokens explicitly if (and only if) they wish to use the
SDK.

changelog.d/20240815_143525_roman_no_token_in_ui_2.md

@@ -0,0 +1,15 @@
+### Fixed
+
+- Logging out of one session will no longer log the user out of all their
+  other sessions
+  (<https://github.com/cvat-ai/cvat/pull/8289>)
+
+### Changed
+
+- User sessions now expire after two weeks of inactivity
+  (<https://github.com/cvat-ai/cvat/pull/8289>)
+
+- A user changing their password will now invalidate all of their sessions
+  except for the current one
+  (<https://github.com/cvat-ai/cvat/pull/8289>)
+

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;

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;

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;
     };

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<SerializedAbout> {
@@ -474,7 +462,6 @@ async function register(
 }
 
 async function login(credential: string, password: string): Promise<void> {
-    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<void> {
 async function logout(): Promise<void> {
     try {
         await Axios.post(`${config.backendAPI}/auth/logout`);
-        removeAuthData();
     } catch (errorData) {
         throw generateError(errorData);
     }
@@ -570,17 +556,9 @@ async function getSelf(): Promise<SerializedUser> {
 
 async function authenticated(): Promise<boolean> {
     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,

cvat/apps/iam/authentication.py

@@ -5,8 +5,6 @@
 from django.core import signing
 from rest_framework import exceptions
 from rest_framework.authentication import BaseAuthentication
-from rest_framework.authentication import TokenAuthentication
-from django.contrib.auth import login
 from django.contrib.auth import get_user_model
 from furl import furl
 import hashlib
@@ -52,21 +50,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.

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'