From 9aeb115ed6ff202bb86a5d7a9c112898cc261c2e Mon Sep 17 00:00:00 2001 From: Roman Donchenko Date: Fri, 9 Aug 2024 13:00:52 +0300 Subject: [PATCH 1/3] Fix incorrect URL joining in `UploadMixin.init_tus_upload` Currently, if `location` has a query string, the result is nonsense. --- cvat/apps/engine/mixins.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cvat/apps/engine/mixins.py b/cvat/apps/engine/mixins.py index b0ab8315ae09..b4cfb279c7dd 100644 --- a/cvat/apps/engine/mixins.py +++ b/cvat/apps/engine/mixins.py @@ -14,6 +14,7 @@ from unittest import mock from textwrap import dedent from typing import Optional, Callable, Dict, Any, Mapping +from urllib.parse import urljoin import django_rq from attr.converters import to_bool @@ -315,7 +316,7 @@ def init_tus_upload(self, request): return self._tus_response( status=status.HTTP_201_CREATED, - extra_headers={'Location': '{}{}'.format(location, tus_file.file_id), + extra_headers={'Location': urljoin(location, tus_file.file_id), 'Upload-Filename': tus_file.filename}) def append_tus_chunk(self, request, file_id): From 8a74cd9af56b7584e6d754bc895961da0ab42d6d Mon Sep 17 00:00:00 2001 From: Roman Donchenko Date: Thu, 8 Aug 2024 18:37:05 +0300 Subject: [PATCH 2/3] Layer tus-js-client on top of Axios This ensures that requests coming from tus-js-client have the same defaults as the ones coming from the rest of the UI. In particular, this ensures that TUS requests include the `X-CSRFTOKEN` header. Currently, this doesn't matter much, because TUS requests are authenticated using the token. However, I'd like to get rid of token authentication in the UI, after which `X-CSRFTOKEN` will become important. --- cvat-core/package.json | 2 +- cvat-core/src/axios-tus.ts | 65 +++++++++++++++++++++++++++++++++++ cvat-core/src/server-proxy.ts | 12 ++----- 3 files changed, 68 insertions(+), 11 deletions(-) create mode 100644 cvat-core/src/axios-tus.ts diff --git a/cvat-core/package.json b/cvat-core/package.json index 030b2cb0e497..a93411284bc1 100644 --- a/cvat-core/package.json +++ b/cvat-core/package.json @@ -1,6 +1,6 @@ { "name": "cvat-core", - "version": "15.1.0", + "version": "15.1.1", "type": "module", "description": "Part of Computer Vision Tool which presents an interface for client-side integration", "main": "src/api.ts", diff --git a/cvat-core/src/axios-tus.ts b/cvat-core/src/axios-tus.ts new file mode 100644 index 000000000000..4a56b7860332 --- /dev/null +++ b/cvat-core/src/axios-tus.ts @@ -0,0 +1,65 @@ +// Copyright (C) 2024 CVAT.ai Corporation +// +// SPDX-License-Identifier: MIT + +import Axios, { AxiosRequestConfig, AxiosResponse } from 'axios'; +import * as tus from 'tus-js-client'; + +class AxiosHttpResponse implements tus.HttpResponse { + readonly #axiosResponse: AxiosResponse; + + constructor(axiosResponse: AxiosResponse) { + this.#axiosResponse = axiosResponse; + } + + getStatus(): number { return this.#axiosResponse.status; } + getHeader(header: string): string { return this.#axiosResponse.headers[header.toLowerCase()]; } + getBody(): string { return this.#axiosResponse.data; } + getUnderlyingObject(): any { return this.#axiosResponse; } +} + +class AxiosHttpRequest implements tus.HttpRequest { + readonly #axiosConfig: AxiosRequestConfig; + readonly #abortController: AbortController; + + constructor(method: string, url: string) { + this.#abortController = new AbortController(); + this.#axiosConfig = { + method, url, headers: {}, signal: this.#abortController.signal, + }; + } + + getMethod(): string { return this.#axiosConfig.method; } + getURL(): string { return this.#axiosConfig.url; } + + setHeader(header: string, value: string): void { + this.#axiosConfig.headers[header.toLowerCase()] = value; + } + getHeader(header: string): string | undefined { + return this.#axiosConfig.headers[header.toLowerCase()]; + } + + setProgressHandler(handler: (bytesSent: number) => void): void { + this.#axiosConfig.onUploadProgress = (progressEvent) => { + handler(progressEvent.loaded); + }; + } + + async send(body: any): Promise { + const axiosResponse = await Axios({ ...this.#axiosConfig, data: body }); + return new AxiosHttpResponse(axiosResponse); + } + + async abort(): Promise { this.#abortController.abort(); } + + getUnderlyingObject(): any { return this.#axiosConfig; } +} + +class AxiosHttpStack implements tus.HttpStack { + createRequest(method: string, url: string): tus.HttpRequest { + return new AxiosHttpRequest(method, url); + } + getName(): string { return 'AxiosHttpStack'; } +} + +export const axiosTusHttpStack = new AxiosHttpStack(); diff --git a/cvat-core/src/server-proxy.ts b/cvat-core/src/server-proxy.ts index caa945d8448f..1400b1e3db3f 100644 --- a/cvat-core/src/server-proxy.ts +++ b/cvat-core/src/server-proxy.ts @@ -10,6 +10,7 @@ import * as tus from 'tus-js-client'; import { ChunkQuality } from 'cvat-data'; import './axios-config'; +import { axiosTusHttpStack } from './axios-tus'; import { SerializedLabel, SerializedAnnotationFormats, ProjectsFilter, SerializedProject, SerializedTask, TasksFilter, SerializedUser, SerializedOrganization, @@ -117,7 +118,6 @@ function fetchAll(url, filter = {}): Promise { } async function chunkUpload(file: File, uploadConfig): Promise<{ uploadSentSize: number; filename: string }> { - const params = enableOrganization(); const { endpoint, chunkSize, totalSize, onUpdate, metadata, totalSentSize, } = uploadConfig; @@ -130,9 +130,7 @@ async function chunkUpload(file: File, uploadConfig): Promise<{ uploadSentSize: filetype: file.type, ...metadata, }, - headers: { - Authorization: Axios.defaults.headers.common.Authorization, - }, + httpStack: axiosTusHttpStack, chunkSize, retryDelays: [2000, 4000, 8000, 16000, 32000, 64000], onShouldRetry(err: tus.DetailedError | Error): boolean { @@ -151,12 +149,6 @@ async function chunkUpload(file: File, uploadConfig): Promise<{ uploadSentSize: onError(error) { reject(error); }, - onBeforeRequest(req) { - const xhr = req.getUnderlyingObject(); - const { org } = params; - req.setHeader('X-Organization', org); - xhr.withCredentials = true; - }, onProgress(bytesUploaded) { if (onUpdate && Number.isInteger(totalSentSize) && Number.isInteger(totalSize)) { const currentUploadedSize = totalSentSize + bytesUploaded; From 446c4f048efa5ed8ff08dc2d6849a18a47922cdb Mon Sep 17 00:00:00 2001 From: Roman Donchenko Date: Thu, 8 Aug 2024 19:57:26 +0300 Subject: [PATCH 3/3] Prolong user sessions while they are used Currently, this shouldn't have any visible effect, because the UI uses token authentication alongside session cookies, and the tokens last indefinitely. However, I'd like to end this practice and rely solely on session cookies. When that's implemented, the user will get logged out as soon as the session cookie expires, or the server-side session data expires (which should happen at the same time). This will irritate users if it happens too often (or worse, in the middle of their work). Therefore, we should prolong a session as long as it is used. --- cvat/apps/iam/middleware.py | 64 +++++++++++++++++++++++++++++++++++++ cvat/settings/base.py | 1 + 2 files changed, 65 insertions(+) diff --git a/cvat/apps/iam/middleware.py b/cvat/apps/iam/middleware.py index 2139e639cc62..f2f1a4bae2e0 100644 --- a/cvat/apps/iam/middleware.py +++ b/cvat/apps/iam/middleware.py @@ -2,9 +2,13 @@ # # SPDX-License-Identifier: MIT +from datetime import timedelta +from typing import Callable + from django.utils.functional import SimpleLazyObject from rest_framework.exceptions import ValidationError, NotFound from django.conf import settings +from django.http import HttpRequest, HttpResponse def get_organization(request): @@ -57,3 +61,63 @@ def __call__(self, request): request.iam_context = SimpleLazyObject(lambda: get_organization(request)) return self.get_response(request) + +class SessionRefreshMiddleware: + """ + Implements behavior similar to SESSION_SAVE_EVERY_REQUEST=True, but instead of + saving the session on every request, saves it at most once per REFRESH_INTERVAL. + This is accomplished by setting a parallel cookie that expires whenever the session + needs to be prolonged. + + This ensures that user sessions are automatically prolonged while in use, + but avoids making an extra DB query on every HTTP request. + + Must be listed after SessionMiddleware in the MIDDLEWARE list. + """ + + _REFRESH_INTERVAL = timedelta(days=1) + _COOKIE_NAME = "sessionfresh" + + def __init__(self, get_response: Callable[[HttpRequest], HttpResponse]) -> None: + self.get_response = get_response + + def __call__(self, request: HttpRequest) -> HttpResponse: + response = self.get_response(request) + + shared_cookie_args = { + "key": self._COOKIE_NAME, + "domain": getattr(settings, "SESSION_COOKIE_DOMAIN"), + "path": getattr(settings, "SESSION_COOKIE_PATH", "/"), + "samesite": getattr(settings, "SESSION_COOKIE_SAMESITE", "Lax"), + } + + if request.session.is_empty(): + if self._COOKIE_NAME in request.COOKIES: + response.delete_cookie(**shared_cookie_args) + return response + + if self._COOKIE_NAME in request.COOKIES: + # Session is still fresh. + return response + + if response.status_code >= 500: + # SessionMiddleware does not save the session for 5xx responses, + # so we should not set our cookie either. + return response + + response.set_cookie( + **shared_cookie_args, + value="1", + max_age=min( + self._REFRESH_INTERVAL, + # Refresh no later than after half of the session lifetime. + timedelta(seconds=request.session.get_expiry_age() // 2), + ), + httponly=True, + secure=getattr(settings, "SESSION_COOKIE_SECURE", False), + ) + + # Force SessionMiddleware to re-save the session. + request.session.modified = True + + return response diff --git a/cvat/settings/base.py b/cvat/settings/base.py index 1f4b3592d696..6b1158690b21 100644 --- a/cvat/settings/base.py +++ b/cvat/settings/base.py @@ -183,6 +183,7 @@ def generate_secret_key(): MIDDLEWARE = [ 'django.middleware.security.SecurityMiddleware', 'django.contrib.sessions.middleware.SessionMiddleware', + 'cvat.apps.iam.middleware.SessionRefreshMiddleware', 'corsheaders.middleware.CorsMiddleware', 'django.middleware.common.CommonMiddleware', 'django.middleware.csrf.CsrfViewMiddleware',