Datahub Frontend: OIDC username claim only changable for google
gschuurman opened this issue · 5 comments
Describe the bug
oidcAuthentication.user_name_claim is only evaluated with oidcAuthentication.provider: "google"
This causes issues for azure ad implementations
To Reproduce
Steps to reproduce the behavior:
Deploy datahub with oidcAuthentication.provider: "azure" and oidcAuthentication.user_name_claim: "preferred_username"
Observe in the logging that email is still being used as email claim
Expected behavior
Expected that preferred_username was used as an email
Screenshots
Additional context
Add any other context about the problem here.
Issue lies with the assignment of the AUTH_OIDC_USER_NAME_CLAIM, this is only set with google
{{- if eq .provider "google" }}
- name: AUTH_OIDC_DISCOVERY_URI
value: https://accounts.google.com/.well-known/openid-configuration
- name: AUTH_OIDC_SCOPE
value: {{ .scope | default "openid profile email" }}
- name: AUTH_OIDC_USER_NAME_CLAIM
value: {{ .user_name_claim | default "email" }}
- name: AUTH_OIDC_USER_NAME_CLAIM_REGEX
value: {{ .user_name_claim_regex | default "([^@]+)" }}Solved with PR #415
@gschuurman Note: this changed the default. Previously (if unset) the default was (.*), see here. For running systems, this can have severe impact, due to duplicated users and users suddenly missing previously assigned roles.
@Gerrit-K I see, Any idea how we can fix this, while still maintaining the default for google that was always there?
There are a few ways, but all of them are a bit verbose in a way:
Index: charts/datahub/subcharts/datahub-frontend/templates/deployment.yaml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/charts/datahub/subcharts/datahub-frontend/templates/deployment.yaml b/charts/datahub/subcharts/datahub-frontend/templates/deployment.yaml
--- a/charts/datahub/subcharts/datahub-frontend/templates/deployment.yaml (revision Staged)
+++ b/charts/datahub/subcharts/datahub-frontend/templates/deployment.yaml (date 1704789122250)
@@ -223,7 +223,11 @@
- name: AUTH_OIDC_USER_NAME_CLAIM
value: {{ .user_name_claim | default "email" }}
- name: AUTH_OIDC_USER_NAME_CLAIM_REGEX
+ {{- if eq .provider "google" }}
value: {{ .user_name_claim_regex | default "([^@]+)" }}
+ {{- else }}
+ value: {{ .user_name_claim_regex | default "(.*)" }}
+ {{- end }}
{{- if eq .provider "google" }}
- name: AUTH_OIDC_DISCOVERY_URI
value: https://accounts.google.com/.well-known/openid-configurationor
Index: charts/datahub/subcharts/datahub-frontend/templates/deployment.yaml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/charts/datahub/subcharts/datahub-frontend/templates/deployment.yaml b/charts/datahub/subcharts/datahub-frontend/templates/deployment.yaml
--- a/charts/datahub/subcharts/datahub-frontend/templates/deployment.yaml (revision Staged)
+++ b/charts/datahub/subcharts/datahub-frontend/templates/deployment.yaml (date 1704900548900)
@@ -222,8 +222,12 @@
value: https://{{ (first $.Values.ingress.hosts).host }}
- name: AUTH_OIDC_USER_NAME_CLAIM
value: {{ .user_name_claim | default "email" }}
+ {{- $claimRegexDefault := "(.*)" }}
+ {{- if eq .provider "google" }}
+ {{ $claimRegexDefault = "([^@]+)" }}
+ {{- end }}
- name: AUTH_OIDC_USER_NAME_CLAIM_REGEX
- value: {{ .user_name_claim_regex | default "([^@]+)" }}
+ value: {{ .user_name_claim_regex | default $claimRegexDefault }}
{{- if eq .provider "google" }}
- name: AUTH_OIDC_DISCOVERY_URI
value: https://accounts.google.com/.well-known/openid-configurationor
Index: charts/datahub/subcharts/datahub-frontend/templates/deployment.yaml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/charts/datahub/subcharts/datahub-frontend/templates/deployment.yaml b/charts/datahub/subcharts/datahub-frontend/templates/deployment.yaml
--- a/charts/datahub/subcharts/datahub-frontend/templates/deployment.yaml (revision Staged)
+++ b/charts/datahub/subcharts/datahub-frontend/templates/deployment.yaml (date 1704900730749)
@@ -222,23 +222,27 @@
value: https://{{ (first $.Values.ingress.hosts).host }}
- name: AUTH_OIDC_USER_NAME_CLAIM
value: {{ .user_name_claim | default "email" }}
- - name: AUTH_OIDC_USER_NAME_CLAIM_REGEX
- value: {{ .user_name_claim_regex | default "([^@]+)" }}
{{- if eq .provider "google" }}
- name: AUTH_OIDC_DISCOVERY_URI
value: https://accounts.google.com/.well-known/openid-configuration
- name: AUTH_OIDC_SCOPE
value: {{ .scope | default "openid profile email" }}
+ - name: AUTH_OIDC_USER_NAME_CLAIM_REGEX
+ value: {{ .user_name_claim_regex | default "([^@]+)" }}
{{- else if eq .provider "okta" }}
- name: AUTH_OIDC_DISCOVERY_URI
value: https://{{ .oktaDomain }}/.well-known/openid-configuration
- name: AUTH_OIDC_SCOPE
value: {{ .scope | default "openid profile email groups" }}
+ - name: AUTH_OIDC_USER_NAME_CLAIM_REGEX
+ value: {{ .user_name_claim_regex | default "(.*)" }}
{{- else if eq .provider "azure" }}
- name: AUTH_OIDC_DISCOVERY_URI
value: https://login.microsoftonline.com/{{ .azureTenantId }}/v2.0/.well-known/openid-configuration
- name: AUTH_OIDC_SCOPE
value: {{ .scope | default "openid profile email" }}
+ - name: AUTH_OIDC_USER_NAME_CLAIM_REGEX
+ value: {{ .user_name_claim_regex | default "(.*)" }}
{{- else }}
{{- fail (printf "unsupported .oidcAuthentication.provider value '%s'" .provider) }}
{{- end }}It depends a bit on whether you want to avoid to duplicate the provider check or the variable definition. There are probably alsomore/better alternatives. I'd say out of the three, the last one looks like it best aligns with what already has been done for the URI and scope variables.
Any chance this can be fixed in the charts? We recently upgraded and suddenly our usernames changed, so people have lost their access rights etc ... took some time to trace it here