chore: add warning log if misconfigured groups oidc (#7874)

Emyrk · web-flow · commit b2324325fa1a · 2023-06-08T08:51:59.000-05:00
* 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
diff --git 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
@@ -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: <unset>, type: string)
   groupField: ""
   # A map of OIDC group IDs and the group in Coder it should map to. This is useful
diff --git 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.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
 	// attempt at using what the claims provide, but if that fails we will
 	// generate a random username.
diff --git 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'. 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/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.
 
diff --git a/docs/cli/server.md b/docs/cli/server.md
@@ -426,7 +426,7 @@ OIDC claim field to use as the email.
 | Environment | <code>$CODER_OIDC_GROUP_FIELD</code> |
 | YAML        | <code>oidc.groupField</code>         |
 
-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
 

Original file line number	Diff line number	Diff line change
`@@ -675,6 +675,12 @@ func (api API) userOIDC(rw http.ResponseWriter, r http.Request) {`
`675`	`675`	`}`
`676`	`676`	`}`
`677`	`677`
	`678`	`+ // This conditional is purely to warn the user they might have misconfigured their OIDC`
	`679`	`+ // configuration.`
	`680`	`+ if _, groupClaimExists := claims["groups"]; !usingGroups && groupClaimExists {`
	`681`	`+ api.Logger.Debug(ctx, "'groups' claim was returned, but 'oidc-group-field' is not set, check your coder oidc settings.")`
	`682`	`+ }`
	`683`	`+`
`678`	`684`	`// The username is a required property in Coder. We make a best-effort`
`679`	`685`	`// attempt at using what the claims provide, but if that fails we will`
`680`	`686`	`// generate a random username.`
Original file line number	Diff line number	Diff line change
@@ -953,7 +953,7 @@ when required by your organization's security policy.`,
`953`	`953`	`},`
`954`	`954`	`{`
`955`	`955`	`Name: "OIDC Group Field",`
`956`		`- Description: "Change the OIDC default 'groups' claim field. By default, will be 'groups' if present in the oidc scopes argument.",`
	`956`	`+ 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.",`
`957`	`957`	`Flag: "oidc-group-field",`
`958`	`958`	`Env: "CODER_OIDC_GROUP_FIELD",`
`959`	`959`	`// This value is intentionally blank. If this is empty, then OIDC group`