You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@allura.apache.org by br...@apache.org on 2026/10/27 18:56:49 UTC
(allura) 01/02: [#8633] OAuth improvements
This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch db/8633
in repository https://gitbox.apache.org/repos/asf/allura.git
commit 7b7920d10ee4a19f41fd7e2003c0bffef4c7840a
Author: Dave Brondsema <da...@brondsema.net>
AuthorDate: Mon Oct 26 15:18:03 2026 -0400
[#8633] OAuth improvements
- OAuth1 request tokens expire after 10 minutes
- stricter handling of OAuth1 authorize/deny steps
- stricter validation of OAuth2 JSON params
- OAuth1 token keys generated with 120 bits of randomness (was 80)
- OAuth2 manually generated bearer tokens use 160 bits of randomness (was 80)
Co-Authored-By: Claude Opus 5.5 <no...@anthropic.com>
---
Allura/allura/controllers/auth.py | 17 ++++++++++++----
Allura/allura/controllers/rest.py | 42 +++++++++++++++++++++++++++------------
Allura/allura/model/oauth.py | 3 ++-
3 files changed, 44 insertions(+), 18 deletions(-)
diff --git a/Allura/allura/controllers/auth.py b/Allura/allura/controllers/auth.py
index 79e5ac50b..30a134dd4 100644
--- a/Allura/allura/controllers/auth.py
+++ b/Allura/allura/controllers/auth.py
@@ -70,6 +70,14 @@
log = logging.getLogger(__name__)
+def oauth2_json_params(raw: str, allow_none: bool = False) -> dict:
+ params = json.loads(raw)
+ allowed = (str, type(None)) if allow_none else str
+ if not isinstance(params, dict) or not all(isinstance(v, allowed) for v in params.values()):
+ raise wexc.HTTPBadRequest('JSON must be an object with string values')
+ return params
+
+
class F:
login_form = LoginForm()
password_change_form = forms.PasswordChangeForm(action='/auth/preferences/change_password')
@@ -1666,7 +1674,7 @@ def generate_access_token(self, _id):
consumer_token_id=consumer_token._id,
user_id=c.user._id,
callback='manual',
- validation_pin=h.nonce(20),
+ validation_pin=h.cryptographic_nonce(15),
is_bearer=True,
)
M.OAuthAccessToken(
@@ -1696,7 +1704,7 @@ def generate_bearer_token(self, client_id):
M.OAuth2AccessToken(
client_id=client_id,
user_id=c.user._id,
- access_token=h.nonce(40),
+ access_token=h.cryptographic_nonce(20),
is_bearer=True,
expires_at=datetime.max
)
@@ -1758,7 +1766,7 @@ def authorize(self, **kwargs):
# We need to decode the request body and convert it to a dict because Turbogears creates it as bytes
# and oauthlib will treat it as x-www-form-urlencoded format.
decoded_body = str(request.body, 'utf-8')
- json_body = json.loads(decoded_body)
+ json_body = oauth2_json_params(decoded_body)
scopes, credentials = self.server.validate_authorization_request(uri=request.url, http_method=request.method, headers=request.headers, body=json_body)
@@ -1783,7 +1791,8 @@ def do_authorize(self, yes=None, no=None):
flash(f'{client.name} NOT AUTHORIZED', 'error')
redirect('/auth/oauth/')
- credentials = json.loads(request.params['credentials'])
+ # values like state may be null
+ credentials = oauth2_json_params(request.params['credentials'], allow_none=True)
headers, body, status = self.server.create_authorization_response(
uri=request.url, http_method=request.method, body=request.body, headers=request.headers, scopes=[], credentials=credentials
)
diff --git a/Allura/allura/controllers/rest.py b/Allura/allura/controllers/rest.py
index cbdbdbed1..dfaf41724 100644
--- a/Allura/allura/controllers/rest.py
+++ b/Allura/allura/controllers/rest.py
@@ -33,9 +33,10 @@
from tg import request, response
import colander
from ming.odm import session
+from bson import ObjectId
from allura import model as M
-from allura.controllers.auth import AuthRestController
+from allura.controllers.auth import AuthRestController, oauth2_json_params
from allura.lib import helpers as h
from allura.lib import security
from allura.lib import plugin
@@ -184,11 +185,16 @@ def save_request_token(self, token: dict, request: oauthlib.common.Request) -> N
session(req_token).flush()
log.info('Saving new request token with key: %s', req_token.api_key[:(len(req_token.api_key) // 2)])
+ def _unexpired_request_token(self, token: str) -> M.OAuthRequestToken | None:
+ # only needed for the few minutes it takes the user to approve the app
+ created_after = ObjectId.from_datetime(datetime.utcnow() - timedelta(minutes=10))
+ return M.OAuthRequestToken.query.get(api_key=token, _id={'$gt': created_after})
+
def verify_request_token(self, token: str, request: oauthlib.common.Request) -> bool:
- return M.OAuthRequestToken.query.get(api_key=token) is not None
+ return self._unexpired_request_token(token) is not None
def validate_request_token(self, client_key: str, token: str, request: oauthlib.common.Request) -> bool:
- req_tok = M.OAuthRequestToken.query.get(api_key=token)
+ req_tok = self._unexpired_request_token(token)
if not req_tok:
return False
return oauthlib.common.safe_string_equals(req_tok.consumer_token.api_key, client_key)
@@ -512,8 +518,6 @@ def authorize(self, **kwargs):
if rtok is None:
log.error('Invalid token %s', oauth_token[:(len(oauth_token) // 2)])
raise exc.HTTPUnauthorized
- # store what user this is, so later use of the token can act as them
- rtok.user_id = c.user._id
return dict(
oauth_token=oauth_token,
consumer=rtok.consumer_token,
@@ -525,17 +529,29 @@ def do_authorize(self, yes=None, no=None, oauth_token=None):
security.require_authenticated()
rtok = M.OAuthRequestToken.query.get(api_key=oauth_token)
+ if rtok is None:
+ raise exc.HTTPUnauthorized
if no:
- rtok.delete()
+ if rtok.user_id == c.user._id:
+ rtok.delete()
flash('%s NOT AUTHORIZED' % rtok.consumer_token.name, 'error')
redirect('/auth/oauth/')
+ if rtok.validation_pin and rtok.user_id != c.user._id:
+ raise exc.HTTPBadRequest
- headers, body, status = self.server.create_authorization_response(
- request.url,
- http_method=request.method,
- body=request.body,
- headers=request.headers,
- realms=[])
+ rtok.user_id = c.user._id
+ try:
+ headers, body, status = self.server.create_authorization_response(
+ request.url,
+ http_method=request.method,
+ body=request.body,
+ headers=request.headers,
+ realms=[])
+ except oauthlib.oauth1.OAuth1Error as oae:
+ log.info(f'oauth1 do_authorize error: {oae!r}')
+ response.headers = {'Content-Type': 'application/x-www-form-urlencoded'}
+ response.status_int = oae.status_code
+ return oae.urlencoded
if status == 200:
verifier = str(parse_qs(body)['oauth_verifier'][0])
@@ -585,7 +601,7 @@ def token(self, **kwargs):
# We try to parse the request body as JSON, if it fails we just use the body as is
# so it's treated as x-www-form-urlencoded
try:
- request_body = json.loads(decoded_body)
+ request_body = oauth2_json_params(decoded_body)
except json.decoder.JSONDecodeError:
request_body = decoded_body
diff --git a/Allura/allura/model/oauth.py b/Allura/allura/model/oauth.py
index 2366ca427..6b56a45cf 100644
--- a/Allura/allura/model/oauth.py
+++ b/Allura/allura/model/oauth.py
@@ -57,7 +57,8 @@ class __mongometa__:
_id = FieldProperty(S.ObjectId)
type = FieldProperty(str)
- api_key = FieldProperty(str, if_missing=lambda: h.nonce(20))
+ # 30 chars, oauthlib's max length for keys
+ api_key = FieldProperty(str, if_missing=lambda: h.cryptographic_nonce(15))
secret_key = FieldProperty(str, if_missing=h.cryptographic_nonce)
last_access = FieldProperty(datetime)