-
Notifications
You must be signed in to change notification settings - Fork 24
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
Feat: enable dynamic profile field mapping via env variables for oidc response #1041
Feat: enable dynamic profile field mapping via env variables for oidc response #1041
Conversation
As not all config settings can be made as environment variables, provide a override mechanism to allow (build-time) configuration adjustments. An example for the graphql access groups provider will be in the next commit. Change-Id: I8dc82ca4f0ac0a1b60fa47eadb147c228a77b841
Instead of requiring an explict service provider for each facility, use extended configurations. Basic enabling/disabling is implemented in the standard config via environment vars, GraphQL needs extendend configuration via localconfiguration. Change-Id: I2ed630bac8f1f66d4f754e5b95d6b232ec63cf3d
Change-Id: I832b6d9e2680aa8423441924de59c4157b50c8e6
#726 raised by @bpedersen2 is merged into this PR to test access-group-provider implementation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Junjiequan I'm not sure that this PR is ready for production. I think we need to do another round of brainstorming and check that the code is correct.
Thoughts?
src/auth/access-group-provider/access-group-from-graphql-api-call.service.ts
Outdated
Show resolved
Hide resolved
src/auth/access-group-provider/access-group-from-multiple-providers.service.ts
Outdated
Show resolved
Hide resolved
src/auth/access-group-provider/access-group-from-payload.service.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure to use SciCat Logger and not directly nestjs one
src/auth/access-group-provider/access-group-from-graphql-api-call.service.ts
Show resolved
Hide resolved
…ion-process-confi
src/auth/access-group-provider/access-group-from-graphql-api-call.service.ts
Show resolved
Hide resolved
…ion-process-confi
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, just docs probably need updates
…ion-process-confi
I confirm that @Junjiequan updated the documentation. Ready to be merged |
…ion-process-confi
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Except for a minor doc glitch looks fine
README.md
Outdated
@@ -110,6 +116,20 @@ Valid environment variables for the .env file. See [.env.example](/.env.example) | |||
- `OIDC_SCOPE` [string] _Optional_ Space separated list of the info returned by the oidc service. Example: "openid profile email" | |||
- `OIDC_SUCCESS_URL` [string] _Optional_ SciCat Frontend auth-callback URL. Required in order to pass user credentials to SciCat Frontend after OIDC login. Example: https://myscicatfrontend/auth-callback | |||
- `OIDC_ACCESS_GROUPS` [string] _Optional_ Functionality is still unclear. | |||
- `OIDC_ACCESS_GROUPS_PROPERTY` [string] _Optional_ Target field to get the access groups value from OIDC response. | |||
- `OIDC_USERINFO_MAPPING_FIELD_USERNAME` [string] _Optional_ comma-separated list. Specifies the fields from the OIDC response to concatenate and use as the user's profile username. For example, setting `OIDC_USERINFO_MAPPING_FIELD_USERNAME="iss, sub"` combines the iss (issuer) and sub (subject) values from the OIDC response, resulting in a username like `myIssuer_myUserName`. This allows for customizable username definitions based on OIDC response attributes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This field is mentioned 2 times here (see line 124)
README.md
Outdated
- `OIDC_USERINFO_MAPPING_FIELD_DISPLAYNAME` [string] _Optional_ Specifies the fields from the OIDC response and use as the user's profile displayname. For example, setting `OIDC_USERINFO_MAPPING_FIELD_DISPLAYNAME="preferred_username"` use displayName value from the OIDC response, resulting in a displayname like `myPreferredName`. This allows for customizable displayname definitions based on OIDC response attributes. | ||
- `OIDC_USERINFO_MAPPING_FIELD_EMAIL` [string] _Optional_ Same as `OIDC_USERINFO_MAPPING_FIELD_DISPLAYNAME`. Defaults to "email" | ||
- `OIDC_USERINFO_MAPPING_FIELD_DISPLAYNAME` [string] _Optional_ Same as `OIDC_USERINFO_MAPPING_FIELD_DISPLAYNAME`. Defaults to "name" | ||
- `OIDC_USERINFO_MAPPING_FIELD_USERNAME` [string] _Optional_ Same as `OIDC_USERINFO_MAPPING_FIELD_DISPLAYNAME`. Defaults to "preferred_username" || "name" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
duplicated, and looking at the code the first one is correct.
Description
This PR introduces configurable userInfo, enabling teams to customize returned values for the frontend. Default behavior remains unchanged if no configuration is provided. Notably,
displayName
inoidc.strategy
now concatenatesgivenName
andfamilyName
.Motivation
Different teams like to show different things in their user profiles. For example, at ESS, we use oidc.sub as the username. This change lets each team pick what works best for them, making sure our system suits everyone's needs.
Fixes:
added userInfoMapping field in the configuration file, which can be customized using the following environment variable name.
Changes:
Tests included/Docs Updated?