This is an automated email from the ASF dual-hosted git repository. gcruz pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/allura.git
commit ca980de86cab8fdd08817156dd00505c635db49d Author: Dave Brondsema <[email protected]> AuthorDate: Wed May 29 14:40:07 2024 -0400 [#7272] do not allow http: redirect urls --- Allura/allura/controllers/rest.py | 2 ++ Allura/allura/lib/validators.py | 10 ++++++++++ Allura/allura/lib/widgets/oauth_widgets.py | 18 ++++++++++++------ 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/Allura/allura/controllers/rest.py b/Allura/allura/controllers/rest.py index bfa95a4bf..ec1a3266f 100644 --- a/Allura/allura/controllers/rest.py +++ b/Allura/allura/controllers/rest.py @@ -259,6 +259,8 @@ class Oauth2Validator(oauthlib.oauth2.RequestValidator): return M.OAuth2ClientApp.query.get(client_id=client_id) is not None def validate_redirect_uri(self, client_id, redirect_uri, request, *args, **kwargs): + if redirect_uri.startswith('http:'): + return False client = M.OAuth2ClientApp.query.get(client_id=client_id) return redirect_uri in client.redirect_uris diff --git a/Allura/allura/lib/validators.py b/Allura/allura/lib/validators.py index 2f2776b39..8d6556c24 100644 --- a/Allura/allura/lib/validators.py +++ b/Allura/allura/lib/validators.py @@ -84,6 +84,16 @@ class NonHttpUrl(URL): ''', re.I | re.VERBOSE) +class HttpsUrl(URL): + add_http = False + + def _convert_to_python(self, value, state): + value = super()._convert_to_python(value, state) + if not value.startswith('https://'): + raise fev.Invalid("Must be https://";, value, state) + return value + + class UnicodeString(fev.UnicodeString): """ Override UnicodeString to fix bytes handling. diff --git a/Allura/allura/lib/widgets/oauth_widgets.py b/Allura/allura/lib/widgets/oauth_widgets.py index 5c7105c1f..ba314c3d0 100644 --- a/Allura/allura/lib/widgets/oauth_widgets.py +++ b/Allura/allura/lib/widgets/oauth_widgets.py @@ -63,17 +63,23 @@ class OAuth2ApplicationForm(ForgeForm): # SortableRepeatedField would be nice to use (and ignore sorting) so you can add many dynamically, # but couldn't get it to work easily + + # use HttpsUrl so unencrypted http is prevented and tokens can't be intercepted + # in theory could allow some other protocols (but not http:) so it can work with mobile apps etc redirect_url_1 = ew.TextField( label='Redirect URL(s)', - validator=fev.URL(not_empty=True), - attrs=dict(type='url', style='min-width:25em', required=True), + validator=V.HttpsUrl(not_empty=True), + attrs=dict(type='url', style='min-width:25em', required=True, placeholder='https://...', + pattern='https://.*', title='must start with https://'), ) redirect_url_2 = ew.TextField( - validator=fev.URL(), - attrs=dict(type='url', style='min-width:25em; margin-left: 162px;'), # match grid-4 label width + validator=V.HttpsUrl(), + attrs=dict(type='url', style='min-width:25em; margin-left: 162px;', # match grid-4 label width + pattern='https://.*', title='must start with https://'), ) redirect_url_3 = ew.TextField( - validator=fev.URL(), - attrs=dict(type='url', style='min-width:25em; margin-left: 162px;'), # match grid-4 label width + validator=V.HttpsUrl(), + attrs=dict(type='url', style='min-width:25em; margin-left: 162px;', # match grid-4 label width + pattern='https://.*', title='must start with https://'), )
