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

Allow setting additional data for OpenID model #30

Closed
sammaphey opened this issue Nov 18, 2022 · 4 comments · Fixed by #148
Closed

Allow setting additional data for OpenID model #30

sammaphey opened this issue Nov 18, 2022 · 4 comments · Fixed by #148
Assignees
Labels
enhancement New feature or request

Comments

@sammaphey
Copy link

This class should allow additional fields instead of just the base fields. There are more useful pieces of information I need as a response from verify_and_process such as: User citizenship, phone number, etc.

@tomasvotava
Copy link
Owner

Wouldn't it be better if your provider returned a subclass of OpenID? In #31 there is no IDE autocomplete for the fields you add as extra. If you subclass OpenID, add your custom fields, and then return your custom OpenID from verify_and_process, you get what you need and still keep all fields transparent.

@sammaphey
Copy link
Author

Yeah, this is what I ended up doing, I guess it seems odd that I must subclass OpenID when all I really want is the data that is returned from my SSO server. It seems like something that I shouldn't have to enforce a BaseModel for since as far as I can tell here the only advantage I get is some dot notation and typeahead for static analysis. But your answer does make sense so I am happy to have the issue closed.

@tomasvotava
Copy link
Owner

Hi again, I've been thinking about this for quite some time now and I understand getting a Pydantic model as a response may not always be what you want. And so I will introduce a way to actually get the bare json response from the server instead. Thank you for this thought-provoking issue.

@tomasvotava tomasvotava self-assigned this Jan 18, 2023
@tomasvotava tomasvotava added the enhancement New feature or request label Jan 18, 2023
@ichernev
Copy link

@sammaphey you can just subclass your SSO and override openid_from_response to return something potentially different. If it's a subclass of OpenID even the type info would be correct :)

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

Successfully merging a pull request may close this issue.

3 participants