Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ability to store the entire access token object #513

Closed
hajekj opened this issue Apr 3, 2016 · 7 comments
Closed

Ability to store the entire access token object #513

hajekj opened this issue Apr 3, 2016 · 7 comments

Comments

@hajekj
Copy link
Contributor

hajekj commented Apr 3, 2016

Hello, do you think it would be possible to add the ability to store and retrieve the entire body containing the access token, refresh token etc. in the AccessToken class? The reason I need it is that when I make a call to Azure AD, it also return an id_token and I need to retrieve it later on.

I understand that it is more OpenID related and this is OAuth2 library, so that's why I am inquiring whether it was possible to store it this way, rather than adjusting it for use of id_token directly.

I can create a PR to implement both scenarios. Is this something that would be accepted?

@shadowhand
Copy link
Member

You could create an extension of AccessToken and use it in the createAccessToken method.

@hajekj
Copy link
Contributor Author

hajekj commented Apr 11, 2016

I think that would complicate the fact that whenever something is changed in the official library regarding the AccessToken class, it would require an update (of course the plugin can be locked to the latest known supported version). I will see how much effort it would take to extend the AccessToken instead of building it in.

@shadowhand
Copy link
Member

Well no... you'd make the modification in your custom provider, so that upgrades would never effect your changes.

@hajekj
Copy link
Contributor Author

hajekj commented Apr 14, 2016

Okay, that makes perfect sense now, thanks for the guidance. I will post here once the modification is done.

@hajekj
Copy link
Contributor Author

hajekj commented May 6, 2016

This is now resolved in my dev branch so I am closing this issue. Thanks for the guidance.

@hajekj hajekj closed this as completed May 6, 2016
@shadowhand
Copy link
Member

Refs #518 not sure why I didn't link it in the commit.

@hajekj
Copy link
Contributor Author

hajekj commented May 6, 2016

I was adding some additional validation for Azure AD tokens so it is fine, however before the merge of TheNetworg/oauth2-azure#16 to my master, I am going to probably make it to use your implementation of the storage, so it remains efficient and doesn't do things twice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants