Apr 5, 2023 · Mar 31, 2023 · Mar 31, 2023 · Mar 31, 2023 · Mar 31, 2023 · Mar 31, 2023
diff --git a/cli/server.go b/cli/server.go
 UsernameField:       cfg.OIDC.UsernameField.String(),
 EmailField:          cfg.OIDC.EmailField.String(),
 AuthURLParams:       cfg.OIDC.AuthURLParams.Value,
 IgnoreUserInfo:      cfg.OIDC.IgnoreUserInfo.Value(),
 GroupField:          cfg.OIDC.GroupField.String(),
 GroupMapping:        cfg.OIDC.GroupMapping.Value,
 SignInText:          cfg.OIDC.SignInText.String(),
diff --git a/cli/server_test.go b/cli/server_test.go
 require.Equal(t, "preferred_username", deploymentConfig.Values.OIDC.UsernameField.Value())
 require.Equal(t, "email", deploymentConfig.Values.OIDC.EmailField.Value())
 require.Equal(t, map[string]string{"access_type": "offline"}, deploymentConfig.Values.OIDC.AuthURLParams.Value)
 require.False(t, deploymentConfig.Values.OIDC.IgnoreUserInfo.Value())
 require.Empty(t, deploymentConfig.Values.OIDC.GroupField.Value())
 require.Empty(t, deploymentConfig.Values.OIDC.GroupMapping.Value)
 require.Equal(t, "OpenID Connect", deploymentConfig.Values.OIDC.SignInText.Value())
 "--oidc-username-field", "not_preferred_username",
 "--oidc-email-field", "not_email",
 "--oidc-auth-url-params", `{"prompt":"consent"}`,
 "--oidc-ignore-userinfo",
 "--oidc-group-field", "serious_business_unit",
 "--oidc-group-mapping", `{"serious_business_unit": "serious_business_unit"}`,
 "--oidc-sign-in-text", "Sign In With Coder",
 require.True(t, deploymentConfig.Values.OIDC.IgnoreEmailVerified.Value())
 require.Equal(t, "not_preferred_username", deploymentConfig.Values.OIDC.UsernameField.Value())
 require.Equal(t, "not_email", deploymentConfig.Values.OIDC.EmailField.Value())
 require.True(t, deploymentConfig.Values.OIDC.IgnoreUserInfo.Value())
 require.Equal(t, map[string]string{"prompt": "consent"}, deploymentConfig.Values.OIDC.AuthURLParams.Value)
 require.Equal(t, "serious_business_unit", deploymentConfig.Values.OIDC.GroupField.Value())
 require.Equal(t, map[string]string{"serious_business_unit": "serious_business_unit"}, deploymentConfig.Values.OIDC.GroupMapping.Value)
diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden
          Change the OIDC default 'groups' claim field. By default, will be
          'groups' if present in the oidc scopes argument.

      --oidc-group-mapping struct[map[string]string], $OIDC_GROUP_MAPPING (default: {})
      --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
          is useful for when OIDC providers only return group IDs.

      --oidc-ignore-email-verified bool, $CODER_OIDC_IGNORE_EMAIL_VERIFIED
          Ignore the email_verified claim from the upstream provider.

      --oidc-ignore-userinfo bool, $CODER_OIDC_IGNORE_USERINFO (default: false)
          Ignore the userinfo endpoint and only use the ID token for user
          information.

      --oidc-issuer-url string, $CODER_OIDC_ISSUER_URL
          Issuer URL to use for Login with OIDC.

diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go
diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json
diff --git a/coderd/userauth.go b/coderd/userauth.go
 "fmt"
 "net/http"
 "net/mail"
 "sort"
 "strconv"
 "strings"

 // AuthURLParams are additional parameters to be passed to the OIDC provider
 // when requesting an access token.
 AuthURLParams map[string]string
 // IgnoreUserInfo causes Coder to only use claims from the ID token to
 // process OIDC logins. This is useful if the OIDC provider does not
 // support the userinfo endpoint, or if the userinfo endpoint causes
 // undesirable behavior.
 IgnoreUserInfo bool
 // GroupField selects the claim field to be used as the created user's
 // groups. If the group field is the empty string, then no group updates
 // will ever come from the OIDC provider.
 return
 }

 api.Logger.Debug(ctx, "got oidc claims",
 slog.F("source", "id_token"),
 slog.F("claim_fields", claimFields(claims)),
 slog.F("blank", blankFields(claims)),
 )

 // Not all claims are necessarily embedded in the `id_token`.
 // In GitLab, the username is left empty and must be fetched in UserInfo.
 //
 // The OIDC specification says claims can be in either place, so we fetch
 // user info and merge the two claim sets to be sure we have all of
 // the correct data.
 userInfo, err := api.OIDCConfig.Provider.UserInfo(ctx, oauth2.StaticTokenSource(state.Token))
 if err == nil {
 userInfoClaims := map[string]interface{}{}
 err = userInfo.Claims(&userInfoClaims)
 if err != nil {
 // user info if required and merge the two claim sets to be sure we have
 // all of the correct data.
 //
 // Some providers (e.g. ADFS) do not support custom OIDC claims in the
 // UserInfo endpoint, so we allow users to disable it and only rely on the
 // ID token.
 if !api.OIDCConfig.IgnoreUserInfo {
 userInfo, err := api.OIDCConfig.Provider.UserInfo(ctx, oauth2.StaticTokenSource(state.Token))
 if err == nil {
 userInfoClaims := map[string]interface{}{}
 err = userInfo.Claims(&userInfoClaims)
 if err != nil {
 httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
 Message: "Failed to unmarshal user info claims.",
 Detail:  err.Error(),
 })
 return
 }
 api.Logger.Debug(ctx, "got oidc claims",
 slog.F("source", "userinfo"),
 slog.F("claim_fields", claimFields(userInfoClaims)),
 slog.F("blank", blankFields(userInfoClaims)),
 )

 // Merge the claims from the ID token and the UserInfo endpoint.
 // Information from UserInfo takes precedence.
 claims = mergeClaims(claims, userInfoClaims)

 // Log all of the field names after merging.
 api.Logger.Debug(ctx, "got oidc claims",
 slog.F("source", "merged"),
 slog.F("claim_fields", claimFields(claims)),
 slog.F("blank", blankFields(claims)),
 )
 } else if !strings.Contains(err.Error(), "user info endpoint is not supported by this provider") {
 httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
 Message: "Failed tounmarshal userinfo claims.",
 Detail:  err.Error(),
 Message: "Failed toobtain userinformation claims.",
 Detail:"The attempt to fetch claims via the UserInfo endpoint failed: " +err.Error(),
 })
 return
 } else {
 // The OIDC provider does not support the UserInfo endpoint.
 // This is not an error, but we should log it as it may mean
 // that some claims are missing.
 api.Logger.Warn(ctx, "OIDC provider does not support the user info endpoint, ensure that all required claims are present in the id_token")
 }
 for k, v := range userInfoClaims {
 claims[k] = v
 }
 } else if !strings.Contains(err.Error(), "user info endpoint is not supported by this provider") {
 httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
 Message: "Failed to obtain user information claims.",
 Detail:  "The OIDC provider returned no claims as part of the `id_token`. The attempt to fetch claims via the UserInfo endpoint failed: " + err.Error(),
 })
 return
 }

 // Log all of the field names returned in the ID token claims, and the
 // userinfo returned from the provider.
 {
 fields := make([]string, 0, len(claims))
 for f := range claims {
 fields = append(fields, f)
 }

 api.Logger.Debug(ctx, "got oidc claims",
 slog.F("user_info", userInfo),
 slog.F("claim_fields", fields),
 )
 }

 usernameRaw, ok := claims[api.OIDCConfig.UsernameField]
 group, ok := groupInterface.(string)
 if !ok {
 httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
 Message: fmt.Sprintf("Invalid group type. Expected string, got: %t",emailRaw),
 Message: fmt.Sprintf("Invalid group type. Expected string, got: %T",groupInterface),
 })
 return
 }
 http.Redirect(rw, r, redirect, http.StatusTemporaryRedirect)
 }

 // claimFields returns the sorted list of fields in the claims map.
 func claimFields(claims map[string]interface{}) []string {
 fields := []string{}
 for field := range claims {
 fields = append(fields, field)
 }
 sort.Strings(fields)
 return fields
 }

 // blankFields returns the list of fields in the claims map that are
 // an empty string.
 func blankFields(claims map[string]interface{}) []string {
 fields := make([]string, 0)
 for field, value := range claims {
 if valueStr, ok := value.(string); ok && valueStr == "" {
 fields = append(fields, field)
 }
 }
 sort.Strings(fields)
 return fields
 }

 // mergeClaims merges the claims from a and b and returns the merged set.
 // claims from b take precedence over claims from a.
 func mergeClaims(a, b map[string]interface{}) map[string]interface{} {
 c := make(map[string]interface{})
 for k, v := range a {
 c[k] = v
 }
 for k, v := range b {
 c[k] = v
 }
 return c
 }

 type oauthLoginParams struct {
 User      database.User
 Link      database.UserLink
diff --git a/coderd/userauth_test.go b/coderd/userauth_test.go
 AvatarURL           string
 StatusCode          int
 IgnoreEmailVerified bool
 IgnoreUserInfo      bool
 }{{
 Name: "EmailOnly",
 IDTokenClaims: jwt.MapClaims{
 },
 AllowSignups: true,
 StatusCode:   http.StatusTemporaryRedirect,
 }, {
 Name: "UserInfoOverridesIDTokenClaims",
 IDTokenClaims: jwt.MapClaims{
 "email":          "internaluser@internal.domain",
 "email_verified": false,
 },
 UserInfoClaims: jwt.MapClaims{
 "email":              "externaluser@external.domain",
 "email_verified":     true,
 "preferred_username": "user",
 },
 Username:            "user",
 AllowSignups:        true,
 IgnoreEmailVerified: false,
 StatusCode:          http.StatusTemporaryRedirect,
 }, {
 Name: "InvalidUserInfo",
 IDTokenClaims: jwt.MapClaims{
 "email":          "internaluser@internal.domain",
 "email_verified": false,
 },
 UserInfoClaims: jwt.MapClaims{
 "email": 1,
 },
 AllowSignups:        true,
 IgnoreEmailVerified: false,
 StatusCode:          http.StatusInternalServerError,
 }, {
 Name: "IgnoreUserInfo",
 IDTokenClaims: jwt.MapClaims{
 "email":              "user@internal.domain",
 "email_verified":     true,
 "preferred_username": "user",
 },
 UserInfoClaims: jwt.MapClaims{
 "email":              "user.mcname@external.domain",
 "preferred_username": "Mr. User McName",
 },
 Username:       "user",
 IgnoreUserInfo: true,
 AllowSignups:   true,
 StatusCode:     http.StatusTemporaryRedirect,
 }} {
 tc := tc
 t.Run(tc.Name, func(t *testing.T) {
 config.AllowSignups = tc.AllowSignups
 config.EmailDomain = tc.EmailDomain
 config.IgnoreEmailVerified = tc.IgnoreEmailVerified
 config.IgnoreUserInfo = tc.IgnoreUserInfo

 client := coderdtest.New(t, &coderdtest.Options{
 Auditor:    auditor,
diff --git a/codersdk/deployment.go b/codersdk/deployment.go
 UsernameField       clibase.String                    `json:"username_field" typescript:",notnull"`
 EmailField          clibase.String                    `json:"email_field" typescript:",notnull"`
 AuthURLParams       clibase.Struct[map[string]string] `json:"auth_url_params" typescript:",notnull"`
 IgnoreUserInfo      clibase.Bool                      `json:"ignore_user_info" typescript:",notnull"`
 GroupField          clibase.String                    `json:"groups_field" typescript:",notnull"`
 GroupMapping        clibase.Struct[map[string]string] `json:"group_mapping" typescript:",notnull"`
 SignInText          clibase.String                    `json:"sign_in_text" typescript:",notnull"`
 Group:       &deploymentGroupOIDC,
 YAML:        "authURLParams",
 },
 {
 Name:        "OIDC Ignore UserInfo",
 Description: "Ignore the userinfo endpoint and only use the ID token for user information.",
 Flag:        "oidc-ignore-userinfo",
 Env:         "CODER_OIDC_IGNORE_USERINFO",
 Default:     "false",
 Value:       &c.OIDC.IgnoreUserInfo,
 Group:       &deploymentGroupOIDC,
 YAML:        "ignoreUserInfo",
 },
 {
 Name:        "OIDC Group Field",
 Description: "Change the OIDC default 'groups' claim field. By default, will be 'groups' if present in the oidc scopes argument.",
 Name:        "OIDC Group Mapping",
 Description: "A map of OIDC group IDs and the group in Coder it should map to. This is useful for when OIDC providers only return group IDs.",
 Flag:        "oidc-group-mapping",
 Env:         "OIDC_GROUP_MAPPING",
 Env:         "CODER_OIDC_GROUP_MAPPING",
 Default:     "{}",
 Value:       &c.OIDC.GroupMapping,
 Group:       &deploymentGroupOIDC,
diff --git a/docs/admin/auth.md b/docs/admin/auth.md
 returned by the upstream identity provider in a message containing the
 string `got oidc claims`, as well as the user info returned.

 > **Note:** If you need to ensure that Coder only uses information from
 > the ID token and does not hit the UserInfo endpoint, you can set the
 > configuration option `CODER_OIDC_IGNORE_USERINFO=true`.

 ### Email Addresses

 By default, Coder will look for the OIDC claim named `email` and use that
   steps as described [here.](https://learn.microsoft.com/en-us/windows-server/identity/ad-fs/development/msal/adfs-msal-web-app-web-api#app-registration-in-ad-fs)
   - **Server Application**: Note the Client ID.
   - **Configure Application Credentials**: Note the Client Secret.
   - **Configure Web API**:Ensure the Client ID is set as the relying party identifier.
   - **Application Permissions**: Allow access to the claims `openid`, `email`, and `profile`.
   - **Configure Web API**:Set the Client ID as the relying party identifier.
   - **Application Permissions**: Allow access to the claims `openid`, `email`,`profile`,and `allatclaims`.
 1. Visit your ADFS server's `/.well-known/openid-configuration` URL and note
   the value for `issuer`.
   > **Note:** This is usually of the form `https://adfs.corp/adfs/.well-known/openid-configuration`
 1. In Coder's configuration file (or Helm values as appropriate), set the following
   environment variables or their corresponding CLI arguments:

   - `CODER_OIDC_ISSUER_URL`: the `issuer` value from the previous step.
   - `CODER_OIDC_CLIENT_ID`: the Client ID from step 1.
   - `CODER_OIDC_CLIENT_SECRET`: the Client Secret from step 1.
   - `CODER_OIDC_AUTH_URL_PARAMS`: set to `{"resource":"urn:microsoft:userinfo"}` ([see here](https://learn.microsoft.com/en-us/windows-server/identity/ad-fs/overview/ad-fs-openid-connect-oauth-flows-scenarios#:~:text=scope%E2%80%AFopenid.-,resource,-optional)). OIDC logins will fail if this is not set.
 1. Ensure that Coder has the required OIDC claims by performing either of the below:
   - Configure your federation server to reuturn both the `email` and `preferred_username` fields by [creating a custom claim rule](https://learn.microsoft.com/en-us/windows-server/identity/ad-fs/operations/create-a-rule-to-send-ldap-attributes-as-claims), or
   - Set `CODER_OIDC_EMAIL_FIELD="upn"`. This will use the User Principal Name as the user email, which is [guaranteed to be unique in an Active Directory Forest](https://learn.microsoft.com/en-us/windows/win32/ad/naming-properties#upn-format).
   - `CODER_OIDC_AUTH_URL_PARAMS`: set to

     ```console
     {"resource":"$CLIENT_ID"}
     ```

     where `$CLIENT_ID` is the Client ID from step 1 ([see here](https://learn.microsoft.com/en-us/windows-server/identity/ad-fs/overview/ad-fs-openid-connect-oauth-flows-scenarios#:~:text=scope%E2%80%AFopenid.-,resource,-optional)).
     This is required for the upstream OIDC provider to return the requested claims.

   - `CODER_OIDC_IGNORE_USERINFO`: Set to `true`.

 1. Configure [Issuance Transform Rules](https://learn.microsoft.com/en-us/windows-server/identity/ad-fs/operations/create-a-rule-to-send-ldap-attributes-as-claims)
   on your federation server to send the following claims:

   - `preferred_username`: You can use e.g. "Display Name" as required.
   - `email`: You can use e.g. the LDAP attribute "E-Mail-Addresses" as required.
   - `email_verified`: Create a custom claim rule:

     ```console
     => issue(Type = "email_verified", Value = "true")
     ```

   - (Optional) If using Group Sync, send the required groups in the configured groups claim field. See [here](https://stackoverflow.com/a/55570286) for an example.
diff --git a/docs/api/general.md b/docs/api/general.md
        "user": {}
      },
      "ignore_email_verified": true,
      "ignore_user_info": true,
      "issuer_url": "string",
      "scopes": ["string"],
      "sign_in_text": "string",
Original file line number	Diff line number	Diff line change
Expand Up		@@ -731,6 +731,7 @@ func (r RootCmd) Server(newAPI func(context.Context, coderd.Options) (*coderd.
		UsernameField: cfg.OIDC.UsernameField.String(),
		EmailField: cfg.OIDC.EmailField.String(),
		AuthURLParams: cfg.OIDC.AuthURLParams.Value,
		IgnoreUserInfo: cfg.OIDC.IgnoreUserInfo.Value(),
		GroupField: cfg.OIDC.GroupField.String(),
		GroupMapping: cfg.OIDC.GroupMapping.Value,
		SignInText: cfg.OIDC.SignInText.String(),
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -1086,6 +1086,7 @@ func TestServer(t *testing.T) {
		require.Equal(t, "preferred_username", deploymentConfig.Values.OIDC.UsernameField.Value())
		require.Equal(t, "email", deploymentConfig.Values.OIDC.EmailField.Value())
		require.Equal(t, map[string]string{"access_type": "offline"}, deploymentConfig.Values.OIDC.AuthURLParams.Value)
		require.False(t, deploymentConfig.Values.OIDC.IgnoreUserInfo.Value())
		require.Empty(t, deploymentConfig.Values.OIDC.GroupField.Value())
		require.Empty(t, deploymentConfig.Values.OIDC.GroupMapping.Value)
		require.Equal(t, "OpenID Connect", deploymentConfig.Values.OIDC.SignInText.Value())
Expand DownExpand Up		@@ -1125,6 +1126,7 @@ func TestServer(t *testing.T) {
		"--oidc-username-field", "not_preferred_username",
		"--oidc-email-field", "not_email",
		"--oidc-auth-url-params", `{"prompt":"consent"}`,
		"--oidc-ignore-userinfo",
		"--oidc-group-field", "serious_business_unit",
		"--oidc-group-mapping", `{"serious_business_unit": "serious_business_unit"}`,
		"--oidc-sign-in-text", "Sign In With Coder",
Expand DownExpand Up		@@ -1169,6 +1171,7 @@ func TestServer(t *testing.T) {
		require.True(t, deploymentConfig.Values.OIDC.IgnoreEmailVerified.Value())
		require.Equal(t, "not_preferred_username", deploymentConfig.Values.OIDC.UsernameField.Value())
		require.Equal(t, "not_email", deploymentConfig.Values.OIDC.EmailField.Value())
		require.True(t, deploymentConfig.Values.OIDC.IgnoreUserInfo.Value())
		require.Equal(t, map[string]string{"prompt": "consent"}, deploymentConfig.Values.OIDC.AuthURLParams.Value)
		require.Equal(t, "serious_business_unit", deploymentConfig.Values.OIDC.GroupField.Value())
		require.Equal(t, map[string]string{"serious_business_unit": "serious_business_unit"}, deploymentConfig.Values.OIDC.GroupMapping.Value)
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -280,13 +280,17 @@ can safely ignore these settings.
		Change the OIDC default 'groups' claim field. By default, will be
		'groups' if present in the oidc scopes argument.

		--oidc-group-mapping struct[map[string]string], $OIDC_GROUP_MAPPING (default: {})
		--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
		is useful for when OIDC providers only return group IDs.

		--oidc-ignore-email-verified bool, $CODER_OIDC_IGNORE_EMAIL_VERIFIED
		Ignore the email_verified claim from the upstream provider.

		--oidc-ignore-userinfo bool, $CODER_OIDC_IGNORE_USERINFO (default: false)
		Ignore the userinfo endpoint and only use the ID token for user
		information.

		--oidc-issuer-url string, $CODER_OIDC_ISSUER_URL
		Issuer URL to use for Login with OIDC.

Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -7,6 +7,7 @@ import (
		"fmt"
		"net/http"
		"net/mail"
		"sort"
		"strconv"
		"strings"

Expand DownExpand Up		@@ -483,6 +484,11 @@ type OIDCConfig struct {
		// AuthURLParams are additional parameters to be passed to the OIDC provider
		// when requesting an access token.
		AuthURLParams map[string]string
		// IgnoreUserInfo causes Coder to only use claims from the ID token to
		// process OIDC logins. This is useful if the OIDC provider does not
		// support the userinfo endpoint, or if the userinfo endpoint causes
		// undesirable behavior.
		IgnoreUserInfo bool
		// GroupField selects the claim field to be used as the created user's
		// groups. If the group field is the empty string, then no group updates
		// will ever come from the OIDC provider.
Expand DownExpand Up		@@ -551,46 +557,62 @@ func (api API) userOIDC(rw http.ResponseWriter, r http.Request) {
		return
		}

		api.Logger.Debug(ctx, "got oidc claims",
		slog.F("source", "id_token"),
		slog.F("claim_fields", claimFields(claims)),
		slog.F("blank", blankFields(claims)),
		)

		// Not all claims are necessarily embedded in the `id_token`.
		// In GitLab, the username is left empty and must be fetched in UserInfo.
		//
		// The OIDC specification says claims can be in either place, so we fetch
		// user info and merge the two claim sets to be sure we have all of
		// the correct data.
		userInfo, err := api.OIDCConfig.Provider.UserInfo(ctx, oauth2.StaticTokenSource(state.Token))
		if err == nil {
		userInfoClaims := map[string]interface{}{}
		err = userInfo.Claims(&userInfoClaims)
		if err != nil {
		// user info if required and merge the two claim sets to be sure we have
		// all of the correct data.
		//
		// Some providers (e.g. ADFS) do not support custom OIDC claims in the
		// UserInfo endpoint, so we allow users to disable it and only rely on the
		// ID token.
		if !api.OIDCConfig.IgnoreUserInfo {
		userInfo, err := api.OIDCConfig.Provider.UserInfo(ctx, oauth2.StaticTokenSource(state.Token))
		if err == nil {
		userInfoClaims := map[string]interface{}{}
		err = userInfo.Claims(&userInfoClaims)
		if err != nil {
		httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
		Message: "Failed to unmarshal user info claims.",
		Detail: err.Error(),
		})
		return
		}
		api.Logger.Debug(ctx, "got oidc claims",
		slog.F("source", "userinfo"),
		slog.F("claim_fields", claimFields(userInfoClaims)),
		slog.F("blank", blankFields(userInfoClaims)),
		)

		// Merge the claims from the ID token and the UserInfo endpoint.
		// Information from UserInfo takes precedence.
		claims = mergeClaims(claims, userInfoClaims)

		// Log all of the field names after merging.
		api.Logger.Debug(ctx, "got oidc claims",
		slog.F("source", "merged"),
		slog.F("claim_fields", claimFields(claims)),
		slog.F("blank", blankFields(claims)),
		)
		} else if !strings.Contains(err.Error(), "user info endpoint is not supported by this provider") {
		httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
		Message: "Failed tounmarshal userinfo claims.",
		Detail: err.Error(),
		Message: "Failed toobtain userinformation claims.",
		Detail:"The attempt to fetch claims via the UserInfo endpoint failed: " +err.Error(),
		})
		return
		} else {
johnstcn marked this conversation as resolved. Show resolvedHide resolved
		// The OIDC provider does not support the UserInfo endpoint.
		// This is not an error, but we should log it as it may mean
		// that some claims are missing.
		api.Logger.Warn(ctx, "OIDC provider does not support the user info endpoint, ensure that all required claims are present in the id_token")
Comment on lines +611 to +614 Copy link Member kylecarbsMar 31, 2023 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Should we be printing an error here too? We don't have the required claims at this point, so it seems odd to just warn with logs but proceed. Copy link MemberAuthor johnstcnMar 31, 2023 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. If we're going to error, I'd say we should just fail the request outright. However, we could still theoretically successfully login a user if we're just missing something like the`preferred_username` field (i.e. we generate it from the email). I could remove the check for the username field as a "required" claim, WDYT? Copy link Member kylecarbsMar 31, 2023 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Ahh, I see. Up to you, I'm really not sure. Kinda uncharted territory for me. Copy link MemberAuthor johnstcnApr 4, 2023• edited Loading Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Instead of making the logic "magical", maybe a better choice is adding an option`CODER_OIDC_IGNORE_USERINFO` and keep the existing behaviour of always hitting UserInfo unless this option is set. This way, we keep the existing behaviour but allow folks who can't or don't want to use the UserInfo endpoint to just rely on what's in the id_token.
		}
		for k, v := range userInfoClaims {
		claims[k] = v
		}
		} else if !strings.Contains(err.Error(), "user info endpoint is not supported by this provider") {
		httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
		Message: "Failed to obtain user information claims.",
		Detail: "The OIDC provider returned no claims as part of the `id_token`. The attempt to fetch claims via the UserInfo endpoint failed: " + err.Error(),
		})
		return
		}

		// Log all of the field names returned in the ID token claims, and the
		// userinfo returned from the provider.
		{
		fields := make([]string, 0, len(claims))
		for f := range claims {
		fields = append(fields, f)
		}

		api.Logger.Debug(ctx, "got oidc claims",
		slog.F("user_info", userInfo),
Copy link MemberAuthor johnstcnMar 31, 2023• edited Loading Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. NOTE: we were previously logging all of the userinfo claims here; this could contain sensitive user information. I modified this to instead log the claim fields, and which of these claim fields are an empty string.
		slog.F("claim_fields", fields),
		)
		}

		usernameRaw, ok := claims[api.OIDCConfig.UsernameField]
Expand DownExpand Up		@@ -657,7 +679,7 @@ func (api API) userOIDC(rw http.ResponseWriter, r http.Request) {
		group, ok := groupInterface.(string)
		if !ok {
		httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
		Message: fmt.Sprintf("Invalid group type. Expected string, got: %t",emailRaw),
		Message: fmt.Sprintf("Invalid group type. Expected string, got: %T",groupInterface),
Copy link MemberAuthor johnstcnMar 31, 2023 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. NOTE: I'm assuming this was what the original author intended here kylecarbs reacted with thumbs up emoji
		})
		return
		}
Expand DownExpand Up		@@ -761,6 +783,42 @@ func (api API) userOIDC(rw http.ResponseWriter, r http.Request) {
		http.Redirect(rw, r, redirect, http.StatusTemporaryRedirect)
		}

		// claimFields returns the sorted list of fields in the claims map.
		func claimFields(claims map[string]interface{}) []string {
		fields := []string{}
		for field := range claims {
		fields = append(fields, field)
		}
		sort.Strings(fields)
		return fields
		}

		// blankFields returns the list of fields in the claims map that are
		// an empty string.
		func blankFields(claims map[string]interface{}) []string {
		fields := make([]string, 0)
johnstcn marked this conversation as resolved. Show resolvedHide resolved
		for field, value := range claims {
		if valueStr, ok := value.(string); ok && valueStr == "" {
		fields = append(fields, field)
		}
		}
		sort.Strings(fields)
		return fields
		}

		// mergeClaims merges the claims from a and b and returns the merged set.
		// claims from b take precedence over claims from a.
		func mergeClaims(a, b map[string]interface{}) map[string]interface{} {
		c := make(map[string]interface{})
		for k, v := range a {
		c[k] = v
		}
		for k, v := range b {
		c[k] = v
		}
		return c
		}

		type oauthLoginParams struct {
		User database.User
		Link database.UserLink
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -501,6 +501,7 @@ func TestUserOIDC(t *testing.T) {
		AvatarURL string
		StatusCode int
		IgnoreEmailVerified bool
		IgnoreUserInfo bool
		}{{
		Name: "EmailOnly",
		IDTokenClaims: jwt.MapClaims{
Expand DownExpand Up		@@ -643,6 +644,48 @@ func TestUserOIDC(t *testing.T) {
		},
		AllowSignups: true,
		StatusCode: http.StatusTemporaryRedirect,
		}, {
		Name: "UserInfoOverridesIDTokenClaims",
		IDTokenClaims: jwt.MapClaims{
		"email": "internaluser@internal.domain",
		"email_verified": false,
		},
		UserInfoClaims: jwt.MapClaims{
		"email": "externaluser@external.domain",
		"email_verified": true,
		"preferred_username": "user",
		},
		Username: "user",
		AllowSignups: true,
		IgnoreEmailVerified: false,
		StatusCode: http.StatusTemporaryRedirect,
		}, {
		Name: "InvalidUserInfo",
		IDTokenClaims: jwt.MapClaims{
		"email": "internaluser@internal.domain",
		"email_verified": false,
		},
		UserInfoClaims: jwt.MapClaims{
		"email": 1,
		},
		AllowSignups: true,
		IgnoreEmailVerified: false,
		StatusCode: http.StatusInternalServerError,
		}, {
		Name: "IgnoreUserInfo",
		IDTokenClaims: jwt.MapClaims{
		"email": "user@internal.domain",
		"email_verified": true,
		"preferred_username": "user",
		},
		UserInfoClaims: jwt.MapClaims{
		"email": "user.mcname@external.domain",
		"preferred_username": "Mr. User McName",
		},
		Username: "user",
		IgnoreUserInfo: true,
		AllowSignups: true,
		StatusCode: http.StatusTemporaryRedirect,
		}} {
		tc := tc
		t.Run(tc.Name, func(t *testing.T) {
Expand All		@@ -654,6 +697,7 @@ func TestUserOIDC(t *testing.T) {
		config.AllowSignups = tc.AllowSignups
		config.EmailDomain = tc.EmailDomain
		config.IgnoreEmailVerified = tc.IgnoreEmailVerified
		config.IgnoreUserInfo = tc.IgnoreUserInfo

		client := coderdtest.New(t, &coderdtest.Options{
		Auditor: auditor,
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -257,6 +257,7 @@ type OIDCConfig struct {
		UsernameField clibase.String `json:"username_field" typescript:",notnull"`
		EmailField clibase.String `json:"email_field" typescript:",notnull"`
		AuthURLParams clibase.Struct[map[string]string] `json:"auth_url_params" typescript:",notnull"`
		IgnoreUserInfo clibase.Bool `json:"ignore_user_info" typescript:",notnull"`
		GroupField clibase.String `json:"groups_field" typescript:",notnull"`
		GroupMapping clibase.Struct[map[string]string] `json:"group_mapping" typescript:",notnull"`
		SignInText clibase.String `json:"sign_in_text" typescript:",notnull"`
Expand DownExpand Up		@@ -881,6 +882,16 @@ when required by your organization's security policy.`,
		Group: &deploymentGroupOIDC,
		YAML: "authURLParams",
		},
		{
		Name: "OIDC Ignore UserInfo",
		Description: "Ignore the userinfo endpoint and only use the ID token for user information.",
		Flag: "oidc-ignore-userinfo",
		Env: "CODER_OIDC_IGNORE_USERINFO",
		Default: "false",
		Value: &c.OIDC.IgnoreUserInfo,
		Group: &deploymentGroupOIDC,
		YAML: "ignoreUserInfo",
		},
		{
		Name: "OIDC Group Field",
		Description: "Change the OIDC default 'groups' claim field. By default, will be 'groups' if present in the oidc scopes argument.",
Expand All		@@ -900,7 +911,7 @@ when required by your organization's security policy.`,
		Name: "OIDC Group Mapping",
		Description: "A map of OIDC group IDs and the group in Coder it should map to. This is useful for when OIDC providers only return group IDs.",
		Flag: "oidc-group-mapping",
		Env: "OIDC_GROUP_MAPPING",
		Env: "CODER_OIDC_GROUP_MAPPING",
Copy link Contributor coadlerApr 3, 2023 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Oops
		Default: "{}",
		Value: &c.OIDC.GroupMapping,
		Group: &deploymentGroupOIDC,
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -144,6 +144,10 @@ while signing in via OIDC as a new user. Coder will log the claim fields
		returned by the upstream identity provider in a message containing the
		string `got oidc claims`, as well as the user info returned.

		> Note: If you need to ensure that Coder only uses information from
		> the ID token and does not hit the UserInfo endpoint, you can set the
		> configuration option `CODER_OIDC_IGNORE_USERINFO=true`.

		### Email Addresses

		By default, Coder will look for the OIDC claim named `email` and use that
Expand DownExpand Up		@@ -262,17 +266,37 @@ Below are some details specific to individual OIDC providers.
		steps as described [here.](https://learn.microsoft.com/en-us/windows-server/identity/ad-fs/development/msal/adfs-msal-web-app-web-api#app-registration-in-ad-fs)
		- Server Application: Note the Client ID.
		- Configure Application Credentials: Note the Client Secret.
		- Configure Web API:Ensure the Client ID is set as the relying party identifier.
		- Application Permissions: Allow access to the claims `openid`, `email`, and `profile`.
		- Configure Web API:Set the Client ID as the relying party identifier.
		- Application Permissions: Allow access to the claims `openid`, `email`,`profile`,and `allatclaims`.
johnstcn marked this conversation as resolved. Show resolvedHide resolved
		1. Visit your ADFS server's `/.well-known/openid-configuration` URL and note
		the value for `issuer`.
		> Note: This is usually of the form `https://adfs.corp/adfs/.well-known/openid-configuration`
		1. In Coder's configuration file (or Helm values as appropriate), set the following
		environment variables or their corresponding CLI arguments:

		- `CODER_OIDC_ISSUER_URL`: the `issuer` value from the previous step.
		- `CODER_OIDC_CLIENT_ID`: the Client ID from step 1.
		- `CODER_OIDC_CLIENT_SECRET`: the Client Secret from step 1.
		- `CODER_OIDC_AUTH_URL_PARAMS`: set to `{"resource":"urn:microsoft:userinfo"}` ([see here](https://learn.microsoft.com/en-us/windows-server/identity/ad-fs/overview/ad-fs-openid-connect-oauth-flows-scenarios#:~:text=scope%E2%80%AFopenid.-,resource,-optional)). OIDC logins will fail if this is not set.
		1. Ensure that Coder has the required OIDC claims by performing either of the below:
		- Configure your federation server to reuturn both the `email` and `preferred_username` fields by [creating a custom claim rule](https://learn.microsoft.com/en-us/windows-server/identity/ad-fs/operations/create-a-rule-to-send-ldap-attributes-as-claims), or
		- Set `CODER_OIDC_EMAIL_FIELD="upn"`. This will use the User Principal Name as the user email, which is [guaranteed to be unique in an Active Directory Forest](https://learn.microsoft.com/en-us/windows/win32/ad/naming-properties#upn-format).
		- `CODER_OIDC_AUTH_URL_PARAMS`: set to

		```console
		{"resource":"$CLIENT_ID"}
		```

		where `$CLIENT_ID` is the Client ID from step 1 ([see here](https://learn.microsoft.com/en-us/windows-server/identity/ad-fs/overview/ad-fs-openid-connect-oauth-flows-scenarios#:~:text=scope%E2%80%AFopenid.-,resource,-optional)).
		This is required for the upstream OIDC provider to return the requested claims.

		- `CODER_OIDC_IGNORE_USERINFO`: Set to `true`.

		1. Configure [Issuance Transform Rules](https://learn.microsoft.com/en-us/windows-server/identity/ad-fs/operations/create-a-rule-to-send-ldap-attributes-as-claims)
johnstcn marked this conversation as resolved. Show resolvedHide resolved
		on your federation server to send the following claims:

		- `preferred_username`: You can use e.g. "Display Name" as required.
		- `email`: You can use e.g. the LDAP attribute "E-Mail-Addresses" as required.
		- `email_verified`: Create a custom claim rule:

		```console
		=> issue(Type = "email_verified", Value = "true")
		```

		- (Optional) If using Group Sync, send the required groups in the configured groups claim field. See [here](https://stackoverflow.com/a/55570286) for an example.
Original file line number	Diff line number	Diff line change
Expand Up		@@ -252,6 +252,7 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \
		"user": {}
		},
		"ignore_email_verified": true,
		"ignore_user_info": true,
		"issuer_url": "string",
		"scopes": ["string"],
		"sign_in_text": "string",
Expand Down