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

feat: make groups scope optional to support azure with OIDC #9773

Merged
merged 4 commits into from
Jul 31, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions docs/reference/deploy/master-config-reference.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1753,6 +1753,7 @@ used for :ref:`remote user <remote-users>` management.
groups_attribute_name: "XYZ"
display_name_attribute_name: "XYZ"
always_redirect: true
exclude_groups_scope: false

``enabled``
===========
Expand Down Expand Up @@ -1830,6 +1831,13 @@ sign-in page. This redirection persists unless the user explicitly signs out wit
SSO user attempts to use an expired session token, they are directly redirected to the SSO provider
and returned to the requested page after authentication.

``exclude_groups_scope``
========================

Specifies if the groups scope should be excluded for this OIDC Provider. For most OIDC providers
salonig23 marked this conversation as resolved.
Show resolved Hide resolved
like Okta, this should be false (or blank) if you'd like to provision group memberships. But for
salonig23 marked this conversation as resolved.
Show resolved Hide resolved
some providers like Azure, which do not support groups scope, this should be set to true.
salonig23 marked this conversation as resolved.
Show resolved Hide resolved

**********
``saml``
**********
Expand Down
3 changes: 3 additions & 0 deletions helm/charts/determined/templates/master-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,9 @@ stringData:
{{- if .Values.oidc.alwaysRedirect }}
always_redirect: {{ .Values.oidc.alwaysRedirect }}
{{- end }}
{{- if .Values.oidc.excludeGroupsScope }}
exclude_groups_scope: {{ .Values.oidc.excludeGroupsScope }}
{{- end }}
{{- end }}

{{- if .Values.scim }}
Expand Down
1 change: 1 addition & 0 deletions helm/charts/determined/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ useNodePortForMaster: false
# groupsAttributeName:
# displayNameAttributeName:
# alwaysRedirect:
# excludeGroupsScope:

# scim (EE-only) enables System for Cross-domain Identity Management (SCIM) integration, which is
# only available if enterpriseEdition is true. It allows administrators to easily and securely
Expand Down
1 change: 1 addition & 0 deletions master/internal/config/oidc_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ type OIDCConfig struct {
GroupsAttributeName string `json:"groups_attribute_name"`
DisplayNameAttributeName string `json:"display_name_attribute_name"`
AlwaysRedirect bool `json:"always_redirect"`
ExcludeGroupsScope bool `json:"exclude_groups_scope"`
}

// Validate implements the check.Validatable interface.
Expand Down
5 changes: 4 additions & 1 deletion master/internal/plugin/oidc/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,10 @@ func New(db *db.PgDB, config config.OIDCConfig, pachEnabled bool) (*Service, err
return nil, fmt.Errorf("client secret has not been set")
}

scope := []string{oidc.ScopeOpenID, "profile", "email", "groups"}
scope := []string{oidc.ScopeOpenID, "profile", "email"}
if !config.ExcludeGroupsScope {
scope = append(scope, "groups")
}
if pachEnabled {
scope = append(scope, "audience:server:client_id:pachd")
}
Expand Down
Loading