Return-Path: X-Original-To: apmail-incubator-allura-commits-archive@minotaur.apache.org Delivered-To: apmail-incubator-allura-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 1B2FD10857 for ; Thu, 31 Oct 2013 18:41:18 +0000 (UTC) Received: (qmail 47570 invoked by uid 500); 31 Oct 2013 18:41:18 -0000 Delivered-To: apmail-incubator-allura-commits-archive@incubator.apache.org Received: (qmail 47508 invoked by uid 500); 31 Oct 2013 18:41:17 -0000 Mailing-List: contact allura-commits-help@incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: allura-dev@incubator.apache.org Delivered-To: mailing list allura-commits@incubator.apache.org Received: (qmail 47255 invoked by uid 99); 31 Oct 2013 18:41:17 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 31 Oct 2013 18:41:17 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id 673F2669F; Thu, 31 Oct 2013 18:41:17 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: johnsca@apache.org To: allura-commits@incubator.apache.org Date: Thu, 31 Oct 2013 18:41:30 -0000 Message-Id: <93869228bf5e473e8c11173597281162@git.apache.org> In-Reply-To: References: X-Mailer: ASF-Git Admin Mailer Subject: [15/27] git commit: [#6692] Fixed issues with OAuth changes from review [#6692] Fixed issues with OAuth changes from review * Removed unused authorized_applications * Fixed Consumer Key value being used for Consumer Secret * Removed unused name constructor param when creating access token * Renamed poorly named "id" param * Removed no longer used views from PreferencesController * Fixed and improved OAuth tests Project: http://git-wip-us.apache.org/repos/asf/incubator-allura/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-allura/commit/b1986d79 Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/b1986d79 Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/b1986d79 Branch: refs/heads/cj/6692 Commit: b1986d793f8451b3ca8d3174b76617d84b57e0b0 Parents: cf0f714 Author: Cory Johns Authored: Fri Oct 4 21:08:02 2013 +0000 Committer: Cory Johns Committed: Thu Oct 31 18:41:01 2013 +0000 ---------------------------------------------------------------------- Allura/allura/controllers/auth.py | 29 +++-------- Allura/allura/controllers/rest.py | 1 - Allura/allura/templates/oauth_applications.html | 16 +++--- Allura/allura/tests/functional/test_auth.py | 54 +++++++++++--------- 4 files changed, 45 insertions(+), 55 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/b1986d79/Allura/allura/controllers/auth.py ---------------------------------------------------------------------- diff --git a/Allura/allura/controllers/auth.py b/Allura/allura/controllers/auth.py index b98f4db..596fda5 100644 --- a/Allura/allura/controllers/auth.py +++ b/Allura/allura/controllers/auth.py @@ -339,7 +339,6 @@ class PreferencesController(BaseController): return dict( menu=menu, api_token=api_token, - authorized_applications=M.OAuthAccessToken.for_user(c.user), ) @h.vardec @@ -403,20 +402,6 @@ class PreferencesController(BaseController): @expose() @require_post() - def revoke_oauth(self, _id=None): - tok = M.OAuthAccessToken.query.get(_id=bson.ObjectId(_id)) - if tok is None: - flash('Invalid app ID', 'error') - redirect('.') - if tok.user_id != c.user._id: - flash('Invalid app ID', 'error') - redirect('.') - tok.delete() - flash('Application access revoked') - redirect(request.referer) - - @expose() - @require_post() @validate(V.NullValidator(), error_handler=index) def change_password(self, **kw): kw = g.theme.password_change_form.to_python(kw, None) @@ -781,8 +766,8 @@ class OAuthController(BaseController): @expose() @require_post() - def deregister(self, id=None): - app = M.OAuthConsumerToken.query.get(_id=bson.ObjectId(id)) + def deregister(self, _id=None): + app = M.OAuthConsumerToken.query.get(_id=bson.ObjectId(_id)) if app is None: flash('Invalid app ID', 'error') redirect('.') @@ -797,7 +782,7 @@ class OAuthController(BaseController): @expose() @require_post() - def generate_access_token(self, id, name=None): + def generate_access_token(self, _id): """ Manually generate an OAuth access token for the given consumer. @@ -805,7 +790,7 @@ class OAuthController(BaseController): less secure (since they rely only on the token, which is transmitted with each request, unlike the access token secret). """ - consumer_token = M.OAuthConsumerToken.query.get(_id=bson.ObjectId(id)) + consumer_token = M.OAuthConsumerToken.query.get(_id=bson.ObjectId(_id)) if consumer_token is None: flash('Invalid app ID', 'error') redirect('.') @@ -817,22 +802,20 @@ class OAuthController(BaseController): user_id=c.user._id, callback='manual', validation_pin=h.nonce(20), - name=name, is_bearer=True, ) access_token = M.OAuthAccessToken( consumer_token_id=consumer_token._id, request_token_id=c.user._id, user_id=request_token.user_id, - name=name, is_bearer=True, ) redirect('.') @expose() @require_post() - def revoke_access_token(self, id): - access_token = M.OAuthAccessToken.query.get(_id=bson.ObjectId(id)) + def revoke_access_token(self, _id): + access_token = M.OAuthAccessToken.query.get(_id=bson.ObjectId(_id)) if access_token is None: flash('Invalid token ID', 'error') redirect('.') http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/b1986d79/Allura/allura/controllers/rest.py ---------------------------------------------------------------------- diff --git a/Allura/allura/controllers/rest.py b/Allura/allura/controllers/rest.py index e97bdac..3969a58 100644 --- a/Allura/allura/controllers/rest.py +++ b/Allura/allura/controllers/rest.py @@ -245,7 +245,6 @@ class OAuthNegotiator(object): consumer_token_id=consumer_token._id, request_token_id=request_token._id, user_id=request_token.user_id, - name=request_token.name, ) return acc_token.to_string() http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/b1986d79/Allura/allura/templates/oauth_applications.html ---------------------------------------------------------------------- diff --git a/Allura/allura/templates/oauth_applications.html b/Allura/allura/templates/oauth_applications.html index 90ba2ea..6bc90a3 100644 --- a/Allura/allura/templates/oauth_applications.html +++ b/Allura/allura/templates/oauth_applications.html @@ -103,8 +103,8 @@
- - + +
@@ -125,16 +125,16 @@ Name:{{consumer_token.name}} Description:{{consumer_token.description_html | safe}} Consumer Key:{{consumer_token.api_key}} - Consumer Secret:{{consumer_token.api_key}} + Consumer Secret:{{consumer_token.secret_key}} -
- - + +
- - + +
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/b1986d79/Allura/allura/tests/functional/test_auth.py ---------------------------------------------------------------------- diff --git a/Allura/allura/tests/functional/test_auth.py b/Allura/allura/tests/functional/test_auth.py index 5ac1168..19fc231 100644 --- a/Allura/allura/tests/functional/test_auth.py +++ b/Allura/allura/tests/functional/test_auth.py @@ -19,8 +19,7 @@ import json from bson import ObjectId import mock -from nose.tools import assert_equal - +from nose.tools import assert_not_equal from datadiff.tools import assert_equal from pylons import tmpl_context as c from allura.tests import TestController @@ -186,27 +185,6 @@ class TestAuth(TestController): r = self.app.get('/auth/preferences/') assert 'No API token generated' in r - def test_oauth(self): - r = self.app.get('/auth/oauth/') - r = self.app.post('/auth/oauth/register', params={'application_name': 'oautstapp', 'application_description': 'Oauth rulez'}).follow() - assert 'oautstapp' in r - r = self.app.post('/auth/oauth/delete').follow() - assert 'Invalid app ID' in r - - def test_revoke_access(self): - self.app.post('/auth/oauth/register', params={'application_name': 'oautstapp', 'application_description': 'Oauth rulez'}).follow() - M.OAuthAccessToken( - consumer_token_id=None, - request_token_id=None, - user_id=M.User.by_username('test-admin')._id) - ThreadLocalORMSession.flush_all() - r = self.app.get('/auth/subscriptions/') - assert '
' in r - r.forms[0].submit() - r = self.app.get('/auth/subscriptions/') - assert '' not in r - assert_equal(M.OAuthAccessToken.for_user(M.User.by_username('test-admin')), []) - @mock.patch('allura.controllers.auth.verify_oid') def test_login_verify_oid_with_provider(self, verify_oid): verify_oid.return_value = dict() @@ -417,6 +395,7 @@ class TestAuth(TestController): assert_equal(r.status_int, 302) assert_equal(r.location, 'http://localhost/auth/?return_to=%2Fp%2Ftest%2Fadmin%2F') + class TestPreferences(TestController): @td.with_user_project('test-admin') @@ -711,3 +690,32 @@ class TestPreferences(TestController): categoryid=str(skill_cat.trove_cat_id))) user = M.User.query.get(username='test-admin') assert len(user.skills) == 0 + + +class TestOAuth(TestController): + + def test_register_deregister_app(self): + # register + r = self.app.get('/auth/oauth/') + r = self.app.post('/auth/oauth/register', params={'application_name': 'oautstapp', 'application_description': 'Oauth rulez'}).follow() + assert 'oautstapp' in r + # deregister + assert_equal(r.forms[0].action, 'deregister') + r.forms[0].submit() + r = self.app.get('/auth/oauth/') + assert 'oautstapp' not in r + + def test_generate_revoke_access_token(self): + # generate + r = self.app.post('/auth/oauth/register', params={'application_name': 'oautstapp', 'application_description': 'Oauth rulez'}).follow() + assert_equal(r.forms[1].action, 'generate_access_token') + r.forms[1].submit() + r = self.app.get('/auth/oauth/') + assert 'Bearer Token:' in r + assert_not_equal(M.OAuthAccessToken.for_user(M.User.by_username('test-admin')), []) + # revoke + assert_equal(r.forms[0].action, 'revoke_access_token') + r.forms[0].submit() + r = self.app.get('/auth/oauth/') + assert_not_equal(r.forms[0].action, 'revoke_access_token') + assert_equal(M.OAuthAccessToken.for_user(M.User.by_username('test-admin')), [])