From 487b95c2076d94dedb8c513636d2737409547bf9 Mon Sep 17 00:00:00 2001 From: Manuel Raynaud Date: Sat, 21 Mar 2026 09:22:45 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B(backend)=20allow=20using=20search?= =?UTF-8?q?=20endpoint=20without=20refresh=20token=20enabled?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The search endpoint was using the refresh_roken method decorator. This decorator force having a valid refresh token stored in the session for the entire viewset. The search endpoint still allow having the legacy search behavior and for this we don't need to configure at all the OIDC refrsh mechanism. --- CHANGELOG.md | 4 +++ env.d/development/common | 4 +-- src/backend/core/api/utils.py | 15 ++++++++ src/backend/core/api/viewsets.py | 5 +-- .../documents/test_api_documents_search.py | 36 ++++++++++++------- ...pi_utils_conditional_refresh_oidc_token.py | 32 +++++++++++++++++ 6 files changed, 78 insertions(+), 18 deletions(-) create mode 100644 src/backend/core/tests/test_api_utils_conditional_refresh_oidc_token.py diff --git a/CHANGELOG.md b/CHANGELOG.md index d959d96f..0d9b4fd4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,10 @@ and this project adheres to - ♿️(frontend) fix aria-labels for table of contents #2065 +### Fixed + +- 🐛(backend) allow using search endpoint without refresh token enabled #2097 + ## [v4.8.2] - 2026-03-19 ### Added diff --git a/env.d/development/common b/env.d/development/common index 7ea51a34..4be4227d 100644 --- a/env.d/development/common +++ b/env.d/development/common @@ -61,8 +61,8 @@ OIDC_RS_AUDIENCE_CLAIM="client_id" # The claim used to identify the audience OIDC_RS_ALLOWED_AUDIENCES="" # Store OIDC tokens in the session. Needed by search/ endpoint. -OIDC_STORE_ACCESS_TOKEN=True -OIDC_STORE_REFRESH_TOKEN=True # Store the encrypted refresh token in the session. +# OIDC_STORE_ACCESS_TOKEN=True +# OIDC_STORE_REFRESH_TOKEN=True # Store the encrypted refresh token in the session. # Must be a valid Fernet key (32 url-safe base64-encoded bytes) # To create one, use the bin/fernetkey command. diff --git a/src/backend/core/api/utils.py b/src/backend/core/api/utils.py index 98dc6548..3a78bccb 100644 --- a/src/backend/core/api/utils.py +++ b/src/backend/core/api/utils.py @@ -6,8 +6,10 @@ from abc import ABC, abstractmethod from django.conf import settings from django.core.cache import cache from django.core.files.storage import default_storage +from django.utils.decorators import method_decorator import botocore +from lasuite.oidc_login.decorators import refresh_oidc_access_token from rest_framework.throttling import BaseThrottle @@ -91,6 +93,19 @@ def generate_s3_authorization_headers(key): return request +def conditional_refresh_oidc_token(func): + """ + Conditionally apply refresh_oidc_access_token decorator. + + The decorator is only applied if OIDC_STORE_REFRESH_TOKEN is True, meaning + we can actually refresh something. Broader settings checks are done in settings.py. + """ + if settings.OIDC_STORE_REFRESH_TOKEN: + return method_decorator(refresh_oidc_access_token)(func) + + return func + + class AIBaseRateThrottle(BaseThrottle, ABC): """Base throttle class for AI-related rate limiting with backoff.""" diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index a1d13221..3296e9f0 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -25,7 +25,6 @@ from django.db.models.functions import Greatest, Left, Length from django.http import Http404, StreamingHttpResponse from django.urls import reverse from django.utils import timezone -from django.utils.decorators import method_decorator from django.utils.functional import cached_property from django.utils.http import content_disposition_header from django.utils.text import capfirst, slugify @@ -38,7 +37,6 @@ from botocore.exceptions import ClientError from csp.constants import NONE from csp.decorators import csp_update from lasuite.malware_detection import malware_detection -from lasuite.oidc_login.decorators import refresh_oidc_access_token from lasuite.tools.email import get_domain_from_email from pydantic import ValidationError as PydanticValidationError from rest_framework import filters, status, viewsets @@ -1415,7 +1413,7 @@ class DocumentViewSet( return duplicated_document @drf.decorators.action(detail=False, methods=["get"], url_path="search") - @method_decorator(refresh_oidc_access_token) + @utils.conditional_refresh_oidc_token def search(self, request, *args, **kwargs): """ Returns an ordered list of documents best matching the search query parameter 'q'. @@ -1426,7 +1424,6 @@ class DocumentViewSet( params = serializers.SearchDocumentSerializer(data=request.query_params) params.is_valid(raise_exception=True) search_type = self._get_search_type() - if search_type == SearchType.TITLE: return self._title_search(request, params.validated_data, *args, **kwargs) diff --git a/src/backend/core/tests/documents/test_api_documents_search.py b/src/backend/core/tests/documents/test_api_documents_search.py index f6c8dd8b..bb33fa10 100644 --- a/src/backend/core/tests/documents/test_api_documents_search.py +++ b/src/backend/core/tests/documents/test_api_documents_search.py @@ -70,17 +70,20 @@ def test_api_documents_search_anonymous(search_query, indexer_settings): @mock.patch("core.api.viewsets.DocumentViewSet.list") -def test_api_documents_search_fall_back_on_search_list(mock_list, indexer_settings): +def test_api_documents_search_fall_back_on_search_list(mock_list, settings): """ When indexer is not configured and no path is provided, should fall back on list method """ - indexer_settings.SEARCH_URL = None assert get_document_indexer() is None + assert settings.OIDC_STORE_REFRESH_TOKEN is False + assert settings.OIDC_STORE_ACCESS_TOKEN is False user = factories.UserFactory() client = APIClient() - client.force_login(user) + client.force_login( + user, backend="core.authentication.backends.OIDCAuthenticationBackend" + ) mocked_response = { "count": 0, @@ -93,6 +96,8 @@ def test_api_documents_search_fall_back_on_search_list(mock_list, indexer_settin q = "alpha" response = client.get("/api/v1.0/documents/search/", data={"q": q}) + assert response.status_code == 200 + assert mock_list.call_count == 1 assert mock_list.call_args[0][0].GET.get("q") == q assert response.json() == mocked_response @@ -100,18 +105,21 @@ def test_api_documents_search_fall_back_on_search_list(mock_list, indexer_settin @mock.patch("core.api.viewsets.DocumentViewSet._list_descendants") def test_api_documents_search_fallback_on_search_list_sub_docs( - mock_list_descendants, indexer_settings + mock_list_descendants, settings ): """ When indexer is not configured and path parameter is provided, should call _list_descendants() method """ - indexer_settings.SEARCH_URL = "http://find/api/v1.0/search" - assert get_document_indexer() is not None + assert get_document_indexer() is None + assert settings.OIDC_STORE_REFRESH_TOKEN is False + assert settings.OIDC_STORE_ACCESS_TOKEN is False user = factories.UserFactory() client = APIClient() - client.force_login(user) + client.force_login( + user, backend="core.authentication.backends.OIDCAuthenticationBackend" + ) parent = factories.DocumentFactory(title="parent", users=[user]) @@ -128,9 +136,9 @@ def test_api_documents_search_fallback_on_search_list_sub_docs( "/api/v1.0/documents/search/", data={"q": q, "path": parent.path} ) - assert mock_list_descendants.call_count == 1 - assert mock_list_descendants.call_args[0][0].GET.get("q") == q - assert mock_list_descendants.call_args[0][0].GET.get("path") == parent.path + mock_list_descendants.assert_called_with( + mock.ANY, {"q": "alpha", "path": parent.path} + ) assert response.json() == mocked_response @@ -152,7 +160,9 @@ def test_api_documents_search_indexer_crashes(mock_title_search, indexer_setting user = factories.UserFactory() client = APIClient() - client.force_login(user) + client.force_login( + user, backend="core.authentication.backends.OIDCAuthenticationBackend" + ) mocked_response = { "count": 0, @@ -185,7 +195,9 @@ def test_api_documents_search_invalid_params(indexer_settings): user = factories.UserFactory() client = APIClient() - client.force_login(user) + client.force_login( + user, backend="core.authentication.backends.OIDCAuthenticationBackend" + ) response = client.get("/api/v1.0/documents/search/") diff --git a/src/backend/core/tests/test_api_utils_conditional_refresh_oidc_token.py b/src/backend/core/tests/test_api_utils_conditional_refresh_oidc_token.py new file mode 100644 index 00000000..92cc39ad --- /dev/null +++ b/src/backend/core/tests/test_api_utils_conditional_refresh_oidc_token.py @@ -0,0 +1,32 @@ +"""module testing the conditional_refresh_oidc_token utils.""" + +from unittest import mock + +from core.api import utils + + +def test_refresh_oidc_access_token_storing_refresh_token_disabled(settings): + """The method_decorator must not be called when OIDC_STORE_REFRESH_TOKEN is False.""" + + settings.OIDC_STORE_REFRESH_TOKEN = False + + callback = mock.MagicMock() + + with mock.patch.object(utils, "method_decorator") as mock_method_decorator: + result = utils.conditional_refresh_oidc_token(callback) + + mock_method_decorator.assert_not_called() + assert result == callback + + +def test_refresh_oidc_access_token_storing_refresh_token_enabled(settings): + """The method_decorator must not be called when OIDC_STORE_REFRESH_TOKEN is False.""" + + settings.OIDC_STORE_REFRESH_TOKEN = True + + callback = mock.MagicMock() + + with mock.patch.object(utils, "method_decorator") as mock_method_decorator: + utils.conditional_refresh_oidc_token(callback) + + mock_method_decorator.assert_called_with(utils.refresh_oidc_access_token)