security(oauth): restrict OAuth signing-key settings to system admins#265
security(oauth): restrict OAuth signing-key settings to system admins#265gonzalesedwin1123 wants to merge 2 commits into
Conversation
The module granted base.group_user read/write on res.config.settings, which exposes oauth_priv_key (the RS256 JWT signing key) and oauth_pub_key. Any internal user could read the private key via RPC and forge OAuth/JWT tokens, or overwrite the keypair to break/subvert authentication. - Remove the over-broad ir.model.access row. Odoo core already grants res.config.settings to base.group_system (read/write/create), so admins keep full access and can still save settings. - Override default_get to strip the signing-key fields for non-system-admins. default_get reads config_parameter values via sudo() with no model/field access check, so the ACL change alone would not stop key exfiltration over RPC. - Add regression tests asserting a plain internal user is denied model access and cannot read the keys via default_get, while a system admin retains both. Operators should treat the deployed keypair as compromised and rotate it.
There was a problem hiding this comment.
Code Review
This pull request enhances security by restricting access to OAuth signing keys to system administrators. It removes broad read/write access to res.config.settings for standard users and overrides default_get to filter out sensitive keys for non-admins, backed by new integration tests. The feedback suggests ensuring that custom group checks bypass restrictions when running in superuser mode (self.env.su) to prevent unexpected behavior during elevated executions.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 19.0 #265 +/- ##
=======================================
Coverage 75.30% 75.30%
=======================================
Files 1095 1095
Lines 64991 64998 +7
=======================================
+ Hits 48939 48946 +7
Misses 16052 16052
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Per review: env.user stays the original (possibly non-admin) user under sudo() while env.su is True. Bypass the group check when env.su is set so trusted server-side sudo contexts still receive the keys. The RPC attack path is never in superuser mode, so the exposure stays closed.
Problem
spp_oauthgrantedbase.group_user(every internal user) read + write onres.config.settings, which exposesoauth_priv_key— the RS256 JWT signing key usedby
calculate_signature()— andoauth_pub_key. A low-privileged internal user could:verify_and_decode_signature()accepts (full API auth bypass / impersonation), and
Severity: High/Critical — this is a signing credential, not a config toggle.
Why the ACL fix alone is not enough
res.config.settings.default_get()(Odoo corebase/models/res_config.py) readsconfig_parameterfields viair.config_parameter.sudo()and performs no model-ACL orfield-group check. It is an
@api.modelmethod reachable over RPC, soenv['res.config.settings'].default_get(['oauth_priv_key'])leaks the key regardless of theACL. This was reproduced by a failing test before the fix (the plain-user
default_getresult contained the private key). Both changes below are required.
Changes
ir.model.accessrow. Odoo core already grantsres.config.settingstobase.group_system(read/write/create), so system admins keepfull access and can still Save settings. This was the only module in the repo widening this
model to
base.group_user.default_getto stripoauth_priv_key/oauth_pub_keyfor users who are notbase.group_system, closing the sudo RPC path. (Agroups=field attribute would not fixthis — field groups are not enforced in
default_get's config loop.)19.0.2.0.0→19.0.2.0.1.Tests
New
spp_oauth/tests/test_config_settings_acl.py:res.config.settings(AccessError);default_get;default_get.Written test-first (failing → passing).
./spp test spp_oauth→ 14 passed, 0 failed;./spp lintclean.Every internal user has had read access to the signing key historically, so the deployed
RSA keypair must be treated as compromised. After upgrading, operators should rotate
(regenerate) the keypair and invalidate any outstanding tokens. A code fix cannot un-leak an
already-exposed key.
Out of scope (noted for awareness)
data/ir_config_parameter_data.xml(YourPrivateKeyHere/YourPublicKeyHere) are non-secret placeholders.spp_oauth/tools/private_key.pem/public_key.pubare unused (keys are readonly from
ir.config_parameter).password="true"on the settings view fields is display masking only, not an access control.