diff --git a/cli/server.go b/cli/server.go index 409056641a771..855c5e6c547bf 100644 --- a/cli/server.go +++ b/cli/server.go @@ -169,6 +169,7 @@ func createOIDCConfig(ctx context.Context, vals *codersdk.DeploymentValues) (*co EmailDomain: vals.OIDC.EmailDomain, AllowSignups: vals.OIDC.AllowSignups.Value(), UsernameField: vals.OIDC.UsernameField.String(), + NameField: vals.OIDC.NameField.String(), EmailField: vals.OIDC.EmailField.String(), AuthURLParams: vals.OIDC.AuthURLParams.Value, IgnoreUserInfo: vals.OIDC.IgnoreUserInfo.Value(), diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index 6d8f866c11c0b..acd2c62ead445 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -407,6 +407,9 @@ OIDC OPTIONS: --oidc-issuer-url string, $CODER_OIDC_ISSUER_URL Issuer URL to use for Login with OIDC. + --oidc-name-field string, $CODER_OIDC_NAME_FIELD (default: name) + OIDC claim field to use as the name. + --oidc-group-regex-filter regexp, $CODER_OIDC_GROUP_REGEX_FILTER (default: .*) If provided any group name not matching the regex is ignored. This allows for filtering out groups that are not needed. This filter is diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden index bf49239bc4e63..9a34d6be56b20 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -306,6 +306,9 @@ oidc: # OIDC claim field to use as the username. # (default: preferred_username, type: string) usernameField: preferred_username + # OIDC claim field to use as the name. + # (default: name, type: string) + nameField: name # OIDC claim field to use as the email. # (default: email, type: string) emailField: email diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index c5e2a6041526f..cca1e1d444407 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -10551,6 +10551,9 @@ const docTemplate = `{ "issuer_url": { "type": "string" }, + "name_field": { + "type": "string" + }, "scopes": { "type": "array", "items": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 66afad1f041f0..4f78e71b67c12 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -9482,6 +9482,9 @@ "issuer_url": { "type": "string" }, + "name_field": { + "type": "string" + }, "scopes": { "type": "array", "items": { diff --git a/coderd/httpapi/name.go b/coderd/httpapi/name.go index d8b64a71bdc44..cdf4a9ad48971 100644 --- a/coderd/httpapi/name.go +++ b/coderd/httpapi/name.go @@ -91,3 +91,14 @@ func UserRealNameValid(str string) error { } return nil } + +// NormalizeUserRealName normalizes a user name such that it will pass +// validation by UserRealNameValid. This is done to avoid blocking +// little Bobby Whitespace from using Coder. +func NormalizeRealUsername(str string) string { + s := strings.TrimSpace(str) + if len(s) > 128 { + s = s[:128] + } + return s +} diff --git a/coderd/httpapi/name_test.go b/coderd/httpapi/name_test.go index a6313c54034f5..15da7b3e82a63 100644 --- a/coderd/httpapi/name_test.go +++ b/coderd/httpapi/name_test.go @@ -1,9 +1,11 @@ package httpapi_test import ( + "strings" "testing" "github.com/moby/moby/pkg/namesgenerator" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/coder/coder/v2/coderd/httpapi" @@ -217,6 +219,10 @@ func TestUserRealNameValid(t *testing.T) { Name string Valid bool }{ + {"", true}, + {" a", false}, + {"a ", false}, + {" a ", false}, {"1", true}, {"A", true}, {"A1", true}, @@ -229,17 +235,22 @@ func TestUserRealNameValid(t *testing.T) { {"Małgorzata Kalinowska-Iszkowska", true}, {"成龍", true}, {". .", true}, - {"Lord Voldemort ", false}, {" Bellatrix Lestrange", false}, {" ", false}, + {strings.Repeat("a", 128), true}, + {strings.Repeat("a", 129), false}, } for _, testCase := range testCases { testCase := testCase t.Run(testCase.Name, func(t *testing.T) { t.Parallel() - valid := httpapi.UserRealNameValid(testCase.Name) - require.Equal(t, testCase.Valid, valid == nil) + err := httpapi.UserRealNameValid(testCase.Name) + norm := httpapi.NormalizeRealUsername(testCase.Name) + normErr := httpapi.UserRealNameValid(norm) + assert.NoError(t, normErr) + assert.Equal(t, testCase.Valid, err == nil) + assert.Equal(t, testCase.Valid, norm == testCase.Name, "invalid name should be different after normalization") }) } } diff --git a/coderd/userauth.go b/coderd/userauth.go index 3f341db65bcb1..b41f496814306 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -607,6 +607,9 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) { return } + ghName := ghUser.GetName() + normName := httpapi.NormalizeRealUsername(ghName) + // If we have a nil GitHub ID, that is a big problem. That would mean we link // this user and all other users with this bug to the same uuid. // We should instead throw an error. This should never occur in production. @@ -652,6 +655,7 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) { Email: verifiedEmail.GetEmail(), Username: ghUser.GetLogin(), AvatarURL: ghUser.GetAvatarURL(), + Name: normName, DebugContext: OauthDebugContext{}, }).SetInitAuditRequest(func(params *audit.RequestParams) (*audit.Request[database.User], func()) { return audit.InitRequest[database.User](rw, params) @@ -701,6 +705,9 @@ type OIDCConfig struct { // EmailField selects the claim field to be used as the created user's // email. EmailField string + // NameField selects the claim field to be used as the created user's + // full / given name. + NameField string // AuthURLParams are additional parameters to be passed to the OIDC provider // when requesting an access token. AuthURLParams map[string]string @@ -952,13 +959,22 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { } } + // The 'name' is an optional property in Coder. If not specified, + // it will be left blank. + var name string + nameRaw, ok := mergedClaims[api.OIDCConfig.NameField] + if ok { + name, _ = nameRaw.(string) + name = httpapi.NormalizeRealUsername(name) + } + var picture string pictureRaw, ok := mergedClaims["picture"] if ok { picture, _ = pictureRaw.(string) } - ctx = slog.With(ctx, slog.F("email", email), slog.F("username", username)) + ctx = slog.With(ctx, slog.F("email", email), slog.F("username", username), slog.F("name", name)) usingGroups, groups, groupErr := api.oidcGroups(ctx, mergedClaims) if groupErr != nil { groupErr.Write(rw, r) @@ -996,6 +1012,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { AllowSignups: api.OIDCConfig.AllowSignups, Email: email, Username: username, + Name: name, AvatarURL: picture, UsingRoles: api.OIDCConfig.RoleSyncEnabled(), Roles: roles, @@ -1222,6 +1239,7 @@ type oauthLoginParams struct { AllowSignups bool Email string Username string + Name string AvatarURL string // Is UsingGroups is true, then the user will be assigned // to the Groups provided. @@ -1544,6 +1562,10 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C user.AvatarURL = params.AvatarURL needsUpdate = true } + if user.Name != params.Name { + user.Name = params.Name + needsUpdate = true + } // If the upstream email or username has changed we should mirror // that in Coder. Many enterprises use a user's email/username as diff --git a/coderd/userauth_test.go b/coderd/userauth_test.go index f1adbfe869610..1c647f3cca281 100644 --- a/coderd/userauth_test.go +++ b/coderd/userauth_test.go @@ -16,6 +16,7 @@ import ( "github.com/google/go-github/v43/github" "github.com/google/uuid" "github.com/prometheus/client_golang/prometheus" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/xerrors" @@ -213,6 +214,7 @@ func TestUserOAuth2Github(t *testing.T) { return &github.User{ ID: github.Int64(100), Login: github.String("kyle"), + Name: github.String("Kylium Carbonate"), }, nil }, TeamMembership: func(ctx context.Context, client *http.Client, org, team, username string) (*github.Membership, error) { @@ -272,7 +274,9 @@ func TestUserOAuth2Github(t *testing.T) { }, AuthenticatedUser: func(ctx context.Context, client *http.Client) (*github.User, error) { return &github.User{ - ID: github.Int64(100), + ID: github.Int64(100), + Login: github.String("testuser"), + Name: github.String("The Right Honorable Sir Test McUser"), }, nil }, ListEmails: func(ctx context.Context, client *http.Client) ([]*github.UserEmail, error) { @@ -305,7 +309,9 @@ func TestUserOAuth2Github(t *testing.T) { }, AuthenticatedUser: func(ctx context.Context, client *http.Client) (*github.User, error) { return &github.User{ - ID: github.Int64(100), + ID: github.Int64(100), + Login: github.String("testuser"), + Name: github.String("The Right Honorable Sir Test McUser"), }, nil }, ListEmails: func(ctx context.Context, client *http.Client) ([]*github.UserEmail, error) { @@ -346,9 +352,10 @@ func TestUserOAuth2Github(t *testing.T) { }, AuthenticatedUser: func(ctx context.Context, _ *http.Client) (*github.User, error) { return &github.User{ - Login: github.String("kyle"), - ID: i64ptr(1234), AvatarURL: github.String("/hello-world"), + ID: i64ptr(1234), + Login: github.String("kyle"), + Name: github.String("Kylium Carbonate"), }, nil }, ListEmails: func(ctx context.Context, client *http.Client) ([]*github.UserEmail, error) { @@ -372,6 +379,60 @@ func TestUserOAuth2Github(t *testing.T) { require.NoError(t, err) require.Equal(t, "kyle@coder.com", user.Email) require.Equal(t, "kyle", user.Username) + require.Equal(t, "Kylium Carbonate", user.Name) + require.Equal(t, "/hello-world", user.AvatarURL) + + require.Len(t, auditor.AuditLogs(), numLogs) + require.NotEqual(t, auditor.AuditLogs()[numLogs-1].UserID, uuid.Nil) + require.Equal(t, database.AuditActionRegister, auditor.AuditLogs()[numLogs-1].Action) + }) + t.Run("SignupWeirdName", func(t *testing.T) { + t.Parallel() + auditor := audit.NewMock() + client := coderdtest.New(t, &coderdtest.Options{ + Auditor: auditor, + GithubOAuth2Config: &coderd.GithubOAuth2Config{ + OAuth2Config: &testutil.OAuth2Config{}, + AllowOrganizations: []string{"coder"}, + AllowSignups: true, + ListOrganizationMemberships: func(_ context.Context, _ *http.Client) ([]*github.Membership, error) { + return []*github.Membership{{ + State: &stateActive, + Organization: &github.Organization{ + Login: github.String("coder"), + }, + }}, nil + }, + AuthenticatedUser: func(_ context.Context, _ *http.Client) (*github.User, error) { + return &github.User{ + AvatarURL: github.String("/hello-world"), + ID: i64ptr(1234), + Login: github.String("kyle"), + Name: github.String(" " + strings.Repeat("a", 129) + " "), + }, nil + }, + ListEmails: func(_ context.Context, _ *http.Client) ([]*github.UserEmail, error) { + return []*github.UserEmail{{ + Email: github.String("kyle@coder.com"), + Verified: github.Bool(true), + Primary: github.Bool(true), + }}, nil + }, + }, + }) + numLogs := len(auditor.AuditLogs()) + + resp := oauth2Callback(t, client) + numLogs++ // add an audit log for login + + require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) + + client.SetSessionToken(authCookieValue(resp.Cookies())) + user, err := client.User(context.Background(), "me") + require.NoError(t, err) + require.Equal(t, "kyle@coder.com", user.Email) + require.Equal(t, "kyle", user.Username) + require.Equal(t, strings.Repeat("a", 128), user.Name) require.Equal(t, "/hello-world", user.AvatarURL) require.Len(t, auditor.AuditLogs(), numLogs) @@ -401,8 +462,10 @@ func TestUserOAuth2Github(t *testing.T) { }, AuthenticatedUser: func(ctx context.Context, client *http.Client) (*github.User, error) { return &github.User{ - ID: github.Int64(100), - Login: github.String("kyle"), + AvatarURL: github.String("/hello-world"), + ID: github.Int64(100), + Login: github.String("kyle"), + Name: github.String("Kylium Carbonate"), }, nil }, ListEmails: func(ctx context.Context, client *http.Client) ([]*github.UserEmail, error) { @@ -419,10 +482,19 @@ func TestUserOAuth2Github(t *testing.T) { resp := oauth2Callback(t, client) numLogs++ // add an audit log for login + client.SetSessionToken(authCookieValue(resp.Cookies())) + user, err := client.User(context.Background(), "me") + require.NoError(t, err) + require.Equal(t, "kyle@coder.com", user.Email) + require.Equal(t, "kyle", user.Username) + require.Equal(t, "Kylium Carbonate", user.Name) + require.Equal(t, "/hello-world", user.AvatarURL) + require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) require.Len(t, auditor.AuditLogs(), numLogs) require.Equal(t, database.AuditActionRegister, auditor.AuditLogs()[numLogs-1].Action) }) + // nolint: dupl t.Run("SignupAllowedTeamInFirstOrganization", func(t *testing.T) { t.Parallel() auditor := audit.NewMock() @@ -456,6 +528,7 @@ func TestUserOAuth2Github(t *testing.T) { return &github.User{ ID: github.Int64(100), Login: github.String("mathias"), + Name: github.String("Mathias Mathias"), }, nil }, ListEmails: func(ctx context.Context, client *http.Client) ([]*github.UserEmail, error) { @@ -472,10 +545,18 @@ func TestUserOAuth2Github(t *testing.T) { resp := oauth2Callback(t, client) numLogs++ // add an audit log for login + client.SetSessionToken(authCookieValue(resp.Cookies())) + user, err := client.User(context.Background(), "me") + require.NoError(t, err) + require.Equal(t, "mathias@coder.com", user.Email) + require.Equal(t, "mathias", user.Username) + require.Equal(t, "Mathias Mathias", user.Name) + require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) require.Len(t, auditor.AuditLogs(), numLogs) require.Equal(t, database.AuditActionRegister, auditor.AuditLogs()[numLogs-1].Action) }) + // nolint: dupl t.Run("SignupAllowedTeamInSecondOrganization", func(t *testing.T) { t.Parallel() auditor := audit.NewMock() @@ -509,6 +590,7 @@ func TestUserOAuth2Github(t *testing.T) { return &github.User{ ID: github.Int64(100), Login: github.String("mathias"), + Name: github.String("Mathias Mathias"), }, nil }, ListEmails: func(ctx context.Context, client *http.Client) ([]*github.UserEmail, error) { @@ -525,6 +607,13 @@ func TestUserOAuth2Github(t *testing.T) { resp := oauth2Callback(t, client) numLogs++ // add an audit log for login + client.SetSessionToken(authCookieValue(resp.Cookies())) + user, err := client.User(context.Background(), "me") + require.NoError(t, err) + require.Equal(t, "mathias@coder.com", user.Email) + require.Equal(t, "mathias", user.Username) + require.Equal(t, "Mathias Mathias", user.Name) + require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) require.Len(t, auditor.AuditLogs(), numLogs) require.Equal(t, database.AuditActionRegister, auditor.AuditLogs()[numLogs-1].Action) @@ -548,6 +637,7 @@ func TestUserOAuth2Github(t *testing.T) { return &github.User{ ID: github.Int64(100), Login: github.String("mathias"), + Name: github.String("Mathias Mathias"), }, nil }, ListEmails: func(ctx context.Context, client *http.Client) ([]*github.UserEmail, error) { @@ -564,6 +654,13 @@ func TestUserOAuth2Github(t *testing.T) { resp := oauth2Callback(t, client) numLogs++ // add an audit log for login + client.SetSessionToken(authCookieValue(resp.Cookies())) + user, err := client.User(context.Background(), "me") + require.NoError(t, err) + require.Equal(t, "mathias@coder.com", user.Email) + require.Equal(t, "mathias", user.Username) + require.Equal(t, "Mathias Mathias", user.Name) + require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) require.Len(t, auditor.AuditLogs(), numLogs) require.Equal(t, database.AuditActionRegister, auditor.AuditLogs()[numLogs-1].Action) @@ -591,6 +688,7 @@ func TestUserOAuth2Github(t *testing.T) { return &github.User{ ID: github.Int64(100), Login: github.String("kyle"), + Name: github.String("Kylium Carbonate"), }, nil }, ListEmails: func(ctx context.Context, client *http.Client) ([]*github.UserEmail, error) { @@ -652,6 +750,7 @@ func TestUserOAuth2Github(t *testing.T) { return &github.User{ Login: github.String("alice"), ID: github.Int64(ghID), + Name: github.String("Alice Liddell"), }, nil }, ListEmails: func(ctx context.Context, client *http.Client) ([]*github.UserEmail, error) { @@ -739,8 +838,7 @@ func TestUserOIDC(t *testing.T) { UserInfoClaims jwt.MapClaims AllowSignups bool EmailDomain []string - Username string - AvatarURL string + AssertUser func(t testing.TB, u codersdk.User) StatusCode int IgnoreEmailVerified bool IgnoreUserInfo bool @@ -752,7 +850,9 @@ func TestUserOIDC(t *testing.T) { }, AllowSignups: true, StatusCode: http.StatusOK, - Username: "kyle", + AssertUser: func(t testing.TB, u codersdk.User) { + assert.Equal(t, "kyle", u.Username) + }, }, { Name: "EmailNotVerified", @@ -778,9 +878,11 @@ func TestUserOIDC(t *testing.T) { "email": "kyle@kwc.io", "email_verified": false, }, - AllowSignups: true, - StatusCode: http.StatusOK, - Username: "kyle", + AllowSignups: true, + StatusCode: http.StatusOK, + AssertUser: func(t testing.TB, u codersdk.User) { + assert.Equal(t, u.Username, "kyle") + }, IgnoreEmailVerified: true, }, { @@ -802,6 +904,9 @@ func TestUserOIDC(t *testing.T) { "email_verified": true, }, AllowSignups: true, + AssertUser: func(t testing.TB, u codersdk.User) { + assert.Equal(t, u.Username, "kyle") + }, EmailDomain: []string{ "kwc.io", }, @@ -839,7 +944,9 @@ func TestUserOIDC(t *testing.T) { "email": "kyle@kwc.io", "email_verified": true, }, - Username: "kyle", + AssertUser: func(t testing.TB, u codersdk.User) { + assert.Equal(t, "kyle", u.Username) + }, AllowSignups: true, StatusCode: http.StatusOK, }, @@ -850,10 +957,56 @@ func TestUserOIDC(t *testing.T) { "email_verified": true, "preferred_username": "hotdog", }, - Username: "hotdog", + AssertUser: func(t testing.TB, u codersdk.User) { + assert.Equal(t, "hotdog", u.Username) + }, + AllowSignups: true, + StatusCode: http.StatusOK, + }, + { + Name: "FullNameFromClaims", + IDTokenClaims: jwt.MapClaims{ + "email": "kyle@kwc.io", + "email_verified": true, + "name": "Hot Dog", + }, + AssertUser: func(t testing.TB, u codersdk.User) { + assert.Equal(t, "Hot Dog", u.Name) + }, AllowSignups: true, StatusCode: http.StatusOK, }, + { + Name: "InvalidFullNameFromClaims", + IDTokenClaims: jwt.MapClaims{ + "email": "kyle@kwc.io", + "email_verified": true, + // Full names must be less or equal to than 128 characters in length. + // However, we should not fail to log someone in if their name is too long. + // Just truncate it. + "name": strings.Repeat("a", 129), + }, + AllowSignups: true, + StatusCode: http.StatusOK, + AssertUser: func(t testing.TB, u codersdk.User) { + assert.Equal(t, strings.Repeat("a", 128), u.Name) + }, + }, + { + Name: "FullNameWhitespace", + IDTokenClaims: jwt.MapClaims{ + "email": "kyle@kwc.io", + "email_verified": true, + // Full names must not have leading or trailing whitespace, but this is a + // daft reason to fail a login. + "name": " Bobby Whitespace ", + }, + AllowSignups: true, + StatusCode: http.StatusOK, + AssertUser: func(t testing.TB, u codersdk.User) { + assert.Equal(t, "Bobby Whitespace", u.Name) + }, + }, { // Services like Okta return the email as the username: // https://developer.okta.com/docs/reference/api/oidc/#base-claims-always-present @@ -861,9 +1014,12 @@ func TestUserOIDC(t *testing.T) { IDTokenClaims: jwt.MapClaims{ "email": "kyle@kwc.io", "email_verified": true, + "name": "Kylium Carbonate", "preferred_username": "kyle@kwc.io", }, - Username: "kyle", + AssertUser: func(t testing.TB, u codersdk.User) { + assert.Equal(t, "kyle", u.Username) + }, AllowSignups: true, StatusCode: http.StatusOK, }, @@ -873,7 +1029,10 @@ func TestUserOIDC(t *testing.T) { IDTokenClaims: jwt.MapClaims{ "preferred_username": "kyle@kwc.io", }, - Username: "kyle", + AssertUser: func(t testing.TB, u codersdk.User) { + assert.Equal(t, "kyle", u.Username) + assert.Empty(t, u.Name) + }, AllowSignups: true, StatusCode: http.StatusOK, }, @@ -885,9 +1044,11 @@ func TestUserOIDC(t *testing.T) { "preferred_username": "kyle", "picture": "/example.png", }, - Username: "kyle", + AssertUser: func(t testing.TB, u codersdk.User) { + assert.Equal(t, "/example.png", u.AvatarURL) + assert.Equal(t, "kyle", u.Username) + }, AllowSignups: true, - AvatarURL: "/example.png", StatusCode: http.StatusOK, }, { @@ -899,10 +1060,14 @@ func TestUserOIDC(t *testing.T) { UserInfoClaims: jwt.MapClaims{ "preferred_username": "potato", "picture": "/example.png", + "name": "Kylium Carbonate", + }, + AssertUser: func(t testing.TB, u codersdk.User) { + assert.Equal(t, "/example.png", u.AvatarURL) + assert.Equal(t, "Kylium Carbonate", u.Name) + assert.Equal(t, "potato", u.Username) }, - Username: "potato", AllowSignups: true, - AvatarURL: "/example.png", StatusCode: http.StatusOK, }, { @@ -925,7 +1090,9 @@ func TestUserOIDC(t *testing.T) { "email_verified": true, "preferred_username": "user", }, - Username: "user", + AssertUser: func(t testing.TB, u codersdk.User) { + assert.Equal(t, "user", u.Username) + }, AllowSignups: true, IgnoreEmailVerified: false, StatusCode: http.StatusOK, @@ -948,13 +1115,18 @@ func TestUserOIDC(t *testing.T) { IDTokenClaims: jwt.MapClaims{ "email": "user@internal.domain", "email_verified": true, + "name": "User McName", "preferred_username": "user", }, UserInfoClaims: jwt.MapClaims{ "email": "user.mcname@external.domain", + "name": "Mr. User McName", "preferred_username": "Mr. User McName", }, - Username: "user", + AssertUser: func(t testing.TB, u codersdk.User) { + assert.Equal(t, "user", u.Username) + assert.Equal(t, "User McName", u.Name) + }, IgnoreUserInfo: true, AllowSignups: true, StatusCode: http.StatusOK, @@ -965,7 +1137,9 @@ func TestUserOIDC(t *testing.T) { "email": "user@domain.tld", "email_verified": true, }, 65536), - Username: "user", + AssertUser: func(t testing.TB, u codersdk.User) { + assert.Equal(t, "user", u.Username) + }, AllowSignups: true, StatusCode: http.StatusOK, }, @@ -976,9 +1150,11 @@ func TestUserOIDC(t *testing.T) { "email_verified": true, }, UserInfoClaims: inflateClaims(t, jwt.MapClaims{}, 65536), - Username: "user", - AllowSignups: true, - StatusCode: http.StatusOK, + AssertUser: func(t testing.TB, u codersdk.User) { + assert.Equal(t, "user", u.Username) + }, + AllowSignups: true, + StatusCode: http.StatusOK, }, } { tc := tc @@ -996,6 +1172,7 @@ func TestUserOIDC(t *testing.T) { cfg.EmailDomain = tc.EmailDomain cfg.IgnoreEmailVerified = tc.IgnoreEmailVerified cfg.IgnoreUserInfo = tc.IgnoreUserInfo + cfg.NameField = "name" }) auditor := audit.NewMock() @@ -1013,22 +1190,13 @@ func TestUserOIDC(t *testing.T) { ctx := testutil.Context(t, testutil.WaitLong) - if tc.Username != "" { - user, err := client.User(ctx, "me") - require.NoError(t, err) - require.Equal(t, tc.Username, user.Username) - - require.Len(t, auditor.AuditLogs(), numLogs) - require.NotEqual(t, auditor.AuditLogs()[numLogs-1].UserID, uuid.Nil) - require.Equal(t, database.AuditActionRegister, auditor.AuditLogs()[numLogs-1].Action) - } - - if tc.AvatarURL != "" { + if tc.AssertUser != nil { user, err := client.User(ctx, "me") require.NoError(t, err) - require.Equal(t, tc.AvatarURL, user.AvatarURL) + tc.AssertUser(t, user) require.Len(t, auditor.AuditLogs(), numLogs) + require.NotEqual(t, uuid.Nil, auditor.AuditLogs()[numLogs-1].UserID) require.Equal(t, database.AuditActionRegister, auditor.AuditLogs()[numLogs-1].Action) } }) diff --git a/codersdk/deployment.go b/codersdk/deployment.go index c89a78668637d..21d33ebc81dc0 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -333,6 +333,7 @@ type OIDCConfig struct { Scopes serpent.StringArray `json:"scopes" typescript:",notnull"` IgnoreEmailVerified serpent.Bool `json:"ignore_email_verified" typescript:",notnull"` UsernameField serpent.String `json:"username_field" typescript:",notnull"` + NameField serpent.String `json:"name_field" typescript:",notnull"` EmailField serpent.String `json:"email_field" typescript:",notnull"` AuthURLParams serpent.Struct[map[string]string] `json:"auth_url_params" typescript:",notnull"` IgnoreUserInfo serpent.Bool `json:"ignore_user_info" typescript:",notnull"` @@ -1192,6 +1193,16 @@ when required by your organization's security policy.`, Group: &deploymentGroupOIDC, YAML: "usernameField", }, + { + Name: "OIDC Name Field", + Description: "OIDC claim field to use as the name.", + Flag: "oidc-name-field", + Env: "CODER_OIDC_NAME_FIELD", + Default: "name", + Value: &c.OIDC.NameField, + Group: &deploymentGroupOIDC, + YAML: "nameField", + }, { Name: "OIDC Email Field", Description: "OIDC claim field to use as the email.", diff --git a/docs/api/general.md b/docs/api/general.md index 52313409cb02c..84424331cf488 100644 --- a/docs/api/general.md +++ b/docs/api/general.md @@ -294,6 +294,7 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \ "ignore_email_verified": true, "ignore_user_info": true, "issuer_url": "string", + "name_field": "string", "scopes": ["string"], "sign_in_text": "string", "signups_disabled_text": "string", diff --git a/docs/api/schemas.md b/docs/api/schemas.md index 7770b091878bd..fffea09e86302 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -2094,6 +2094,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "ignore_email_verified": true, "ignore_user_info": true, "issuer_url": "string", + "name_field": "string", "scopes": ["string"], "sign_in_text": "string", "signups_disabled_text": "string", @@ -2467,6 +2468,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "ignore_email_verified": true, "ignore_user_info": true, "issuer_url": "string", + "name_field": "string", "scopes": ["string"], "sign_in_text": "string", "signups_disabled_text": "string", @@ -3546,6 +3548,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "ignore_email_verified": true, "ignore_user_info": true, "issuer_url": "string", + "name_field": "string", "scopes": ["string"], "sign_in_text": "string", "signups_disabled_text": "string", @@ -3577,6 +3580,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o | `ignore_email_verified` | boolean | false | | | | `ignore_user_info` | boolean | false | | | | `issuer_url` | string | false | | | +| `name_field` | string | false | | | | `scopes` | array of string | false | | | | `sign_in_text` | string | false | | | | `signups_disabled_text` | string | false | | | diff --git a/docs/cli/server.md b/docs/cli/server.md index a7c32c2d78420..ea3672a1cb2d7 100644 --- a/docs/cli/server.md +++ b/docs/cli/server.md @@ -514,6 +514,17 @@ Ignore the email_verified claim from the upstream provider. OIDC claim field to use as the username. +### --oidc-name-field + +| | | +| ----------- | ----------------------------------- | +| Type | string | +| Environment | $CODER_OIDC_NAME_FIELD | +| YAML | oidc.nameField | +| Default | name | + +OIDC claim field to use as the name. + ### --oidc-email-field | | | diff --git a/enterprise/cli/testdata/coder_server_--help.golden b/enterprise/cli/testdata/coder_server_--help.golden index 4d4576d6d57cc..2c094e84913f0 100644 --- a/enterprise/cli/testdata/coder_server_--help.golden +++ b/enterprise/cli/testdata/coder_server_--help.golden @@ -408,6 +408,9 @@ OIDC OPTIONS: --oidc-issuer-url string, $CODER_OIDC_ISSUER_URL Issuer URL to use for Login with OIDC. + --oidc-name-field string, $CODER_OIDC_NAME_FIELD (default: name) + OIDC claim field to use as the name. + --oidc-group-regex-filter regexp, $CODER_OIDC_GROUP_REGEX_FILTER (default: .*) If provided any group name not matching the regex is ignored. This allows for filtering out groups that are not needed. This filter is diff --git a/scripts/dev-oidc.sh b/scripts/dev-oidc.sh index 017c7f07c646d..6a6d6e08ac705 100755 --- a/scripts/dev-oidc.sh +++ b/scripts/dev-oidc.sh @@ -10,6 +10,7 @@ set -euo pipefail KEYCLOAK_VERSION="${KEYCLOAK_VERSION:-22.0}" +# NOTE: the trailing space in "lastName" is intentional. cat </tmp/example-realm.json { "realm": "coder", @@ -23,6 +24,8 @@ cat </tmp/example-realm.json { "username": "oidcuser", "email": "oidcuser@coder.com", + "firstName": "OIDC", + "lastName": "user ", "emailVerified": true, "enabled": true, "credentials": [ diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 88e5c7e508f67..9cc3113cbb214 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -756,6 +756,7 @@ export interface OIDCConfig { readonly scopes: string[]; readonly ignore_email_verified: boolean; readonly username_field: string; + readonly name_field: string; readonly email_field: string; readonly auth_url_params: Record; readonly ignore_user_info: boolean;