Fixing incremental auth in flask_util. #322

theacodes · 2015-10-12T18:30:26Z

Flow is now stored in the session, ensuring that the scopes survive the round trip to the auth server.
The base credentials are now checked in required, solving an issue where it's possible to pass the check with the incremental scopes but without the base scopes.

Fixes #320

theacodes · 2015-10-12T18:35:15Z

Sigh, python 2.6, you are a thorn in my side.

waprin · 2015-10-12T19:10:02Z

LGTM. Tested it on my sample cases and it worked fine.

oauth2client/flask_util.py

@@ -331,6 +333,20 @@ def _make_flow(self, return_url=None, **kwargs):
            redirect_uri=url_for('oauth2.callback', _external=True),
            **kw)

+        flow_key = 'google_oauth2_flow_{0}'.format(csrf_token)
+        session[flow_key] = pickle.dumps(flow)


theacodes · 2015-10-16T17:21:19Z

@dhermes anything holding this back from being merged?

dhermes · 2015-10-18T22:57:01Z

I didn't really review this and don't have throughput to do it. I'll leave it to @nathanielmanistaatgoogle

nathanielmanistaatgoogle · 2015-10-19T17:23:39Z

I'll try to get to this sometime this week. Does it have time-sensitivity to it?

waprin · 2015-10-19T17:29:14Z

I don't think it's especially urgent (though not sure what Jon thinks), but it does fix some critical bugs with the extension and it's a somewhat small patch.

theacodes · 2015-10-19T18:05:06Z

Not overly urgent as I doubt many people are using incremental auth at this point.

I would like to have this, #326, #325, #323, and DictionaryStorage from #319 released together.

All-in-all, lots of new features specific to the usage of web applications. Once we have those in and battle tested a little, we can start promoting them.

oauth2client/flask_util.py

+
+        return flow
+
+    def _get_flow_for_token(self, csrf_token):


theacodes · 2015-10-20T17:51:41Z

Thank you for that thorough review @nathanielmanistaatgoogle. I think I got everything.

oauth2client/flask_util.py

+        flow_pickle = session.get(
+            self._flow_key.format(csrf_token), None)
+
+        if not flow_pickle:


theacodes · 2015-10-21T19:43:04Z

Okay, moved _get_flow_for_token into a private module-level function, and ensured tests all pass. @nathanielmanistaatgoogle it's ready for another look.

oauth2client/flask_util.py

@@ -191,6 +192,21 @@ def requires_calendar():
 __author__ = '[email protected] (Jon Wayne Parrott)'

 DEFAULT_SCOPES = ('email',)
+CREDENTIALS_KEY = 'google_oauth2_credentials'


tests/test_flask_util.py

                session['google_oauth2_csrf_token'] = 'tokenz'

            state = '[{'
-            rv = c.get('/oauth2callback?state=%s&code=codez' % state)
-            self.assertEqual(rv.status_code, httplib.BAD_REQUEST)
+            response = client.get('/oauth2callback?state=%s&code=codez' % state)


nathanielmanistaatgoogle · 2015-10-21T20:19:49Z

I think I'm done with comments now. Fingers crossed.

theacodes · 2015-10-21T21:18:19Z

Fingers crossed here too, thanks for the review. :)

* Flow is now stored in the session, ensuring that the scopes survive the round trip to the auth server. * The base credentials are now checked in `required`, solving an issue where it's possible to pass the check with the incremental scopes but without the base scopes. Fixes googleapis#320

Fixed incremental auth in flask_util.

theacodes · 2015-10-22T00:36:41Z

Thanks!

On Wed, Oct 21, 2015, 5:04 PM Nathaniel Manista [email protected]
wrote:

Merged #322 #322.

—
Reply to this email directly or view it on GitHub
#322 (comment).

googlebot added the cla: yes label Oct 12, 2015

theacodes force-pushed the master branch from 4f4821e to 29f6412 Compare October 12, 2015 18:36

dhermes reviewed Oct 12, 2015
View reviewed changes

nathanielmanistaatgoogle reviewed Oct 19, 2015
View reviewed changes

oauth2client/flask_util.py

return flow

def _get_flow_for_token(self, csrf_token):

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

theacodes force-pushed the master branch from 29f6412 to 73fb3d4 Compare October 20, 2015 17:47

theacodes force-pushed the master branch from 73fb3d4 to a28bc55 Compare October 20, 2015 17:53

nathanielmanistaatgoogle reviewed Oct 21, 2015
View reviewed changes

oauth2client/flask_util.py

flow_pickle = session.get(

self._flow_key.format(csrf_token), None)

if not flow_pickle:

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

theacodes force-pushed the master branch from a28bc55 to 4cf9259 Compare October 21, 2015 19:04

theacodes force-pushed the master branch from 4cf9259 to b5273e6 Compare October 21, 2015 19:42

nathanielmanistaatgoogle reviewed Oct 21, 2015
View reviewed changes

theacodes force-pushed the master branch from b5273e6 to f2d3bff Compare October 21, 2015 20:06

nathanielmanistaatgoogle reviewed Oct 21, 2015
View reviewed changes

theacodes force-pushed the master branch from f2d3bff to b35420a Compare October 21, 2015 21:17

theacodes force-pushed the master branch from b35420a to 07ea7c5 Compare October 21, 2015 21:52

nathanielmanistaatgoogle added a commit that referenced this pull request Oct 22, 2015

Merge pull request #322 from jonparrott/master

311a53f

Fixed incremental auth in flask_util.

nathanielmanistaatgoogle merged commit 311a53f into googleapis:master Oct 22, 2015

dhermes mentioned this pull request Nov 18, 2015

Release v1.5.2 #341

Merged

Fixing incremental auth in flask_util. #322

Fixing incremental auth in flask_util. #322

Uh oh!

Conversation

theacodes commented Oct 12, 2015

Uh oh!

theacodes commented Oct 12, 2015

Uh oh!

waprin commented Oct 12, 2015

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

theacodes commented Oct 16, 2015

Uh oh!

dhermes commented Oct 18, 2015

Uh oh!

nathanielmanistaatgoogle commented Oct 19, 2015

Uh oh!

waprin commented Oct 19, 2015

Uh oh!

theacodes commented Oct 19, 2015

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

theacodes commented Oct 20, 2015

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

theacodes commented Oct 21, 2015

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

nathanielmanistaatgoogle commented Oct 21, 2015

Uh oh!

theacodes commented Oct 21, 2015

Uh oh!

theacodes commented Oct 22, 2015

Uh oh!

Uh oh!