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

return user on auth #46

Closed
wants to merge 3 commits into from
Closed

Conversation

butonic
Copy link
Contributor

@butonic butonic commented Nov 21, 2019

born out of cs3org/reva#382 and the previous discussion in cs3org/reva#239 (comment)

The userid is not good enough:

  • Depending on the authentication mechanism we can provide more 'claims' than just the id
  • splitting auth from userlookup doubles the number of ldap lookups (one to check credentials, one to fetch user metadata ... which could be done in the same request)
  • oidc does a user metadata lookup during authentication ... but has no way to forward the resolved claims to the user manager

Even the basic auth provider can return a username and a userid.

The job of the auth provider is to establish the identity of the user and return a stable identifier. But he should be allowed to provide more 'claims'.

I also cleaned up the user type to get rid of the direct subjectand issuer members, which are part of the id. And I added the mail_verified member ....

TBH I would just add all oidc standard claims ... in a subsequent PR. For now, this would be awesome!

@butonic butonic requested a review from labkode as a code owner November 21, 2019 11:54
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Copy link
Member

@labkode labkode left a comment

Choose a reason for hiding this comment

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

@butonic don't make them backwards compatible as no one issuing them yet so we can make them clean from the first version

labkode and others added 2 commits November 25, 2019 07:12
@butonic
Copy link
Contributor Author

butonic commented Nov 25, 2019

ok, that also updated the proto.lock ...

@butonic
Copy link
Contributor Author

butonic commented Dec 2, 2019

merged as part of #48

@butonic butonic closed this Dec 2, 2019
@butonic butonic deleted the return-user-on-auth branch December 2, 2019 10:50
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

Successfully merging this pull request may close these issues.

2 participants