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

Request more than one info url #97

Open
joscha-alisch opened this issue Oct 5, 2021 · 4 comments
Open

Request more than one info url #97

joscha-alisch opened this issue Oct 5, 2021 · 4 comments

Comments

@joscha-alisch
Copy link

After authentication with my provider (GitHub) I would like to fill the token.User with additional info like the organizations/teams they are in to allow RBAC later on. Unfortunately, it seems like the access token is only used to retrieve the infoURL and avatar and is discarded afterwards.

Is there some functionality in this to use the access token to do another request in MapUserFn or ClaimsUpd? If not, do you think it would be possible to add another hook for this? Something like

GetUserInfoFn(client *http.Client) provider.UserData

that would by default do the current implementation

uinfo, err := client.Get(p.infoURL) 

but could be overridden with a custom implementation?

@umputun
Copy link
Member

umputun commented Oct 6, 2021

Generally, I think this is a nice idea to allow custom GetUserInfoFn to be provided optionally. However, oath2 providers also force the scope, like here and that custom user mapper will be limited to the predefined (usually the minimal) scope only. Maybe it makes sense to allow passing the scope as well?

@umputun
Copy link
Member

umputun commented Oct 6, 2021

another thing - I don't think the custom GetUserInfoFn should do the basic part at all. I.e. the basic mapping for ID, name, and picture is kind of a must-have anyway, and making the user's function to repeat the current implementation of the mapUser won't be too friendly.

Probably we may want some ExtraUserFn to be invoked as a part of mapUser and it won't touch the current fields but will be able to fill free-form User.Attributes

@joscha-alisch
Copy link
Author

Thanks @umputun. I think both of your remarks make a lot of sense.

Where do you think would be a good place to add this on the API surface?

  1. Just make it available for AddCustomProvider (this one has configurable scopes already)
  2. Adapting Params in NewGithub(p Params) to allow using the predefined one and instantiating it via AddCustomHandler
  3. Add an "overload" for AddProvider(name, cid, csecret string) like AddProviderWithOptions(name, cid, secret string, scopes []string, extraUserFn)

No 3 would be the most useful from my point of view. But you probably have a better overview and can think of issues with those approaches/other ideas?

@joscha-alisch
Copy link
Author

I went ahead and created a draft PR for this, as the implementation seemed simple enough. Let me know how you feel about it :)

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