Skip to content
This repository has been archived by the owner on Jan 24, 2019. It is now read-only.

Auto refresh auth token #88

Merged
merged 16 commits into from
May 12, 2015
Merged

Auto refresh auth token #88

merged 16 commits into from
May 12, 2015

Conversation

mbland
Copy link
Contributor

@mbland mbland commented May 8, 2015

Hi @jehiah, at the prompting of a non-technical colleague at work (@lsgitter) and after being inspired by an idea from @adelevie, I've taken a stab at a new cookie-refresh flag. The idea is that the cookie's expiration date would automatically get updated whenever the user accesses an authenticated page within some window of time before it expires.

As you'll see, the first three commits are all refactorings; the one after that adds a test for the existing behavior; and the final commit is where the cookie-refresh feature is added.

Good idea? Bad idea?

Mike Bland added 5 commits May 8, 2015 12:41
The intention is to refresh the cookie whenever the user accesses an
authenticated service with less than `cookie-refresh` time to go before the
cookie expires.
@jehiah
Copy link
Member

jehiah commented May 8, 2015

I think this idea is great. For me it's important that this leverage work from #80 which stored the access token so that this increases the timeframe before user access is validated instead of perpetuating it forever.

The example in my mind is that currently i have a 7day cookie and i expect that if i remove someone's permission (ie: invalid access token, or ability to get a new one) i'm expecting the cookie TTL to enforce removing access. The refresh would be great to give control over shortening that timeframe (ie: at cookie-refresh the token is also validated) without forcing every user to re-authenticate on a shorter timeframe (say hourly or daily).

thoughts?

@mbland
Copy link
Contributor Author

mbland commented May 8, 2015

Ah, yeah, I think I got it. Let me see what I can do.

@mbland
Copy link
Contributor Author

mbland commented May 8, 2015

OK, thinking this through here... I think it's definitely doable, but let me get your feedback on what I'm thinking, and I'll let you decide whether I should try to include it in this PR or a follow-up.

So normally the access token is requested when handling the OAuth callback via this line:

access_token, email, err = p.redeemCode(req.Host, req.Form.Get("code"))

Only thing is, as of now, that code from the form isn't getting saved in the cookie. I would presume that if we did save this code in the cookie, it could be used to perform this access token validation via the one redeemCode() call? And if that request succeeds, then we can go ahead and refresh the cookie (without having to update the value, even)?

@jehiah
Copy link
Member

jehiah commented May 8, 2015

Right. We can't re-redeem the code for a new access token, I'm thinking that we'd make a call to an endpoint using the access token to validate. For the google provider this would be this: https://developers.google.com/identity/protocols/OAuth2UserAgent?hl=es#validatetoken

@mbland
Copy link
Contributor Author

mbland commented May 8, 2015

Heh, yeah, I started realizing that, too... Wondering what the state of this is:

https://tools.ietf.org/html/draft-ietf-oauth-introspection-04

In a meeting now, but will look this afternoon...

@mbland
Copy link
Contributor Author

mbland commented May 8, 2015

Seems like /introspect isn't yet implemented by anyone. The /oauth2/v1/tokeninfo endpoint for Google and /api/v1/tokeninfo for MyUSA seems the way to go. @balshor: Do you know of a comparable endpoint for LinkedIn? Try as I might, I can't find any adequate LinkedIn API docs.

@balshor
Copy link

balshor commented May 8, 2015

I'm not sure off the top of my head. I'll ask our OAuth team and see if they have a recommended method of checking whether a token is active or not.

@mbland
Copy link
Contributor Author

mbland commented May 9, 2015

Working on this today, realized I have a question, @jehiah: Per #80, the access token is only stored in the cookie if pass-access-token is set. Should we make a change to always store the access token, and limit pass-access-token to only controlling the header? This will also enforce that cookie-secret be of a size amenable to AES.

@mbland
Copy link
Contributor Author

mbland commented May 9, 2015

@jehiah Think I've got it together now. Using cookie-refresh now implies that the access token is stored in the cookie (requiring an AES-compatible cookie-secret), and the token and the email address are both validated when the refresh threshold is crossed. The default for cookie-refresh is now zero, to avoid surprising anyone who upgrades.

I've successfully tried these changes out on our own running instance, and verified that the cookie was getting auto-refreshed successfully (using our MyUSA backend).

Also, I know you like to squash commits, but in this case, I think having at least most of them separate helps with seeing how the different pieces involved fit together. Ultimately up to you, though, of course.

Found out the hard way that _incoming_ cookies do _not_ have their expiration
timestamps encoded. To perform auto-refresh based on expiration time, we have
to recalculate it from the time encoded in the cookie value.
@jehiah
Copy link
Member

jehiah commented May 12, 2015

Sorry it's taken a few days to get back to this. The changes look great, and i'm super excited to have this functionality.

(I do indeed generally like small break/fix commits squashed to represent one for the final version of the feature 🎉 but these are descriptive commits so it's cool.)

jehiah added a commit that referenced this pull request May 12, 2015
@jehiah jehiah merged commit 9047920 into bitly:master May 12, 2015
@mbland mbland deleted the auto-refresh branch May 12, 2015 04:33
@balshor
Copy link

balshor commented May 12, 2015

It looks like the LinkedIn OAuth2 API does not have an introspection or equivalent endpoint available that will give a time-to-expire for an existing access token. The closest available thing would be to capture the expiration time when the token is created (currently should always be 60 days). We can also validate that a token is currently active by (eg) requesting the user's email address again.

@mbland
Copy link
Contributor Author

mbland commented May 12, 2015

Would it be possible to just set the LinkedIn provider's ValidateUrl to its ProfileUrl and allow OauthProxy.ValidateToken() to make the request? If not, guess I can take a stab at adding a Provider.ValidateToken() method.

@balshor
Copy link

balshor commented May 12, 2015

In principle, checking the return status of a call to the profile url will validate the token. However, the code in this PR that uses the ValidateUrl simply adds the access token as a query parameter. Calling the LinkedIn profile url requires adding the token as an authentication header.

So, it's not quite as simple as dropping in the value of the ProfileUrl into the ValidateUrl field. I think that some sort of Provider.ValidateToken() method would be required anyway to abstract over providers that did not conform to Google's tokeninfo endpoint, including those that chose to support introspection instead.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

3 participants