Skip to content
This repository was archived by the owner on Apr 27, 2023. It is now read-only.

added token refresh functionallity#16

Open
MihaMarkic wants to merge 1 commit into
nbarbettini:masterfrom
MihaMarkic:add-refresh-token
Open

added token refresh functionallity#16
MihaMarkic wants to merge 1 commit into
nbarbettini:masterfrom
MihaMarkic:add-refresh-token

Conversation

@MihaMarkic
Copy link
Copy Markdown

Added functionality that issues a new token with new expiration based on existing valid one.
So the client can get a new token without a new user authentication.

Copy link
Copy Markdown

@raupes raupes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Line 30 of the simple token provider you check if app is null but throw an exception with parametername options. Should be nameof(app).
Regards.

@MihaMarkic
Copy link
Copy Markdown
Author

@raupes Oh, I think that's just bad diff visualization.

@ogix
Copy link
Copy Markdown

ogix commented Mar 25, 2017

@nbarbettini, are you going to merge this PR? :)

@nbarbettini
Copy link
Copy Markdown
Owner

Thanks @ogix, this got buried in my inbox. I want to refactor my code (and this code) to use a more OAuth2-style contract. Then I'll merge. 👍

@gregoryagu
Copy link
Copy Markdown

I would love to see this feature completed.

@gregoryagu
Copy link
Copy Markdown

@MihaMarkic The issue I see with this implementation of refreshing the token is that it doesn't actual use a refresh_token (normally a different token than the access_token). It just uses the existing access_token to create a new one. Since there is no way revoke an existing token it's essentially the same thing as issuing a non-expiring access_token.

A simple fix would be to check to make sure the existing token has not yet expired before refreshing it:

if (originalToken.ValidTo < now)

Then if the access_token was leaked, it would most likely be expired before anyone had a chance to use it.

I would also suggest adding a method called "AllowRefresh()" to be added to the Options (like the current IdentityResolver.) This would call out to user code and return whether or not the Token should be refreshed. For example, the app could check the IP address of the user and see if it had changed, and if so, require that the user re-authenticate.

I know this is supposed to be a "Simple" token provider, but these two items would address two important security issues.

@challe
Copy link
Copy Markdown

challe commented Aug 1, 2017

Any updates on this?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants