Fixed reauthentication issue

Fixed reauthentication issue

Renewal of OAuth2 credentials did not work on appengine because it
replies with 302 instead of 401 when authentication fails. Configured
it to attempt credentials renewal on a 302.

Of course this will prevent any 302 from working normally, but it works
with Rietveld since it does not use 302.

BUG=319446
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=264639

[pgervais, 2014-04-17 17:03:43]

This should fix issues we have with an authenticated Rietveld.

[Ryan Tseng, 2014-04-17 17:10:40]

lgtm % comment
Also upstream it to oa2client.REFRESH_STATUS_CODES

https://codereview.chromium.org/238273011/diff/1/rietveld.py
File rietveld.py (right):

https://codereview.chromium.org/238273011/diff/1/rietveld.py#newcode33
rietveld.py:33: # appengine replies with 302 when authentication fails (sigh)
nit: s/appengine/Appengine/g
Also add period.

https://codereview.chromium.org/238273011/diff/1/rietveld.py#newcode34
rietveld.py:34: oa2client.REFRESH_STATUS_CODES = [302, 401]
Use oa2client.REFRESH_STATUS_CODES.append(302) instead, just incase the library
changes this in the future.

[Ryan Tseng, 2014-04-17 17:12:43]

I meant, upstream to
https://code.google.com/p/rietveld/source/browse/rietveld.py?name=chromium.
 You can send a patch to maruel@ and he'll commit it


On Thu, Apr 17, 2014 at 1:10 PM, <hinoka@google.com> wrote:

> lgtm % comment
> Also upstream it to oa2client.REFRESH_STATUS_CODES
>
>
> https://codereview.chromium.org/238273011/diff/1/rietveld.py
> File rietveld.py (right):
>
> https://codereview.chromium.org/238273011/diff/1/rietveld.py#newcode33
> rietveld.py:33: # appengine replies with 302 when authentication fails
> (sigh)
> nit: s/appengine/Appengine/g
> Also add period.
>
> https://codereview.chromium.org/238273011/diff/1/rietveld.py#newcode34
> rietveld.py:34: oa2client.REFRESH_STATUS_CODES = [302, 401]
> Use oa2client.REFRESH_STATUS_CODES.append(302) instead, just incase the
> library changes this in the future.
>
> https://codereview.chromium.org/238273011/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[pgervais, 2014-04-17 17:28:00]

https://codereview.chromium.org/238273011/diff/1/rietveld.py
File rietveld.py (right):

https://codereview.chromium.org/238273011/diff/1/rietveld.py#newcode33
rietveld.py:33: # appengine replies with 302 when authentication fails (sigh)
On 2014/04/17 17:10:40, Ryan T. wrote:
> nit: s/appengine/Appengine/g
> Also add period.

Done.

https://codereview.chromium.org/238273011/diff/1/rietveld.py#newcode34
rietveld.py:34: oa2client.REFRESH_STATUS_CODES = [302, 401]
On 2014/04/17 17:10:40, Ryan T. wrote:
> Use oa2client.REFRESH_STATUS_CODES.append(302) instead, just incase the
library
> changes this in the future.

Done.

[pgervais, 2014-04-17 17:30:49]

On 2014/04/17 17:12:43, Ryan T. wrote:
> I meant, upstream to
> https://code.google.com/p/rietveld/source/browse/rietveld.py?name=chromium.
>  You can send a patch to maruel@ and he'll commit it

I don't see what to upstream. It seems to me that appcfg.py already handles
authentication properly. In addition, this is specific to OAuth2 authentication,
which is not used by the admin tools. 

Or did I miss something?

[pgervais, 2014-04-17 17:30:54]

The CQ bit was checked by pgervais@chromium.org

[commit-bot: I haz the power, 2014-04-17 17:31:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pgervais@chromium.org/238273011/1

[commit-bot: I haz the power, 2014-04-17 17:34:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-17 17:34:04]

Presubmit check for 238273011-1 failed and returned exit status 1.

Running presubmit commit checks ...
Checking out rietveld...
Running save-description-on-failure.sh
Running push-basic.sh
Running upstream.sh
Running submit-from-new-dir.sh
Running abandon.sh
Running submodule-merge-test.sh
Running upload-local-tracking-branch.sh
Running hooks.sh
Running post-dcommit-hook-test.sh
Running upload-stale.sh
Running patch.sh
Running basic.sh

** Presubmit ERRORS **
Missing LGTM from an OWNER for these files:
    depot_tools/rietveld.py

Presubmit checks took 158.9s to calculate.

Was the presubmit check useful? If not, run "git cl presubmit -v"
to figure out which PRESUBMIT.py was run, then run git blame
on the file to figure out who to ask for help.

[pgervais, 2014-04-17 17:35:29]

maruel? can you lgtm that as well?

[iannucci, 2014-04-17 18:27:44]

let's see if my rslgtm is good enough.

[M-A Ruel, 2014-04-17 19:12:47]

lgtm assuming you did want Ryan asked.

https://codereview.chromium.org/238273011/diff/1/rietveld.py
File rietveld.py (right):

https://codereview.chromium.org/238273011/diff/1/rietveld.py#newcode34
rietveld.py:34: oa2client.REFRESH_STATUS_CODES = [302, 401]
On 2014/04/17 17:28:00, pgervais wrote:
> On 2014/04/17 17:10:40, Ryan T. wrote:
> > Use oa2client.REFRESH_STATUS_CODES.append(302) instead, just incase the
> library
> > changes this in the future.
> 
> Done.

You forgot to upload a new version.

[pgervais, 2014-04-17 21:16:15]

The CQ bit was checked by pgervais@chromium.org

[commit-bot: I haz the power, 2014-04-17 21:16:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pgervais@chromium.org/238273011/20001

[commit-bot: I haz the power, 2014-04-17 21:19:37]

Change committed as 264639