From e553d87c6b576699c88439a11bb7a10d2cdf1937 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 6 Jun 2023 09:23:46 -0500 Subject: [PATCH 1/6] chore: add warning log if misconfigured groups oidc This is not perfect, but if we find a 'groups' claim and it is not configured, put out a warning log to give some information --- coderd/userauth.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/coderd/userauth.go b/coderd/userauth.go index 83c6be248a130..ede081f8b5a59 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -675,6 +675,12 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { } } + // This conditional is purely to warn the user they might have misconfigured their OIDC + // configuration. + if _, groupClaimExists := claims["groups"]; !usingGroups && groupClaimExists { + api.Logger.Warn(ctx, "'groups' claim was returned, but 'oidc-group-field' is not set, check your coder oidc settings.") + } + // The username is a required property in Coder. We make a best-effort // attempt at using what the claims provide, but if that fails we will // generate a random username. From 53c51e62b4afa7c1441410b3ea521607dd89473e Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 6 Jun 2023 09:25:05 -0500 Subject: [PATCH 2/6] Push it to debug log --- coderd/userauth.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/userauth.go b/coderd/userauth.go index ede081f8b5a59..d9b87f1ca7b82 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -678,7 +678,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { // This conditional is purely to warn the user they might have misconfigured their OIDC // configuration. if _, groupClaimExists := claims["groups"]; !usingGroups && groupClaimExists { - api.Logger.Warn(ctx, "'groups' claim was returned, but 'oidc-group-field' is not set, check your coder oidc settings.") + api.Logger.Debug(ctx, "'groups' claim was returned, but 'oidc-group-field' is not set, check your coder oidc settings.") } // The username is a required property in Coder. We make a best-effort From 2d3b335873d4b79a2a8cedc374a2000649902eb5 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 6 Jun 2023 09:32:09 -0500 Subject: [PATCH 3/6] Add docs --- docs/admin/auth.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/docs/admin/auth.md b/docs/admin/auth.md index 234133a04f3ad..27859f9724577 100644 --- a/docs/admin/auth.md +++ b/docs/admin/auth.md @@ -231,7 +231,7 @@ CODER_TLS_CLIENT_KEY_FILE=/path/to/key.pem If your OpenID Connect provider supports group claims, you can configure Coder to synchronize groups in your auth provider to groups within Coder. -To enable group sync, ensure that the `groups` claim is set. If group sync is +To enable group sync, ensure that the `groups` claim is set by adding the correct scope to request. If group sync is enabled, the user's groups will be controlled by the OIDC provider. This means manual group additions/removals will be overwritten on the next login. @@ -242,6 +242,15 @@ CODER_OIDC_SCOPES=openid,profile,email,groups --oidc-scopes openid,profile,email,groups ``` +With the `groups` scope requested, we also need to map the `groups` claim name. Coder recommends using `groups` for the claim name. This step is necessary if your **scope's name** is something other than `groups`. + +```console +# as an environment variable +CODER_OIDC_GROUP_FIELD=groups +# as a flag +--oidc-group-field groups +``` + On login, users will automatically be assigned to groups that have matching names in Coder and removed from groups that the user no longer belongs to. From e81a81bf981145939853cdb08d17cd3c09bb80f6 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 6 Jun 2023 11:07:20 -0500 Subject: [PATCH 4/6] Update cli help msg --- codersdk/deployment.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 12d84b3c94fe8..8d09dee228892 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -953,7 +953,7 @@ when required by your organization's security policy.`, }, { Name: "OIDC Group Field", - Description: "Change the OIDC default 'groups' claim field. By default, will be 'groups' if present in the oidc scopes argument.", + Description: "This field must be set if using the group sync feature and the scope name is not 'groups'", Flag: "oidc-group-field", Env: "CODER_OIDC_GROUP_FIELD", // This value is intentionally blank. If this is empty, then OIDC group From 404370e7bf1de67c4327c71715330ff75bc3c158 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 6 Jun 2023 16:14:44 +0000 Subject: [PATCH 5/6] Make gen --- codersdk/deployment.go | 2 +- docs/cli/server.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 8d09dee228892..ee6d28893b49b 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -953,7 +953,7 @@ when required by your organization's security policy.`, }, { Name: "OIDC Group Field", - Description: "This field must be set if using the group sync feature and the scope name is not 'groups'", + Description: "This field must be set if using the group sync feature and the scope name is not 'groups'. Set to the claim to be used for groups.", Flag: "oidc-group-field", Env: "CODER_OIDC_GROUP_FIELD", // This value is intentionally blank. If this is empty, then OIDC group diff --git a/docs/cli/server.md b/docs/cli/server.md index 3cbcafe9bbc35..bd36bac943ac0 100644 --- a/docs/cli/server.md +++ b/docs/cli/server.md @@ -426,7 +426,7 @@ OIDC claim field to use as the email. | Environment | $CODER_OIDC_GROUP_FIELD | | YAML | oidc.groupField | -Change the OIDC default 'groups' claim field. By default, will be 'groups' if present in the oidc scopes argument. +This field must be set if using the group sync feature and the scope name is not 'groups'. Set to the claim to be used for groups. ### --oidc-group-mapping From 1e45b1fe044a56d056bf1cbd046ca0873f9449a2 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 7 Jun 2023 13:50:04 +0000 Subject: [PATCH 6/6] Update golden --- cli/testdata/coder_server_--help.golden | 4 ++-- cli/testdata/server-config.yaml.golden | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index 6e3de2eabfae5..14f4d6aaa063d 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -296,8 +296,8 @@ can safely ignore these settings. OIDC claim field to use as the email. --oidc-group-field string, $CODER_OIDC_GROUP_FIELD - Change the OIDC default 'groups' claim field. By default, will be - 'groups' if present in the oidc scopes argument. + This field must be set if using the group sync feature and the scope + name is not 'groups'. Set to the claim to be used for groups. --oidc-group-mapping struct[map[string]string], $CODER_OIDC_GROUP_MAPPING (default: {}) A map of OIDC group IDs and the group in Coder it should map to. This diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden index a05cdbfac60c6..33f3d6cfe9eb5 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -238,8 +238,8 @@ oidc: # Ignore the userinfo endpoint and only use the ID token for user information. # (default: false, type: bool) ignoreUserInfo: false - # Change the OIDC default 'groups' claim field. By default, will be 'groups' if - # present in the oidc scopes argument. + # This field must be set if using the group sync feature and the scope name is not + # 'groups'. Set to the claim to be used for groups. # (default: , type: string) groupField: "" # A map of OIDC group IDs and the group in Coder it should map to. This is useful