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

Autoprovsioning fixes #8952

Merged
merged 3 commits into from
May 2, 2024
Merged

Autoprovsioning fixes #8952

merged 3 commits into from
May 2, 2024

Conversation

rhafer
Copy link
Contributor

@rhafer rhafer commented Apr 24, 2024

We introduce the new environment variables "PROXY_AUTO_PROVISION_CLAIM_USERNAME", "PROXY_AUTO_PROVISION_CLAIM_EMAIL", and "PROXY_AUTO_PROVISION_CLAIM_DISPLAYNAME" which can be used to configure the OIDC claims that should be used for auto-provisioning user accounts.

The automatic fallback to use the 'email' claim value as the username when the 'preferred_username' claim is not set, has been removed.

Also it is now possible to autoprovision users without an email address

@rhafer rhafer requested a review from micbar April 24, 2024 14:11
@rhafer rhafer self-assigned this Apr 24, 2024
@rhafer rhafer marked this pull request as ready for review April 24, 2024 14:12
@rhafer
Copy link
Contributor Author

rhafer commented Apr 24, 2024

@nicholas-wilson-au With this you should be able to set:

PROXY_USER_OIDC_CLAIM="sub"
PROXY_USER_CS3_CLAIM="username"
PROXY_AUTOPROVISION_ACCOUNTS="true"
PROXY_AUTOPROVISION_CLAIM_USERNAME="sub"

The "only" new thing is PROXY_AUTOPROVISION_CLAIM_USERNAME="sub" which means that the autoprovisioned users that the sub claim set as their username.

You'll likely also want to set:

GRAPH_USERNAME_MATCH="none"

To disable the graph service's internal username validation. When autoprovisioning is enabled we need to trust the username from the external IDP to be valid.

@rhafer rhafer force-pushed the issue/8635 branch 3 times, most recently from 98f3530 to f763004 Compare April 25, 2024 14:14
@nicholas-wilson-au
Copy link

Will this be merged or is it stuck somewhere?

@micbar
Copy link
Contributor

micbar commented Apr 29, 2024

Needs a code review

@mmattel
Copy link
Contributor

mmattel commented Apr 30, 2024

Please add some description in readme.md

…r auto-provisioning user accounts

When auto-provisioning user accounts we used a fixed mapping for claims
for the userinfo response to user attributes. This change introduces
configuration options to defined which claims should be user for the
username, display name and email address of the auto-provisioned
accounts.

This also removes the automatic fallback to use the 'mail' claim as the
username when the 'preferred_username' claim does not exist.

Fixes: owncloud#8635
The mail address is not a required attrbute for our users. So we can auto-provision users without it.

Fixes: owncloud#6909
Copy link

sonarcloud bot commented Apr 30, 2024

Copy link
Member

@dragonchaser dragonchaser left a comment

Choose a reason for hiding this comment

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

Works, I am a bit unhappy about the behavior when one of the variables is misconfigured. I get an error in the logs telling me "claim 'xxx_xxxx' does not exist" but for the user it just redirects me to the "Login again" page. Some verbosity there would be nice (e.g. error message: "Login with the OIDC provider failed, please contact your administrator".

@micbar
Copy link
Contributor

micbar commented May 2, 2024

Works, I am a bit unhappy about the behavior when one of the variables is misconfigured. I get an error in the logs telling me "claim 'xxx_xxxx' does not exist" but for the user it just redirects me to the "Login again" page. Some verbosity there would be nice (e.g. error message: "Login with the OIDC provider failed, please contact your administrator".

That is no new behavior. Happens also when you misconfigure the OIDC roles from claim config.

Merging.

@micbar
Copy link
Contributor

micbar commented May 2, 2024

@kulmann @JammingBen We should create a follow up for web.

Given our user endpoint would return something more reasonable, (not 500 as of now), we should provide something for the end user like "contact your administrator".

@micbar micbar merged commit 6356be8 into owncloud:master May 2, 2024
4 checks passed
ownclouders pushed a commit that referenced this pull request May 2, 2024
@micbar micbar mentioned this pull request Jun 19, 2024
24 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

5 participants