[REF] fastapi_auth_api_key: review improvements

Author: petrus-v

fastapi_auth_api_key/README.rst

@@ -1,5 +1,5 @@
 ====================
-Fastapi Auth Api Key
+Fastapi Auth API Key
 ====================
 
 .. 
@@ -95,8 +95,8 @@ Authors
 Contributors
 ------------
 
--  Matthieu Méquignon <matthieu.mequignon@camptocamp.com>
--  Son Ho <sonhd@trobz.com>
+- Matthieu Méquignon <matthieu.mequignon@camptocamp.com>
+- Son Ho <sonhd@trobz.com>
 
 Other credits
 -------------

fastapi_auth_api_key/__manifest__.py

@@ -2,7 +2,7 @@
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl)
 
 {
-    "name": "Fastapi Auth Api Key",
+    "name": "Fastapi Auth API Key",
     "version": "17.0.1.0.0",
     "category": "Others",
     "website": "https://github.com/OCA/rest-framework",

fastapi_auth_api_key/dependencies.py

@@ -1,9 +1,9 @@
 # Copyright 2024 Camptocamp SA
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl)
-
+import os
 from typing import Annotated
 
-from odoo import SUPERUSER_ID
+from odoo import SUPERUSER_ID, _
 from odoo.api import Environment
 from odoo.exceptions import ValidationError
 
@@ -16,17 +16,20 @@
 from fastapi.exceptions import HTTPException
 from fastapi.security import APIKeyHeader
 
+HTTP_API_KEY_HEADER = os.environ.get("FASTAPI_AUTH_HTTP_API_KEY_HEADER", "HTTP-API-KEY")
+
 
 def authenticated_auth_api_key(
-    key: Annotated[str, Depends(APIKeyHeader(name="HTTP-API-KEY"))],
+    key: Annotated[str, Depends(APIKeyHeader(name=HTTP_API_KEY_HEADER))],
     env: Annotated[Environment, Depends(odoo_env)],
     endpoint: Annotated[FastapiEndpoint, Depends(fastapi_endpoint)],
 ) -> AuthApiKey:
     if not key:
         raise HTTPException(
             status_code=status.HTTP_401_UNAUTHORIZED,
-            detail="No HTTP-API-KEY provided",
-            headers={"WWW-Authenticate": "HTTP-API-KEY"},
+            detail=_("Missing %(HTTP_API_KEY_HEADER)s header")
+            % {"HTTP_API_KEY_HEADER": HTTP_API_KEY_HEADER},
+            headers={"WWW-Authenticate": HTTP_API_KEY_HEADER},
         )
     admin_env = Environment(env.cr, SUPERUSER_ID, {})
     try:
@@ -35,14 +38,17 @@ def authenticated_auth_api_key(
         raise HTTPException(
             status_code=status.HTTP_401_UNAUTHORIZED,
             detail=error.args,
-            headers={"WWW-Authenticate": "HTTP-API-KEY"},
+            headers={"WWW-Authenticate": HTTP_API_KEY_HEADER},
         ) from error
     # Ensure the api key is authorized for the current endpoint.
-    if auth_api_key not in endpoint.sudo().auth_api_key_group_id.auth_api_key_ids:
+    if (
+        endpoint.sudo().auth_api_key_group_id
+        and auth_api_key not in endpoint.sudo().auth_api_key_group_id.auth_api_key_ids
+    ):
         raise HTTPException(
             status_code=status.HTTP_401_UNAUTHORIZED,
-            detail="Unauthorized",
-            headers={"WWW-Authenticate": "HTTP-API-KEY"},
+            detail=_("Unauthorized"),
+            headers={"WWW-Authenticate": HTTP_API_KEY_HEADER},
         )
     return auth_api_key
 

fastapi_auth_api_key/models/fastapi_endpoint.py

@@ -7,4 +7,7 @@
 class FastapiEndpoint(models.Model):
     _inherit = "fastapi.endpoint"
 
-    auth_api_key_group_id = fields.Many2one("auth.api.key.group")
+    auth_api_key_group_id = fields.Many2one(
+        "auth.api.key.group",
+        help="If not set, all 'auth.api.key' are allowed to access to endpoints",
+    )

fastapi_auth_api_key/tests/test_fastapi_api_key_dependencies.py

@@ -1,22 +1,21 @@
 from odoo.tests import tagged
 from odoo.tests.common import TransactionCase
 
-from fastapi.exceptions import HTTPException
-
-from ..dependencies import (
+from odoo.addons.fastapi_auth_api_key.dependencies import (
     authenticated_auth_api_key,
     authenticated_env_by_auth_api_key,
     authenticated_partner_by_api_key,
 )
 
+from fastapi.exceptions import HTTPException
+
 
 @tagged("-at_install", "post_install")
 class TestFastapiAuthApiKey(TransactionCase):
     @classmethod
     def setUpClass(cls):
         super().setUpClass()
         cls.env = cls.env(context=dict(cls.env.context, tracking_disable=True))
-        # Is it valid? We need an env without superpowers
         demo_user = cls.env.ref("base.user_demo")
         cls.demo_env = demo_user.with_user(demo_user).env
         cls.demo_endpoint = cls.env["fastapi.endpoint"].create(
@@ -68,29 +67,41 @@ def setUpClassApiKey(cls):
                 "auth_api_key_ids": [(6, 0, cls.authorized_api_key.ids)],
             }
         )
+        cls.demo_endpoint.auth_api_key_group_id = cls.authorized_api_key_group
 
-    def test_authenticated_auth_api_key(self):
+    def test_authenticated_auth_api_key_no_api_key(self):
         # An exception is raised when no api key is used
         with self.assertRaises(HTTPException) as error:
             authenticated_auth_api_key(False, self.demo_env, self.demo_endpoint)
-        self.assertEqual(error.exception.detail, "No HTTP-API-KEY provided")
+        self.assertEqual(error.exception.detail, "Missing HTTP-API-KEY header")
+
+    def test_authenticated_auth_api_key_no_api_key_found(self):
         # An exception is raised when no api key record is found
         with self.assertRaises(HTTPException) as error:
             authenticated_auth_api_key("404", self.demo_env, self.demo_endpoint)
         self.assertEqual(error.exception.detail, ("The key 404 is not allowed",))
-        # TODO enable this when we know how to filter keys based
-        # on endpoint's api key group.
+
+    def test_authenticated_auth_api_key_unauthorized_key(self):
         # An exception is raised when unauthorized api key record is found
-        # with self.assertRaises(HTTPException) as error:
-        # authenticated_auth_api_key("not_authorized", self.demo_env)
-        self.demo_endpoint.auth_api_key_group_id = (
-            self.authorized_api_key.auth_api_key_group_ids[0]
-        )
+        with self.assertRaisesRegex(HTTPException, r"Unauthorized"):
+            authenticated_auth_api_key(
+                "unauthorized_key", self.demo_env, self.demo_endpoint
+            )
+
+    def test_authenticated_auth_api_key(self):
         result_key = authenticated_auth_api_key(
-            self.authorized_api_key.key, self.demo_env, self.demo_endpoint
+            "authorized_key", self.demo_env, self.demo_endpoint
         )
         self.assertEqual(result_key, self.authorized_api_key)
 
+    def test_authenticated_auth_api_key_without_group(self):
+        # test with no group set unauthorized is allow to access
+        self.demo_endpoint.auth_api_key_group_id = False
+        result_key = authenticated_auth_api_key(
+            "unauthorized_key", self.demo_env, self.demo_endpoint
+        )
+        self.assertEqual(result_key, self.unauthorized_api_key)
+
     def test_authenticated_partner_by_api_key(self):
         result_partner = authenticated_partner_by_api_key(self.authorized_api_key)
         self.assertEqual(result_partner, self.authorized_user.partner_id)