You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@allura.apache.org by di...@apache.org on 2024/05/20 16:09:41 UTC

(allura) branch master updated (c0a6fa44b -> 8af73c110)

This is an automated email from the ASF dual-hosted git repository.

dill0wn pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/allura.git


    from c0a6fa44b make TestClearOldNotifications pass even when it runs really fast (executing on same timestamp)
     new 614d02dd8 [#7272] convert HTML comments to jinja comments
     new 84d9051e7 [#7272] combine oauth1 and oauth2 settings onto existing page, improve tests
     new 8af73c110 [#7272] small improvements

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 Allura/allura/controllers/auth.py                  | 182 +++++++++++++--------
 Allura/allura/controllers/rest.py                  |  29 ++--
 Allura/allura/lib/widgets/oauth_widgets.py         |  31 +++-
 Allura/allura/model/oauth.py                       |   9 +
 Allura/allura/templates/oauth_applications.html    |  82 +++++++++-
 Allura/allura/templates/widgets/field_cluster.html |   4 +-
 .../templates/widgets/sortable_repeated_field.html |   6 +-
 .../allura/templates/widgets/sortable_table.html   |   4 +-
 Allura/allura/templates/widgets/state_field.html   |   4 +-
 Allura/allura/tests/functional/test_auth.py        | 140 +++++++++-------
 Allura/development.ini                             |   3 +
 pytest.ini                                         |   2 +
 12 files changed, 334 insertions(+), 162 deletions(-)


(allura) 02/03: [#7272] combine oauth1 and oauth2 settings onto existing page, improve tests

Posted by di...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

dill0wn pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/allura.git

commit 84d9051e762557333b36e1fdc36bd94c57e4b020
Author: Dave Brondsema <db...@slashdotmedia.com>
AuthorDate: Wed May 8 16:37:13 2024 -0400

    [#7272] combine oauth1 and oauth2 settings onto existing page, improve tests
---
 Allura/allura/controllers/auth.py               | 182 +++++++++++++++---------
 Allura/allura/controllers/rest.py               |   2 +-
 Allura/allura/lib/widgets/oauth_widgets.py      |  31 +++-
 Allura/allura/model/oauth.py                    |   9 ++
 Allura/allura/templates/oauth_applications.html |  82 +++++++++--
 Allura/allura/tests/functional/test_auth.py     | 140 ++++++++++--------
 Allura/development.ini                          |   3 +
 pytest.ini                                      |   2 +
 8 files changed, 313 insertions(+), 138 deletions(-)

diff --git a/Allura/allura/controllers/auth.py b/Allura/allura/controllers/auth.py
index 06ac5183c..2ab770a69 100644
--- a/Allura/allura/controllers/auth.py
+++ b/Allura/allura/controllers/auth.py
@@ -14,6 +14,7 @@
 #       KIND, either express or implied.  See the License for the
 #       specific language governing permissions and limitations
 #       under the License.
+from __future__ import annotations
 
 import logging
 import os
@@ -75,7 +76,7 @@ class F:
     subscription_form = SubscriptionForm()
     registration_form = forms.RegistrationForm(action='/auth/save_new')
     oauth_application_form = OAuthApplicationForm(action='register')
-    oauth2_application_form = OAuth2ApplicationForm(action='register')
+    oauth2_application_form = OAuth2ApplicationForm(action='register2')
     oauth_revocation_form = OAuthRevocationForm(
         action='/auth/preferences/revoke_oauth')
     change_personal_data_form = forms.PersonalDataForm()
@@ -115,9 +116,6 @@ class AuthController(BaseController):
         self.subscriptions = SubscriptionsController()
         self.oauth = OAuthController()
 
-        if asbool(config.get('auth.oauth2.enabled', False)):
-            self.oauth2 = OAuth2Controller()
-
         if asbool(config.get('auth.allow_user_to_disable_account', False)):
             self.disable = DisableAccountController()
 
@@ -1329,13 +1327,52 @@ class OAuthController(BaseController):
     @expose('jinja:allura:templates/oauth_applications.html')
     def index(self, **kw):
         c.form = F.oauth_application_form
+        c.form2 = F.oauth2_application_form
+
         consumer_tokens = M.OAuthConsumerToken.for_user(c.user)
-        access_tokens = M.OAuthAccessToken.for_user(c.user)
+        access_tokens: list[dict] = []  # simple dict format to work for both OAuth1 and OAuth2 tokens
+        for oauth1_tok in M.OAuthAccessToken.for_user(c.user):
+            access_tokens.append({
+                'type': 1,
+                'app': oauth1_tok.consumer_token,
+                'is_bearer': oauth1_tok.is_bearer,
+                'api_key': oauth1_tok.api_key,
+                'last_access': oauth1_tok.last_access,
+                '_id': oauth1_tok._id,
+            })
+        if asbool(config.get('auth.oauth2.enabled', False)):
+            oauth2_client_apps = M.OAuth2ClientApp.for_user(c.user)
+            for oauth2_tok in M.OAuth2AccessToken.query.find({'user_id': c.user._id}):
+                access_tokens.append({
+                    'type': 2,
+                    'app': M.OAuth2ClientApp.query.get(client_id=oauth2_tok.client_id),
+                    # TODO personal bearer tokens:
+                    'is_bearer': False,
+                    'api_key': None,
+                    'last_access': oauth2_tok.last_access,
+                    '_id': oauth2_tok._id,
+                })
+            # include auth codes too, but only if they're not already listed via an access/refresh token
+            for oauth2_auth in M.OAuth2AuthorizationCode.query.find({'user_id': c.user._id}):
+                client_app = M.OAuth2ClientApp.query.get(client_id=oauth2_auth.client_id)
+                if client_app not in (app['app'] for app in access_tokens):
+                    access_tokens.append({
+                        'type': '2authcode',
+                        'app': client_app,
+                        'is_bearer': False,
+                        'api_key': None,
+                        'last_access': None,
+                        '_id': oauth2_auth._id,
+                    })
+        else:
+            oauth2_client_apps = []
+
         provider = plugin.AuthenticationProvider.get(request)
         return dict(
             menu=provider.account_navigation(),
             consumer_tokens=consumer_tokens,
             access_tokens=access_tokens,
+            oauth2_client_apps=oauth2_client_apps,
         )
 
     @expose()
@@ -1347,6 +1384,24 @@ class OAuthController(BaseController):
         flash('OAuth Application registered')
         redirect('.')
 
+    @expose()
+    @require_post()
+    @validate(F.oauth2_application_form, error_handler=index)
+    def register2(self, application_name=None, application_description=None, **kw):
+        if not asbool(config.get('auth.oauth2.enabled', False)):
+            raise wexc.HTTPNotFound
+
+        redirect_urls = [
+            v for k, v in kw.items()
+            if k.startswith('redirect_url_') and v
+        ]
+        M.OAuth2ClientApp(name=application_name,
+                          description=application_description,
+                          redirect_uris=redirect_urls,
+                          user_id=c.user._id)
+        flash('OAuth2 Client registered')
+        redirect('.')
+
     @expose()
     @require_post()
     def deregister(self, _id=None):
@@ -1363,6 +1418,37 @@ class OAuthController(BaseController):
         flash('Application deleted')
         redirect('.')
 
+    def _check_perm_and_revoke_all2(self, client_id: str):
+        """
+        Revokes the authorization code and access tokens for a given client for all its users
+        """
+        if not asbool(config.get('auth.oauth2.enabled', False)):
+            raise wexc.HTTPNotFound
+
+        client = M.OAuth2ClientApp.query.get(client_id=client_id)
+        if client is None or client.user_id != c.user._id:
+            flash('Invalid client ID', 'error')
+            redirect('.')
+
+        M.OAuth2AuthorizationCode.query.remove({'client_id': client_id})
+        M.OAuth2AccessToken.query.remove({'client_id': client_id})
+        return client
+
+    @expose()
+    @require_post()
+    def deregister2(self, client_id):
+        client = self._check_perm_and_revoke_all2(client_id)
+        client.delete()
+        flash('Client deleted and all access tokens revoked.')
+        redirect('.')
+
+    @expose()
+    @require_post()
+    def revoke_all_access_tokens2(self, client_id):
+        self._check_perm_and_revoke_all2(client_id)
+        flash('Access tokens revoked.')
+        redirect('.')
+
     @expose()
     @require_post()
     def generate_access_token(self, _id):
@@ -1395,83 +1481,51 @@ class OAuthController(BaseController):
         )
         redirect('.')
 
-    @expose()
-    @require_post()
-    def revoke_access_token(self, _id):
-        access_token = M.OAuthAccessToken.query.get(_id=bson.ObjectId(_id))
+    def _check_revoke_perm(self, access_token):
         if access_token is None:
             flash('Invalid token ID', 'error')
             redirect('.')
         if access_token.user_id != c.user._id:
             flash('Invalid token ID', 'error')
             redirect('.')
+
+    @expose()
+    @require_post()
+    def revoke_access_token(self, _id):
+        access_token = M.OAuthAccessToken.query.get(_id=bson.ObjectId(_id))
+        self._check_revoke_perm(access_token)
         access_token.delete()
-        flash('Token revoked')
+        flash('Authorization revoked')
         redirect('.')
 
-
-class OAuth2Controller(BaseController):
-    def _check_security(self):
-        require_authenticated()
-
-    # Revokes the authorization code and access tokens for a given client and user
-    def _revoke_user_tokens(self, client_id, user_id):
-        M.OAuth2AuthorizationCode.query.remove({'client_id': client_id, 'user_id': user_id})
-        M.OAuth2AccessToken.query.remove({'client_id': client_id, 'user_id': user_id})
-
-    # Revokes the authorization code and access tokens for a given client and all its users
-    def _revoke_all(self, client_id):
-        M.OAuth2AuthorizationCode.query.remove({'client_id': client_id})
-        M.OAuth2AccessToken.query.remove({'client_id': client_id})
-
-    @with_trailing_slash
-    @expose('jinja:allura:templates/oauth2_applications.html')
-    def index(self, **kw):
-        c.form = F.oauth2_application_form
-        provider = plugin.AuthenticationProvider.get(request)
-        clients = M.OAuth2ClientApp.for_user(c.user)
-        model = []
-
-        for client in clients:
-            authorization = M.OAuth2AuthorizationCode.query.get(client_id=client.client_id, user_id=c.user._id)
-            token = M.OAuth2AccessToken.query.get(client_id=client.client_id, user_id=c.user._id)
-            model.append(dict(client=client, authorization=authorization, token=token))
-
-        return dict(
-            menu=provider.account_navigation(),
-            model=model
-        )
-
     @expose()
     @require_post()
-    @validate(F.oauth2_application_form, error_handler=index)
-    def register(self, application_name=None, application_description=None, redirect_url=None, **kw):
-        M.OAuth2ClientApp(name=application_name,
-                          description=application_description,
-                          redirect_uris=[redirect_url] if redirect_url else [],
-                          user_id=c.user._id)
-        flash('Oauth2 Client registered')
+    def revoke_access_token2(self, _id):
+        if not asbool(config.get('auth.oauth2.enabled', False)):
+            raise wexc.HTTPNotFound
+
+        tok = M.OAuth2AccessToken.query.get(_id=bson.ObjectId(_id))
+        self._check_revoke_perm(tok)
+        # also any auth codes associated
+        M.OAuth2AuthorizationCode.query.remove({'client_id': tok.client_id, 'user_id': c.user._id})
+        tok.delete()
+        flash('Authorization revoked')
         redirect('.')
 
     @expose()
     @require_post()
-    def do_client_action(self, _id=None, deregister=None, revoke=None):
-        client = M.OAuth2ClientApp.query.get(client_id=_id)
-        if client is None or client.user_id != c.user._id:
-            flash('Invalid client ID', 'error')
-            redirect('.')
-
-        if deregister:
-            self._revoke_all(_id)
-            client.delete()
-            flash('Client deleted and access tokens revoked.')
+    def revoke_access_token2authcode(self, _id):
+        if not asbool(config.get('auth.oauth2.enabled', False)):
+            raise wexc.HTTPNotFound
 
-        if revoke:
-            self._revoke_user_tokens(_id, c.user._id)
-            flash('Access tokens revoked.')
+        auth = M.OAuth2AuthorizationCode.query.get(_id=bson.ObjectId(_id))
+        self._check_revoke_perm(auth)
+        # also any access tokens associated
+        M.OAuth2AccessToken.query.remove({'client_id': auth.client_id, 'user_id': c.user._id})
+        auth.delete()
+        flash('Authorization revoked')
         redirect('.')
 
-
 class DisableAccountController(BaseController):
 
     def _check_security(self):
diff --git a/Allura/allura/controllers/rest.py b/Allura/allura/controllers/rest.py
index f0b8f8448..173585ea7 100644
--- a/Allura/allura/controllers/rest.py
+++ b/Allura/allura/controllers/rest.py
@@ -555,7 +555,7 @@ class Oauth2Negotiator:
 
         if no:
             flash(f'{client.name} NOT AUTHORIZED', 'error')
-            redirect('/auth/oauth2/')
+            redirect('/auth/oauth/')
 
         credentials = json.loads(request.params['credentials'])
         headers, body, status = self.server.create_authorization_response(
diff --git a/Allura/allura/lib/widgets/oauth_widgets.py b/Allura/allura/lib/widgets/oauth_widgets.py
index 201bc35ce..5c7105c1f 100644
--- a/Allura/allura/lib/widgets/oauth_widgets.py
+++ b/Allura/allura/lib/widgets/oauth_widgets.py
@@ -17,6 +17,7 @@
 
 import ew as ew_core
 import ew.jinja2_ew as ew
+from formencode import validators as fev
 
 from allura.lib import validators as V
 
@@ -30,7 +31,11 @@ class OAuthApplicationForm(ForgeForm):
 
     class fields(ew_core.NameList):
         application_name = ew.TextField(label='Application Name',
-                                        validator=V.UniqueOAuthApplicationName())
+                                        validator=V.UniqueOAuthApplicationName(),
+                                        attrs=dict(
+                                            required=True,
+                                        ),
+                                        )
         application_description = AutoResizeTextarea(
             label='Application Description')
 
@@ -49,6 +54,26 @@ class OAuth2ApplicationForm(ForgeForm):
 
     class fields(ew_core.NameList):
         application_name = ew.TextField(label='Application Name',
-                                        validator=V.UniqueOAuthApplicationName())
+                                        validator=V.UnicodeString(not_empty=True),
+                                        attrs=dict(
+                                            required=True,
+                                        ),
+                                        )
         application_description = AutoResizeTextarea(label='Application Description')
-        redirect_url = ew.TextField(label='Redirect URL')
+
+        # SortableRepeatedField would be nice to use (and ignore sorting) so you can add many dynamically,
+        # but couldn't get it to work easily
+        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),
+        )
+        redirect_url_2 = ew.TextField(
+            validator=fev.URL(),
+            attrs=dict(type='url', style='min-width:25em; margin-left: 162px;'),  # match grid-4 label width
+        )
+        redirect_url_3 = ew.TextField(
+            validator=fev.URL(),
+            attrs=dict(type='url', style='min-width:25em; margin-left: 162px;'),  # match grid-4 label width
+        )
+
diff --git a/Allura/allura/model/oauth.py b/Allura/allura/model/oauth.py
index 0e9b752c8..b8b21d8a3 100644
--- a/Allura/allura/model/oauth.py
+++ b/Allura/allura/model/oauth.py
@@ -159,6 +159,9 @@ class OAuth2ClientApp(MappedClass):
         session = main_orm_session
         name = 'oauth2_client_app'
         unique_indexes = [('client_id', 'user_id')]
+        indexes = [
+            ('user_id'),
+        ]
 
     query: 'Query[OAuth2ClientApp]'
 
@@ -192,6 +195,9 @@ class OAuth2AuthorizationCode(MappedClass):
         session = main_orm_session
         name = 'oauth2_authorization_code'
         unique_indexes = [('authorization_code', 'client_id', 'user_id')]
+        indexes = [
+            ('user_id'),
+        ]
 
     query: 'Query[OAuth2AuthorizationCode]'
 
@@ -214,6 +220,9 @@ class OAuth2AccessToken(MappedClass):
         session = main_orm_session
         name = 'oauth2_access_token'
         unique_indexes = [('access_token', 'client_id', 'user_id')]
+        indexes = [
+            ('user_id'),
+        ]
 
     query: 'Query[OAuth2AccessToken]'
 
diff --git a/Allura/allura/templates/oauth_applications.html b/Allura/allura/templates/oauth_applications.html
index c788e7722..9b22cfc68 100644
--- a/Allura/allura/templates/oauth_applications.html
+++ b/Allura/allura/templates/oauth_applications.html
@@ -57,6 +57,14 @@
             }
         });
 
+        $('.revoke_access_tokens').submit(function(e) {
+            var ok = confirm('Revoke all current access tokens?')
+            if(!ok) {
+                e.preventDefault();
+                return false;
+            }
+        });
+
         $('.revoke_access_token').submit(function(e) {
             var ok = confirm('Revoke access?')
             if(!ok) {
@@ -77,13 +85,13 @@
         no longer using an application listed here, you should revoke its
         access.
     </p>
-    {% for access_token in access_tokens %}
+    {% for access_token in access_tokens %}{# both oauth1 & oauth2 #}
     <table class="authorized_app">
         <tr class="name">
-            <th>Name:</th><td>{{access_token.consumer_token.name}}</td>
+            <th>Name:</th><td>{{access_token.app.name}}</td>
         </tr>
         <tr class="description">
-            <th>Description:</th><td>{{access_token.consumer_token.description_html }}</td>
+            <th>Description:</th><td>{{access_token.app.description_html }}</td>
         </tr>
         {% if access_token.is_bearer %}
         <tr class="bearer_token">
@@ -91,11 +99,11 @@
         </tr>
         {% endif %}
          <tr>
-            <th>Last Access:</th><td>{%  if access_token.last_access %} {{ access_token.last_access.strftime('%a %b %d, %Y %I:%M %p UTC')  }} {%  endif %}</td>
+            <th>Last Access:</th><td>{% if access_token.last_access %} {{ access_token.last_access.strftime('%a %b %d, %Y %I:%M %p UTC')  }} {%  endif %}</td>
         </tr>
         <tr class="controls">
             <td colspan="2">
-                <form method="POST" action="revoke_access_token" class="revoke_access_token">
+                <form method="POST" action="revoke_access_token{{ '' if access_token.type == 1 else access_token.type}}" class="revoke_access_token">
                     <input type="hidden" name="_id" value="{{access_token._id}}"/>
                     <input type="submit" value="Revoke"/>
                     {{lib.csrf_token()}}
@@ -103,9 +111,58 @@
             </td>
         </tr>
     </table>
+    {% else %}
+        No applications have been authorized to access your account.
+    {% endfor %}
+
+    {% if oauth2_client_apps %}
+    <h2>My OAuth2 Applications</h2>
+    <p>
+    These are the client applications you have registered.  They can request authorization
+    for a user using the Client ID and Client Secret via OAuth negotiation.
+    See the <a href="{{ config['doc.url.api'] }}">API documentation</a> for more information.
+    </p>
+    {% for client in oauth2_client_apps %}
+    <table>
+        <tr><th>Type:</th><td>OAuth2</td></tr>
+        <tr><th>Name:</th><td>{{client.name}}</td></tr>
+        <tr class="description"><th>Description:</th><td>{{client.description_html }}</td></tr>
+        <tr><th>Client ID:</th><td>{{client.client_id}}</td></tr>
+        <tr><th>Client Secret:</th><td>{{client.client_secret}}</td></tr>
+        <tr><th>Redirect URL:</th><td>
+            {% for uri in client.redirect_uris %}
+                {{ uri }}<br/>
+            {% else %}
+                None!
+            {% endfor %}
+        </td></tr>
+        <tr class="controls">
+            <td colspan="2">
+                <form method="POST" action="deregister2" class="deregister_consumer_token">
+                    <input type="hidden" name="client_id" value="{{client.client_id}}"/>
+                    <input type="submit" value="Delete App"/>
+                    {{lib.csrf_token()}}
+                </form>
+                <form method="POST" action="revoke_all_access_tokens2" class="revoke_access_tokens">
+                    <input type="hidden" name="client_id" value="{{client.client_id}}"/>
+                    <input type="submit" value="Delete all access tokens"/>
+                    {{lib.csrf_token()}}
+                </form>
+                {#
+                <form method="POST" action="generate_access_token2" class="generate_access_token">
+                    <input type="hidden" name="_id" value="{{consumer_token._id}}"/>
+                    <input type="submit" value="Generate Bearer Token"/>
+                    {{lib.csrf_token()}}
+                </form>
+                 #}
+            </td>
+        </tr>
+    </table>
     {% endfor %}
+    {% endif %}
 
-    <h2>My Applications</h2>
+    {% if consumer_tokens %}
+    <h2>My OAuth1 Applications</h2>
     <p>
     These are the applications you have registered.  They can request authorization
     for a user using the Consumer Key and Consumer Secret via OAuth negotiation.
@@ -116,6 +173,7 @@
     </p>
     {% for consumer_token in consumer_tokens %}
     <table class="registered_app">
+        <tr><th>Type:</th><td>OAuth1</td></tr>
         <tr><th>Name:</th><td>{{consumer_token.name}}</td></tr>
         <tr class="description"><th>Description:</th><td>{{consumer_token.description_html }}</td></tr>
         <tr class="consumer_key"><th>Consumer Key:</th><td>{{consumer_token.api_key}}</td></tr>
@@ -124,7 +182,7 @@
             <td colspan="2">
                 <form method="POST" action="deregister" class="deregister_consumer_token">
                     <input type="hidden" name="_id" value="{{consumer_token._id}}"/>
-                    <input type="submit" value="Deregister"/>
+                    <input type="submit" value="Delete App"/>
                     {{lib.csrf_token()}}
                 </form>
                 <form method="POST" action="generate_access_token" class="generate_access_token">
@@ -136,7 +194,15 @@
         </tr>
     </table>
     {% endfor %}
+    {% endif %}
+
+    {% if h.asbool(config.get('auth.oauth2.enabled', False)) %}
+    <div class="grid-24" style="margin-left:0">
+    <h2>Register New OAuth2 Application</h2>
+    {{ c.form2.display() }}
+    </div>
+    {% endif %}
 
-    <h2>Register New Application</h2>
+    <h2>Register New OAuth1 Application</h2>
     {{ c.form.display() }}
 {% endblock %}
diff --git a/Allura/allura/tests/functional/test_auth.py b/Allura/allura/tests/functional/test_auth.py
index a5e69e2d9..214caaee2 100644
--- a/Allura/allura/tests/functional/test_auth.py
+++ b/Allura/allura/tests/functional/test_auth.py
@@ -2070,18 +2070,19 @@ class TestOAuth2(TestController):
     @mock.patch.dict(config, {'auth.oauth2.enabled': True})
     def test_register_deregister_client(self):
         # register
-        r = self.app.get('/auth/oauth2/')
-        r = self.app.post('/auth/oauth2/register',
-                          params={'application_name': 'testoauth2', 'application_description': 'Oauth2 Test',
-                                  'redirect_url': '', '_session_id': self.app.cookies['_session_id'],
-                                  }).follow()
+        r = self.app.get('/auth/oauth/')
+        form = [f for f in r.forms.values() if f.action == 'register2'][0]
+        form['application_name'] = 'testoauth2'
+        form['application_description'] = 'Oauth2 Test'
+        form['redirect_url_1'] = 'https://example.com/'
+        r = form.submit().follow()
 
         assert 'testoauth2' in r
 
         # deregister
-        assert r.forms[0].action == 'do_client_action'
-        r.forms[0].submit('deregister')
-        r = self.app.get('/auth/oauth2/')
+        form = [f for f in r.forms.values() if f.action == 'deregister2'][0]
+        form.submit()
+        r = self.app.get('/auth/oauth/')
         assert 'testoauth2' not in r
 
     @mock.patch.dict(config, {'auth.oauth2.enabled': True})
@@ -2089,6 +2090,7 @@ class TestOAuth2(TestController):
         user = M.User.by_username('test-admin')
         M.OAuth2ClientApp(
             client_id='client_12345',
+            client_secret='98765',
             user_id=user._id,
             name='testoauth2',
             description='test client',
@@ -2105,6 +2107,7 @@ class TestOAuth2(TestController):
         user = M.User.by_username('test-admin')
         M.OAuth2ClientApp(
             client_id='client_12345',
+            client_secret='98765',
             user_id=user._id,
             name='testoauth2',
             description='test client',
@@ -2112,44 +2115,19 @@ class TestOAuth2(TestController):
             redirect_uris=['https://localhost/']
         )
         ThreadLocalODMSession.flush_all()
-        r = self.app.post('/rest/oauth2/do_authorize', params={'no': '1', 'client_id': 'client_12345', 'response_type': 'code'})
-        assert M.OAuth2AuthorizationCode.query.get(client_id='client_12345') is None
-
-    @mock.patch.dict(config, {'auth.oauth2.enabled': True})
-    def test_do_authorize(self):
-        user = M.User.by_username('test-admin')
-        M.OAuth2ClientApp(
-            client_id='client_12345',
-            user_id=user._id,
-            name='testoauth2',
-            description='test client',
-            response_type='code',
-            redirect_uris=['https://localhost/']
-        )
-        ThreadLocalODMSession.flush_all()
-
-        # First navigate to the authorization page for the backend to validate the authorization request
         r = self.app.get('/rest/oauth2/authorize', params={'client_id': 'client_12345', 'response_type': 'code', 'redirect_uri': 'https://localhost/'})
+        r = r.forms[0].submit('no')
+        assert M.OAuth2AuthorizationCode.query.get(client_id='client_12345') is None
 
-        # The submit authorization for the authorization code to be created
-        mock_credentials = dict(client_id='client_12345', redirect_uri='https://localhost/', response_type='code', state=None)
-        r = self.app.post('/rest/oauth2/do_authorize',
-                          params={'yes': '1', 'client_id': 'client_12345', 'response_type': 'code',
-                                  'redirect_uri': 'https://localhost/', 'credentials': json.dumps(mock_credentials)})
-
-        q = M.OAuth2AuthorizationCode.query.get(client_id='client_12345')
-        assert q is not None
-
-        r = self.app.get('/auth/oauth2/')
-        assert 'Authorization Code:' in r
 
     @mock.patch.dict(config, {'auth.oauth2.enabled': True})
-    def test_create_access_token(self):
-        user = M.User.by_username('test-admin')
+    def test_authorize_and_create_access_token(self):
+        # client owned by someone other than the test-admin user that self.app.get/post use
+        app_owner = M.User.by_username('test-user')
         M.OAuth2ClientApp(
             client_id='client_12345',
             client_secret='98765',
-            user_id=user._id,
+            user_id=app_owner._id,
             name='testoauth2',
             description='test client',
             response_type='code',
@@ -2157,20 +2135,19 @@ class TestOAuth2(TestController):
         )
         ThreadLocalODMSession.flush_all()
 
+        r = self.app.get('/auth/oauth/')
+        r.mustcontain(no='testoauth2')
+
         # First navigate to the authorization page for the backend to validate the authorization request
         r = self.app.get('/rest/oauth2/authorize', params={'client_id': 'client_12345', 'response_type': 'code', 'redirect_uri': 'https://localhost/'})
-
         # The submit authorization for the authorization code to be created
-        mock_credentials = dict(client_id='client_12345', redirect_uri='https://localhost/', response_type='code', state=None)
-        r = self.app.post('/rest/oauth2/do_authorize',
-                          params={'yes': '1', 'client_id': 'client_12345', 'response_type': 'code',
-                                  'redirect_uri': 'https://localhost/', 'credentials': json.dumps(mock_credentials)})
+        r.forms[0].submit('yes')
 
         ac = M.OAuth2AuthorizationCode.query.get(client_id='client_12345')
         assert ac is not None
 
-        r = self.app.get('/auth/oauth2/')
-        assert 'Authorization Code:' in r
+        r = self.app.get('/auth/oauth/')
+        r.mustcontain('testoauth2')
 
         # Create the authorization token
         oauth2_params = dict(
@@ -2180,30 +2157,69 @@ class TestOAuth2(TestController):
             grant_type='authorization_code',
             redirect_uri='https://localhost/'
         )
-        r = self.app.post_json('/rest/oauth2/token', oauth2_params)
+        self.app.post_json('/rest/oauth2/token', oauth2_params, extra_environ={'username': '*anonymous'})
         t = M.OAuth2AccessToken.query.get(client_id='client_12345')
         assert t is not None
         assert t.access_token is not None and t.refresh_token is not None
 
-        r = self.app.get('/auth/oauth2/')
-        assert 'Access Token:' in r
-        assert 'Refresh Token:' in r
+        r = self.app.get('/auth/oauth/')
+        r.mustcontain('testoauth2')
+
 
     @mock.patch.dict(config, {'auth.oauth2.enabled': True})
-    def test_revoke_tokens(self):
-        user = M.User.by_username('test-admin')
+    def test_revoke_auth_code(self):
+        # only the auth code is present, and it gets revoked
+        app_owner = M.User.by_username('test-user')
         M.OAuth2ClientApp(
             client_id='client_12345',
+            client_secret='98765',
+            user_id=app_owner._id,
+            name='testoauth2',
+            description='test client',
+            response_type='code',
+            redirect_uris=['https://localhost/']
+        )
+
+        user = M.User.by_username('test-admin')
+        M.OAuth2AuthorizationCode(
+            client_id='client_12345',
+            authorization_code='authcode_12345',
+            expires_at=datetime.utcnow() + timedelta(minutes=10),
             user_id=user._id,
+        )
+
+        ThreadLocalODMSession.flush_all()
+
+        r = self.app.get('/auth/oauth/')
+        r.mustcontain('testoauth2')
+
+        form = [f for f in r.forms.values() if f.action == 'revoke_access_token2authcode'][0]
+        form.submit()
+
+        assert not M.OAuth2AuthorizationCode.query.get(user_id=user._id)
+
+        r = self.app.get('/auth/oauth/')
+        r.mustcontain(no='testoauth2')
+
+
+    @mock.patch.dict(config, {'auth.oauth2.enabled': True})
+    def test_revoke_access_token(self):
+        # both auth code and access token are present, both get revoked
+        app_owner = M.User.by_username('test-user')
+        M.OAuth2ClientApp(
+            client_id='client_12345',
+            client_secret='98765',
+            user_id=app_owner._id,
             name='testoauth2',
             description='test client',
             response_type='code',
             redirect_uris=['https://localhost/']
         )
 
+        user = M.User.by_username('test-admin')
         M.OAuth2AuthorizationCode(
             client_id='client_12345',
-            authotization_code='authcode_12345',
+            authorization_code='authcode_12345',
             expires_at=datetime.utcnow() + timedelta(minutes=10),
             user_id=user._id,
         )
@@ -2218,17 +2234,17 @@ class TestOAuth2(TestController):
 
         ThreadLocalODMSession.flush_all()
 
-        r = self.app.get('/auth/oauth2/')
-        assert 'authorization code' in r
-        assert 'access token' in r
-        assert r.forms[0].action == 'do_client_action'
+        r = self.app.get('/auth/oauth/')
+        r.mustcontain('testoauth2')
+
+        form = [f for f in r.forms.values() if f.action == 'revoke_access_token2'][0]
+        form.submit()
 
-        r.forms[0].submit('revoke')
+        assert not M.OAuth2AuthorizationCode.query.get(user_id=user._id)
+        assert not M.OAuth2AccessToken.query.get(user_id=user._id)
 
-        r = self.app.get('/auth/oauth2/')
-        assert 'testoauth2' in r
-        assert 'Authorization Code:' not in r
-        assert 'Access Token:' not in r
+        r = self.app.get('/auth/oauth/')
+        r.mustcontain(no='testoauth2')
 
 
 class TestOAuthRequestToken(TestController):
diff --git a/Allura/development.ini b/Allura/development.ini
index 424538d3f..25384c647 100644
--- a/Allura/development.ini
+++ b/Allura/development.ini
@@ -497,6 +497,9 @@ disable_entry_points.allura.theme.override = responsive
 ; they'll appear on the import forms
 ;doc.url.importers.GitHub = http://...
 
+; If you have specific API documentation for your site, you can change this
+doc.url.api = https://forge-allura.apache.org/docs/getting_started/administration.html#public-api
+
 ; List of oauth API keys permitted to use special forum import API
 ; (should be converted to newer importer system)
 ;oauth.can_import_forum = api-key-1234, fa832r0fdsafd, f23f80sdf32fd
diff --git a/pytest.ini b/pytest.ini
index 1d26fdcf3..ffc5fc09b 100644
--- a/pytest.ini
+++ b/pytest.ini
@@ -35,6 +35,8 @@ filterwarnings =
     ignore:Deprecated call to `pkg_resources.declare_namespace:DeprecationWarning:pkg_resources
     ignore:pkg_resources is deprecated as an API:DeprecationWarning:tg.util.files
     ignore:pkg_resources is deprecated as an API:DeprecationWarning:formencode
+    # supporting py3.8 still then can revert https://sourceforge.net/p/activitystream/code/ci/c0884668ac0f4445acb423edb25d18b7bd368be7/
+    ignore:SelectableGroups dict interface is deprecated. Use select.:DeprecationWarning:activitystream
 
     # py3.12
     ignore::DeprecationWarning:smtpd


(allura) 03/03: [#7272] small improvements

Posted by di...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

dill0wn pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/allura.git

commit 8af73c1104ce6e3e43d8924e78d8da40a55c248d
Author: Dave Brondsema <db...@slashdotmedia.com>
AuthorDate: Tue May 14 18:13:45 2024 -0400

    [#7272] small improvements
---
 Allura/allura/controllers/rest.py | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/Allura/allura/controllers/rest.py b/Allura/allura/controllers/rest.py
index 173585ea7..0d4fbe60e 100644
--- a/Allura/allura/controllers/rest.py
+++ b/Allura/allura/controllers/rest.py
@@ -306,7 +306,11 @@ class Oauth2Validator(oauthlib.oauth2.RequestValidator):
 
     def validate_bearer_token(self, token: str, scopes: list[str], request: oauthlib.common.Request) -> bool:
         access_token = M.OAuth2AccessToken.query.get(access_token=token)
-        return access_token.expires_at >= datetime.utcnow() if access_token else False
+        if access_token and access_token.expires_at >= datetime.utcnow():
+            request.access_token = access_token
+            return True
+        else:
+            return False
 
     def validate_refresh_token(self, refresh_token: str, client: oauthlib.oauth2.Client, request: oauthlib.common.Request, *args, **kwargs) -> bool:
         return M.OAuth2AccessToken.query.get(refresh_token=refresh_token) is not None
@@ -426,7 +430,7 @@ class OAuthNegotiator:
             http_method=request.method,
             body=request.body,
             headers=request.headers)
-        response.headers = headers
+        response.headers.update(headers)
         response.status_int = status
         return body
 
@@ -481,7 +485,7 @@ class OAuthNegotiator:
             rtok.validation_pin = verifier
             return dict(rtok=rtok)
         else:
-            response.headers = headers
+            response.headers.update(headers)
             response.status_int = status
             return body
 
@@ -492,7 +496,7 @@ class OAuthNegotiator:
             http_method=request.method,
             body=request.body,
             headers=request.headers)
-        response.headers = headers
+        response.headers.update(headers)
         response.status_int = status
         return body
 
@@ -512,14 +516,7 @@ class Oauth2Negotiator:
         if not valid:
             raise exc.HTTPUnauthorized
 
-        bearer_token_prefix = 'Bearer '  # noqa: S105
-        auth_header = req.headers.get('Authorization')
-        if auth_header and auth_header.startswith(bearer_token_prefix):
-            access_token = auth_header[len(bearer_token_prefix):]
-        else:
-            raise exc.HTTPUnauthorized
-
-        token = M.OAuth2AccessToken.query.get(access_token=access_token)
+        token = req.access_token  # set by validate_bearer_token
         token.last_access = datetime.utcnow()
         return token
 
@@ -563,9 +560,7 @@ class Oauth2Negotiator:
         )
 
         response.status_int = status
-        for k, v in headers.items():
-            response.headers[k] = v
-
+        response.headers.update(headers)
         return body
 
     @expose('json:')
@@ -581,6 +576,8 @@ class Oauth2Negotiator:
             request_body = decoded_body
 
         headers, body, status = self.server.create_token_response(uri=request.url, http_method=request.method, body=request_body, headers=request.headers)
+        response.headers.update(headers)
+        response.status_int = status
         return body
 
 def rest_has_access(obj, user, perm):


(allura) 01/03: [#7272] convert HTML comments to jinja comments

Posted by di...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

dill0wn pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/allura.git

commit 614d02dd84daeb690c8161728143b23c13303d65
Author: Dave Brondsema <db...@slashdotmedia.com>
AuthorDate: Tue May 14 12:40:02 2024 -0400

    [#7272] convert HTML comments to jinja comments
---
 Allura/allura/templates/widgets/field_cluster.html           | 4 ++--
 Allura/allura/templates/widgets/sortable_repeated_field.html | 6 +++---
 Allura/allura/templates/widgets/sortable_table.html          | 4 ++--
 Allura/allura/templates/widgets/state_field.html             | 4 ++--
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/Allura/allura/templates/widgets/field_cluster.html b/Allura/allura/templates/widgets/field_cluster.html
index 1ff84e83c..5d0ac8c66 100644
--- a/Allura/allura/templates/widgets/field_cluster.html
+++ b/Allura/allura/templates/widgets/field_cluster.html
@@ -1,4 +1,4 @@
-<!--!
+{#-
        Licensed to the Apache Software Foundation (ASF) under one
        or more contributor license agreements.  See the NOTICE file
        distributed with this work for additional information
@@ -15,7 +15,7 @@
        KIND, either express or implied.  See the License for the
        specific language governing permissions and limitations
        under the License.
--->
+-#}
 {% for field in widget.fields %}
     {{ field.display(**widget.context_for(field)) }}
 {% endfor %}
diff --git a/Allura/allura/templates/widgets/sortable_repeated_field.html b/Allura/allura/templates/widgets/sortable_repeated_field.html
index 2e5e9c6ee..dd7fb83d1 100644
--- a/Allura/allura/templates/widgets/sortable_repeated_field.html
+++ b/Allura/allura/templates/widgets/sortable_repeated_field.html
@@ -1,4 +1,4 @@
-<!--!
+{#-
        Licensed to the Apache Software Foundation (ASF) under one
        or more contributor license agreements.  See the NOTICE file
        distributed with this work for additional information
@@ -15,9 +15,9 @@
        KIND, either express or implied.  See the License for the
        specific language governing permissions and limitations
        under the License.
--->
+-#}
 <div class="{{ container_cls }}" data-name="{{ name }}">
-   <!-- We need "overflow:auto" for sortable.js to properly work in Firefox http://stackoverflow.com/a/3393282/4640690 -->
+   {# We need "overflow:auto" for sortable.js to properly work in Firefox http://stackoverflow.com/a/3393282/4640690  #}
    <div>{% if show_msg %}<p
           class="{{ msg_cls }}"
           data-empty-message="{{ empty_msg }}"
diff --git a/Allura/allura/templates/widgets/sortable_table.html b/Allura/allura/templates/widgets/sortable_table.html
index 74141fc8b..3e0b578dd 100644
--- a/Allura/allura/templates/widgets/sortable_table.html
+++ b/Allura/allura/templates/widgets/sortable_table.html
@@ -1,4 +1,4 @@
-<!--!
+{#-
        Licensed to the Apache Software Foundation (ASF) under one
        or more contributor license agreements.  See the NOTICE file
        distributed with this work for additional information
@@ -15,7 +15,7 @@
        KIND, either express or implied.  See the License for the
        specific language governing permissions and limitations
        under the License.
--->
+-#}
 <div class="{{ container_cls }}" data-name="{{ name }}">
   {{ button.display() }}
   <br/>
diff --git a/Allura/allura/templates/widgets/state_field.html b/Allura/allura/templates/widgets/state_field.html
index 3dca6796a..eeeb4efa7 100644
--- a/Allura/allura/templates/widgets/state_field.html
+++ b/Allura/allura/templates/widgets/state_field.html
@@ -1,4 +1,4 @@
-<!--!
+{#-
        Licensed to the Apache Software Foundation (ASF) under one
        or more contributor license agreements.  See the NOTICE file
        distributed with this work for additional information
@@ -15,7 +15,7 @@
        KIND, either express or implied.  See the License for the
        specific language governing permissions and limitations
        under the License.
--->
+-#}
 <div class="{{ container_cls }}">
     {% set ctx = widget.context_for(selector) %}
     {% if selector.show_label %}