Merge branch 'feat/return_information_about_forbidden_from_server' in…

…to 'main' feat: Get all information about Forbidden error from the server Closes PACMAN-1020 See merge request espressif/idf-component-manager!472
espressif · Dec 4, 2024 · fdf1111 · fdf1111
2 parents bfaabc8 + b1c404b
commit fdf1111
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 71 deletions.
diff --git a/idf_component_tools/registry/request_processor.py b/idf_component_tools/registry/request_processor.py
@@ -108,7 +108,6 @@ def handle_response_errors(
     response: requests.Response,
     endpoint: str,
     use_storage: bool,
-    token_scope: str,
 ) -> t.Dict:
     if response.status_code == HTTPStatus.NO_CONTENT:
         return {}
@@ -121,7 +120,7 @@ def handle_response_errors(
                 endpoint=endpoint,
                 status_code=response.status_code,
             )
-        handle_4xx_error(response, token_scope)
+        handle_4xx_error(response)
     elif 500 <= response.status_code < 600:
         raise APIClientError(
             'Internal server error happened while processing request.',
@@ -132,7 +131,7 @@ def handle_response_errors(
     return response.json()
 
 
-def handle_4xx_error(response: requests.Response, token_scope: str) -> None:
+def handle_4xx_error(response: requests.Response) -> None:
     if response.status_code == HTTPStatus.REQUEST_ENTITY_TOO_LARGE:
         raise ContentTooLargeError(
             'Error during request. The provided content is too large '
@@ -142,17 +141,7 @@ def handle_4xx_error(response: requests.Response, token_scope: str) -> None:
         )
 
     if response.status_code == HTTPStatus.FORBIDDEN:
-        if 'write:components' not in token_scope.split():
-            raise APIClientError(
-                f'Your token does not have permissions to perform this action. URL: {response.url}. '
-                f'Token scope: {token_scope}'
-            )
-        else:
-            raise APIClientError(
-                'You do not have namespace/component role to perform this action. '
-                'Contact the namespace/component owner/maintainer to add your ESP Component Registry '
-                f'account to the namespace/component. URL: {response.url}',
-            )
+        raise APIClientError(' '.join(response.json()['messages']) + f'\nURL: {response.url}')
 
     try:
         error = ErrorResponse.model_validate(response.json())
@@ -182,23 +171,6 @@ def handle_4xx_error(response: requests.Response, token_scope: str) -> None:
         )
 
 
-def get_token_scope(
-    method: str,
-    session: requests.Session,
-    url: str,
-    path: t.List[str],
-    timeout: t.Union[float, t.Tuple[float, float]],
-) -> str:
-    # Check if user is uploading a component
-    if path[-1] == 'versions' and method == 'post':
-        # Check if the token has write permissions
-        response = make_request(session, url + '/api/tokens/current', None, None, None, timeout)
-        scope = response.json()['scope'] if response.status_code == HTTPStatus.OK else ''
-        handle_response_errors(response, url + '/api/tokens/current', False, scope)
-        return scope
-    return ''
-
-
 def base_request(
     url: str,
     session: requests.Session,
@@ -220,9 +192,8 @@ def base_request(
     if request_timeout is None:
         request_timeout = DEFAULT_REQUEST_TIMEOUT
 
-    token_scope = get_token_scope(method, session, url, path, request_timeout)
     response = make_request(session, endpoint, data, json, headers, request_timeout, method=method)
-    response_json = handle_response_errors(response, endpoint, use_storage, token_scope)
+    response_json = handle_response_errors(response, endpoint, use_storage)
 
     if schema is None:
         return response_json

diff --git a/tests/test_api_client.py b/tests/test_api_client.py
@@ -2,6 +2,7 @@
 # SPDX-License-Identifier: Apache-2.0
 import logging
 import os
+import typing as t
 from ssl import SSLEOFError
 
 import pytest
@@ -41,20 +42,13 @@ def response_413(*_, **__):
     return response
 
 
-def response_403(*_, **__):
+def response_403(messages: t.List[str], *_, **__):
     response = Response()
     response.status_code = 403
+    response.json = lambda: {'messages': messages}  # type: ignore
     return response
 
 
-def get_token_scope_user(*_, **__):
-    return 'user'
-
-
-def get_token_scope_write(*_, **__):
-    return 'write:components'
-
-
 def raise_SSLEOFError(*_, **__):
     raise SSLEOFError()
 
@@ -271,62 +265,51 @@ def test_upload_component_returns_413_status(self, tmp_path, registry_url, monke
         assert str(e.value).startswith('The component archive exceeds the maximum allowed size')
 
     def test_upload_component_token_forbidden(self, tmp_path, registry_url, monkeypatch):
+        messages = ['Your token does not have the required scope: write:components']
         monkeypatch.setattr(
-            'idf_component_tools.registry.request_processor.make_request', response_403
-        )
-        monkeypatch.setattr(
-            'idf_component_tools.registry.request_processor.get_token_scope',
-            get_token_scope_user,
+            'idf_component_tools.registry.request_processor.make_request',
+            lambda *_, **__: response_403(messages),
         )
+
         client = APIClient(
             registry_url=registry_url,
             api_token='test',
         )
 
-        file_path = str(tmp_path / 'cmp.tgz')
-        with open(file_path, 'w+') as f:
-            f.write('a')
+        (tmp_path / 'cmp.tgz').touch()
 
         with pytest.raises(APIClientError) as e:
             client.upload_version(
                 component_name='kumekay/cmp',
-                file_path=file_path,
+                file_path=tmp_path / 'cmp.tgz',
             )
-        assert str(e.value).startswith(
-            'Your token does not have permissions to perform this action.'
-        )
-        assert 'Token scope: user' in str(e.value)
+        assert str(e.value).startswith(messages[0])
 
     def test_upload_component_role_forbidden(
         self,
         tmp_path,
         registry_url,
         monkeypatch,
     ):
+        messages = ['You do not have a required role, to access a']
         monkeypatch.setattr(
-            'idf_component_tools.registry.request_processor.make_request', response_403
-        )
-        monkeypatch.setattr(
-            'idf_component_tools.registry.request_processor.get_token_scope',
-            get_token_scope_write,
+            'idf_component_tools.registry.request_processor.make_request',
+            lambda *_, **__: response_403(messages),
         )
+
         client = APIClient(
             registry_url=registry_url,
             api_token='test',
         )
 
-        file_path = str(tmp_path / 'cmp.tgz')
-        with open(file_path, 'w+') as f:
-            f.write('a')
+        (tmp_path / 'cmp.tgz').touch()
 
         with pytest.raises(APIClientError) as e:
             client.upload_version(
                 component_name='kumekay/cmp',
-                file_path=file_path,
+                file_path=tmp_path / 'cmp.tgz',
             )
-        assert str(e.value).startswith(
-            'You do not have namespace/component role to perform this action.'
-        )
+        assert str(e.value).startswith(messages[0])
 
     def test_upload_component_SSLEOFError(self, tmp_path, registry_url, monkeypatch):
         monkeypatch.setattr(
@@ -338,13 +321,11 @@ def test_upload_component_SSLEOFError(self, tmp_path, registry_url, monkeypatch)
             api_token='test',
         )
 
-        file_path = str(tmp_path / 'cmp.tgz')
-        with open(file_path, 'w+') as f:
-            f.write('a')
+        (tmp_path / 'cmp.tgz').touch()
 
         with pytest.raises(APIClientError) as e:
             client.upload_version(
                 component_name='kumekay/cmp',
-                file_path=file_path,
+                file_path=tmp_path / 'cmp.tgz',
             )
         assert str(e.value).startswith('The component archive exceeds the maximum allowed size')