Automatically reauthenticate to Rietveld

Automatically reauthenticate to Rietveld

Added automatic detection of OAuth2 token expiracy. Token is
automatically renewed when necessary.

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

[pgervais, 2014-04-18 17:19:05]

This allows authenticated commit queues to renew their OAuth2 token.

[Sergey Berezin, 2014-04-18 18:33:44]

On 2014/04/18 17:19:05, pgervais wrote:
> This allows authenticated commit queues to renew their OAuth2 token.

This may look like a hack, but after trying to come up with a cleaner solution
in an offline discussion, I would say LGTM to this CL and let it be.

Due to required security measures, we cannot possibly tell a reason for the 302
response, it's exactly the same no matter what happened, and in particular, it
is the same for an expired OAuth token. So we have to make a wild guess once,
and give up if it fails again.

[M-A Ruel, 2014-04-18 21:07:24]

On 2014/04/18 18:33:44, Sergey Berezin wrote:
> On 2014/04/18 17:19:05, pgervais wrote:
> > This allows authenticated commit queues to renew their OAuth2 token.
> 
> This may look like a hack, but after trying to come up with a cleaner solution
> in an offline discussion, I would say LGTM to this CL and let it be.
> 
> Due to required security measures, we cannot possibly tell a reason for the
302
> response, it's exactly the same no matter what happened, and in particular, it
> is the same for an expired OAuth token. So we have to make a wild guess once,
> and give up if it fails again.

The access_token comes with an expiration date, why not use it?

[pgervais, 2014-04-18 21:22:16]

On 2014/04/18 21:07:24, M-A Ruel wrote:
> On 2014/04/18 18:33:44, Sergey Berezin wrote:
> > On 2014/04/18 17:19:05, pgervais wrote:
> > > This allows authenticated commit queues to renew their OAuth2 token.
> > 
> > This may look like a hack, but after trying to come up with a cleaner
solution
> > in an offline discussion, I would say LGTM to this CL and let it be.
> > 
> > Due to required security measures, we cannot possibly tell a reason for the
> 302
> > response, it's exactly the same no matter what happened, and in particular,
it
> > is the same for an expired OAuth token. So we have to make a wild guess
once,
> > and give up if it fails again.
> 
> The access_token comes with an expiration date, why not use it?

Because oauth2client does not set it. And I don't want to fix that library. In
addition, this expiration date may not be perfectly accurate, which means that
we must handle unexpected failure anyway.

[ghost stip (do not use), 2014-04-18 22:02:11]

lgtm

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

https://codereview.chromium.org/243323003/diff/1/rietveld.py#newcode530
rietveld.py:530: # assumption That a 302 is triggered only by an expired OAuth2
token. This
nit: that

https://codereview.chromium.org/243323003/diff/1/rietveld.py#newcode531
rietveld.py:531: # prevents any usage of redirections in pages accessed this
way.
Add a comment that the redirect_caught check means the loop only runs through
twice.

[pgervais, 2014-04-18 22:12:46]

The CQ bit was checked by pgervais@chromium.org

[commit-bot: I haz the power, 2014-04-18 22:13:40]

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

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

Change committed as 264871