refactor(spp_oauth,spp_api_v2_oauth): address staff engineer review findings

Rename abbreviated field/param names to full forms per naming conventions:
- oauth_priv_key → oauth_private_key
- oauth_pub_key → oauth_public_key
across models, config params, views, data, tests, and docs.

spp_oauth changes:
- Category: "OpenSPP" → "OpenSPP/Integration"
- ACL: restrict to base.group_system with perm_create=1
- Remove ERROR logging from exception constructor (let callers decide)
- Rename model class RegistryConfig → OAuthConfig
- Add wrong-key verification test
- Improve test assertions (assertIsNotNone) and remove numbered prefixes
- Fix stale doc references to removed logging behavior

spp_api_v2_oauth changes:
- Remove empty ir.model.access.csv (no new models)
- Add RS256 endpoint to API auth allowlist
- Fix nosemgrep annotation for multiline sudo()
- Remove Python <3.11 compat guard (Odoo 19 requires 3.11+)
- Import constants from ..constants instead of redeclaring
- Replace silent if-guards with skipTest() for endpoint tests
- Safe int() cast for token_lifetime_hours config
- asyncio.get_event_loop() → asyncio.run()
- Manual key restore → addCleanup for test safety
- SimpleNamespace for mock credentials

Author: gonzalesedwin1123

scripts/audit-api-auth.py

@@ -48,8 +48,9 @@
 # Format: (module_dir, router_file_basename, function_name)
 # Keep this list small and review changes carefully.
 ALLOWED_PUBLIC = {
-    # OAuth token endpoint - public by design
+    # OAuth token endpoints - public by design
     ("spp_api_v2", "oauth.py", "get_token"),
+    ("spp_api_v2_oauth", "oauth_rs256.py", "get_rs256_token"),
     # Capability/metadata discovery - public by design
     ("spp_api_v2", "metadata.py", "get_metadata"),
     # DCI callback endpoints - called by external systems

spp_api_v2_oauth/README.rst

@@ -167,7 +167,7 @@ verification):
 
    import jwt
    header = jwt.get_unverified_header(token)
-   print(header["alg"])  # "RS256" or "HS256"
+   # header["alg"] will be "RS256" or "HS256"
 
 Error Responses
 ~~~~~~~~~~~~~~~
@@ -185,6 +185,9 @@ Error Responses
 +---------------------------+-------------+---------------------------+
 | Invalid signature         | 401         | "Invalid token"           |
 +---------------------------+-------------+---------------------------+
+| Unsupported algorithm     | 401         | "Unsupported token        |
+|                           |             | algorithm: {alg}"         |
++---------------------------+-------------+---------------------------+
 | Rate limit exceeded       | 429         | "Rate limit exceeded"     |
 +---------------------------+-------------+---------------------------+
 

spp_api_v2_oauth/pyproject.toml

@@ -1,2 +1,6 @@
 [project]
 name = "odoo-addon-spp_api_v2_oauth"
+
+[build-system]
+requires = ["whool"]
+build-backend = "whool.buildapi"

spp_api_v2_oauth/readme/USAGE.md

@@ -56,7 +56,7 @@ To confirm which algorithm a token uses, decode the JWT header (without verifica
 ```python
 import jwt
 header = jwt.get_unverified_header(token)
-print(header["alg"])  # "RS256" or "HS256"
+# header["alg"] will be "RS256" or "HS256"
 ```
 
 ### Error Responses
@@ -67,4 +67,5 @@ print(header["alg"])  # "RS256" or "HS256"
 | Invalid credentials | 401 | "Invalid client credentials" |
 | Expired token | 401 | "Token expired" |
 | Invalid signature | 401 | "Invalid token" |
+| Unsupported algorithm | 401 | "Unsupported token algorithm: {alg}" |
 | Rate limit exceeded | 429 | "Rate limit exceeded" |

spp_api_v2_oauth/routers/oauth_rs256.py

@@ -70,12 +70,14 @@ async def get_rs256_token(
         )
 
     # Read configurable token lifetime (same config as HS256 endpoint)
-    # nosemgrep: odoo-sudo-without-context
-    token_lifetime_hours = int(
-        env["ir.config_parameter"]
-        .sudo()
-        .get_param("spp_api_v2.token_lifetime_hours", str(DEFAULT_TOKEN_LIFETIME_HOURS))
-    )
+    config_param = env["ir.config_parameter"].sudo()  # nosemgrep: odoo-sudo-without-context
+    try:
+        token_lifetime_hours = int(
+            config_param.get_param("spp_api_v2.token_lifetime_hours", str(DEFAULT_TOKEN_LIFETIME_HOURS))
+        )
+    except (ValueError, TypeError):
+        _logger.warning("Invalid token_lifetime_hours config, using default %s", DEFAULT_TOKEN_LIFETIME_HOURS)
+        token_lifetime_hours = DEFAULT_TOKEN_LIFETIME_HOURS
     expires_in = token_lifetime_hours * 3600
 
     # Generate RS256 JWT token

spp_api_v2_oauth/security/ir.model.access.csv

@@ -529,7 +529,7 @@ <h1>Verify Token Algorithm</h1>
 <pre class="code python literal-block">
 <span class="kn">import</span><span class="w"> </span><span class="nn">jwt</span><span class="w">
 </span><span class="n">header</span> <span class="o">=</span> <span class="n">jwt</span><span class="o">.</span><span class="n">get_unverified_header</span><span class="p">(</span><span class="n">token</span><span class="p">)</span><span class="w">
-</span><span class="nb">print</span><span class="p">(</span><span class="n">header</span><span class="p">[</span><span class="s2">&quot;alg&quot;</span><span class="p">])</span>  <span class="c1"># &quot;RS256&quot; or &quot;HS256&quot;</span>
+</span><span class="c1"># header[&quot;alg&quot;] will be &quot;RS256&quot; or &quot;HS256&quot;</span>
 </pre>
 </div>
 <div class="section" id="error-responses">
@@ -565,6 +565,11 @@ <h1>Error Responses</h1>
 <td>401</td>
 <td>“Invalid token”</td>
 </tr>
+<tr><td>Unsupported algorithm</td>
+<td>401</td>
+<td>“Unsupported token
+algorithm: {alg}”</td>
+</tr>
 <tr><td>Rate limit exceeded</td>
 <td>429</td>
 <td>“Rate limit exceeded”</td>

spp_api_v2_oauth/static/description/index.html

@@ -1,23 +1,16 @@
 # Part of OpenSPP. See LICENSE file for full copyright and licensing details.
 """Common test utilities for spp_api_v2_oauth tests."""
 
-import sys
-from datetime import datetime, timedelta, timezone
+from datetime import UTC, datetime, timedelta
+from types import SimpleNamespace
 
 import jwt
 from cryptography.hazmat.primitives import serialization
 from cryptography.hazmat.primitives.asymmetric import rsa
 
 from odoo.tests.common import TransactionCase
 
-if sys.version_info >= (3, 11):  # noqa: UP036
-    from datetime import UTC
-else:
-    UTC = timezone.utc  # noqa: UP017
-
-# Constants matching spp_api_v2's JWT claims
-JWT_AUDIENCE = "openspp"
-JWT_ISSUER = "openspp-api-v2"
+from ..constants import JWT_AUDIENCE, JWT_ISSUER
 
 # HS256 test secret (same as spp_api_v2 tests)
 HS256_TEST_SECRET = "test-secret-key-for-testing-only-do-not-use-in-production"
@@ -53,8 +46,8 @@ def setUpClass(cls):
         )
 
         # Store RSA keys in spp_oauth config parameters
-        cls.env["ir.config_parameter"].sudo().set_param("spp_oauth.oauth_priv_key", cls.rsa_private_key_pem)
-        cls.env["ir.config_parameter"].sudo().set_param("spp_oauth.oauth_pub_key", cls.rsa_public_key_pem)
+        cls.env["ir.config_parameter"].sudo().set_param("spp_oauth.oauth_private_key", cls.rsa_private_key_pem)
+        cls.env["ir.config_parameter"].sudo().set_param("spp_oauth.oauth_public_key", cls.rsa_public_key_pem)
 
         # Store HS256 secret for spp_api_v2
         cls.env["ir.config_parameter"].sudo().set_param("spp_api_v2.jwt_secret", HS256_TEST_SECRET)
@@ -115,10 +108,4 @@ def generate_hs256_token(self, payload_overrides=None):
     @staticmethod
     def make_credentials(token):
         """Create a mock HTTPAuthorizationCredentials-like object."""
-
-        class _Creds:
-            pass
-
-        creds = _Creds()
-        creds.credentials = token
-        return creds
+        return SimpleNamespace(credentials=token)

spp_api_v2_oauth/tests/common.py

@@ -68,18 +68,54 @@ def test_dependency_override_applied(self):
         from odoo.addons.spp_api_v2.middleware.auth import get_authenticated_client
 
         endpoint = self.env["fastapi.endpoint"].search([("app", "=", "api_v2")], limit=1)
-        if endpoint:
-            overrides = endpoint._get_app_dependencies_overrides()
-            self.assertIn(
-                get_authenticated_client,
-                overrides,
-                "get_authenticated_client should be in dependency overrides",
-            )
-
-            from ..middleware.auth_rs256 import get_authenticated_client_rs256
-
-            self.assertEqual(
-                overrides[get_authenticated_client],
-                get_authenticated_client_rs256,
-                "Override should point to the RS256 bridge function",
-            )
+        if not endpoint:
+            self.skipTest("No api_v2 endpoint configured in test database")
+
+        overrides = endpoint._get_app_dependencies_overrides()
+        self.assertIn(
+            get_authenticated_client,
+            overrides,
+            "get_authenticated_client should be in dependency overrides",
+        )
+
+        from ..middleware.auth_rs256 import get_authenticated_client_rs256
+
+        self.assertEqual(
+            overrides[get_authenticated_client],
+            get_authenticated_client_rs256,
+            "Override should point to the RS256 bridge function",
+        )
+
+    def test_router_registration(self):
+        """Verify the RS256 router is registered for api_v2 endpoints."""
+        endpoint = self.env["fastapi.endpoint"].search([("app", "=", "api_v2")], limit=1)
+        if not endpoint:
+            self.skipTest("No api_v2 endpoint configured in test database")
+
+        routers = endpoint._get_fastapi_routers()
+        # Check that at least one router contains a route to /oauth/token/rs256
+        rs256_routes = [
+            route
+            for router in routers
+            for route in router.routes
+            if hasattr(route, "path") and route.path == "/oauth/token/rs256"
+        ]
+        self.assertTrue(
+            rs256_routes,
+            "RS256 token endpoint should be registered in api_v2 routers",
+        )
+
+    def test_no_override_for_non_api_v2(self):
+        """Bridge overrides should NOT apply to non-api_v2 endpoints."""
+        from odoo.addons.spp_api_v2.middleware.auth import get_authenticated_client
+
+        endpoint = self.env["fastapi.endpoint"].search([("app", "!=", "api_v2")], limit=1)
+        if not endpoint:
+            self.skipTest("No non-api_v2 endpoint configured in test database")
+
+        overrides = endpoint._get_app_dependencies_overrides()
+        self.assertNotIn(
+            get_authenticated_client,
+            overrides,
+            "get_authenticated_client should NOT be overridden for non-api_v2 endpoints",
+        )

spp_api_v2_oauth/tests/test_auth_hs256.py

@@ -89,7 +89,7 @@ def test_rs256_keys_not_configured(self):
         from ..middleware.auth_rs256 import _validate_rs256_token
 
         # Clear RSA public key
-        self.env["ir.config_parameter"].sudo().set_param("spp_oauth.oauth_pub_key", False)
+        self.env["ir.config_parameter"].sudo().set_param("spp_oauth.oauth_public_key", False)
 
         token = self.generate_rs256_token()
 
@@ -160,3 +160,32 @@ def test_header_routing_rs256(self):
 
         client = get_authenticated_client_rs256(creds, self.env)
         self.assertEqual(client.client_id, self.api_client.client_id)
+
+    def test_unsupported_algorithm_rejected(self):
+        """Token with unsupported algorithm (not RS256/HS256) is rejected."""
+        from ..middleware.auth_rs256 import get_authenticated_client_rs256
+
+        # Create a token with HS384 algorithm (unsupported by our bridge)
+        payload = self._build_jwt_payload()
+        secret = "a-secret-key-long-enough-for-hs384-testing-only!!"
+        token = jwt.encode(payload, secret, algorithm="HS384")
+        creds = self.make_credentials(token)
+
+        with self.assertRaises(HTTPException) as ctx:
+            get_authenticated_client_rs256(creds, self.env)
+        self.assertEqual(ctx.exception.status_code, 401)
+        self.assertIn("Unsupported token algorithm", ctx.exception.detail)
+
+    def test_rs256_client_not_found(self):
+        """RS256 token with valid signature but non-existent client_id is rejected."""
+        from ..middleware.auth_rs256 import get_authenticated_client_rs256
+
+        token = self.generate_rs256_token(
+            payload_overrides={"client_id": "non-existent-client-id", "sub": "non-existent-client-id"}
+        )
+        creds = self.make_credentials(token)
+
+        with self.assertRaises(HTTPException) as ctx:
+            get_authenticated_client_rs256(creds, self.env)
+        self.assertEqual(ctx.exception.status_code, 401)
+        self.assertIn("not found", ctx.exception.detail.lower())