Skip to content

Commit 1131772

Browse files
johnstcnjohnstcn
authored
feat(coderd): set full name from IDP name claim (#13468)
* Updates OIDC and GitHub OAuth login to fetch set name from relevant claim fields * Adds CODER_OIDC_NAME_FIELD as configurable source of user name claim * Adds httpapi function to normalize a username such that it will pass validation * Adds firstName / lastName fields to dev OIDC setup
1 parent e743588 commit 1131772

File tree

16 files changed

+301
-42
lines changed

16 files changed

+301
-42
lines changed

‎cli/server.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,7 @@ func createOIDCConfig(ctx context.Context, vals *codersdk.DeploymentValues) (*co
169169
EmailDomain: vals.OIDC.EmailDomain,
170170
AllowSignups: vals.OIDC.AllowSignups.Value(),
171171
UsernameField: vals.OIDC.UsernameField.String(),
172+
NameField: vals.OIDC.NameField.String(),
172173
EmailField: vals.OIDC.EmailField.String(),
173174
AuthURLParams: vals.OIDC.AuthURLParams.Value,
174175
IgnoreUserInfo: vals.OIDC.IgnoreUserInfo.Value(),

‎cli/testdata/coder_server_--help.golden

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,9 @@ OIDC OPTIONS:
407407
--oidc-issuer-url string, $CODER_OIDC_ISSUER_URL
408408
Issuer URL to use for Login with OIDC.
409409

410+
--oidc-name-field string, $CODER_OIDC_NAME_FIELD (default: name)
411+
OIDC claim field to use as the name.
412+
410413
--oidc-group-regex-filter regexp, $CODER_OIDC_GROUP_REGEX_FILTER (default: .*)
411414
If provided any group name not matching the regex is ignored. This
412415
allows for filtering out groups that are not needed. This filter is

‎cli/testdata/server-config.yaml.golden

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,9 @@ oidc:
306306
# OIDC claim field to use as the username.
307307
# (default: preferred_username, type: string)
308308
usernameField: preferred_username
309+
# OIDC claim field to use as the name.
310+
# (default: name, type: string)
311+
nameField: name
309312
# OIDC claim field to use as the email.
310313
# (default: email, type: string)
311314
emailField: email

‎coderd/apidoc/docs.go

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/httpapi/name.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,3 +91,14 @@ func UserRealNameValid(str string) error {
9191
}
9292
return nil
9393
}
94+
95+
// NormalizeUserRealName normalizes a user name such that it will pass
96+
// validation by UserRealNameValid. This is done to avoid blocking
97+
// little Bobby Whitespace from using Coder.
98+
func NormalizeRealUsername(str string) string {
99+
s := strings.TrimSpace(str)
100+
if len(s) > 128 {
101+
s = s[:128]
102+
}
103+
return s
104+
}

‎coderd/httpapi/name_test.go

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
package httpapi_test
22

33
import (
4+
"strings"
45
"testing"
56

67
"github.com/moby/moby/pkg/namesgenerator"
8+
"github.com/stretchr/testify/assert"
79
"github.com/stretchr/testify/require"
810

911
"github.com/coder/coder/v2/coderd/httpapi"
@@ -217,6 +219,10 @@ func TestUserRealNameValid(t *testing.T) {
217219
Name string
218220
Valid bool
219221
}{
222+
{"", true},
223+
{" a", false},
224+
{"a ", false},
225+
{" a ", false},
220226
{"1", true},
221227
{"A", true},
222228
{"A1", true},
@@ -229,17 +235,22 @@ func TestUserRealNameValid(t *testing.T) {
229235
{"Małgorzata Kalinowska-Iszkowska", true},
230236
{"成龍", true},
231237
{". .", true},
232-
233238
{"Lord Voldemort ", false},
234239
{" Bellatrix Lestrange", false},
235240
{" ", false},
241+
{strings.Repeat("a", 128), true},
242+
{strings.Repeat("a", 129), false},
236243
}
237244
for _, testCase := range testCases {
238245
testCase := testCase
239246
t.Run(testCase.Name, func(t *testing.T) {
240247
t.Parallel()
241-
valid := httpapi.UserRealNameValid(testCase.Name)
242-
require.Equal(t, testCase.Valid, valid == nil)
248+
err := httpapi.UserRealNameValid(testCase.Name)
249+
norm := httpapi.NormalizeRealUsername(testCase.Name)
250+
normErr := httpapi.UserRealNameValid(norm)
251+
assert.NoError(t, normErr)
252+
assert.Equal(t, testCase.Valid, err == nil)
253+
assert.Equal(t, testCase.Valid, norm == testCase.Name, "invalid name should be different after normalization")
243254
})
244255
}
245256
}

‎coderd/userauth.go

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -607,6 +607,9 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
607607
return
608608
}
609609

610+
ghName := ghUser.GetName()
611+
normName := httpapi.NormalizeRealUsername(ghName)
612+
610613
// If we have a nil GitHub ID, that is a big problem. That would mean we link
611614
// this user and all other users with this bug to the same uuid.
612615
// 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) {
652655
Email: verifiedEmail.GetEmail(),
653656
Username: ghUser.GetLogin(),
654657
AvatarURL: ghUser.GetAvatarURL(),
658+
Name: normName,
655659
DebugContext: OauthDebugContext{},
656660
}).SetInitAuditRequest(func(params *audit.RequestParams) (*audit.Request[database.User], func()) {
657661
return audit.InitRequest[database.User](rw, params)
@@ -701,6 +705,9 @@ type OIDCConfig struct {
701705
// EmailField selects the claim field to be used as the created user's
702706
// email.
703707
EmailField string
708+
// NameField selects the claim field to be used as the created user's
709+
// full / given name.
710+
NameField string
704711
// AuthURLParams are additional parameters to be passed to the OIDC provider
705712
// when requesting an access token.
706713
AuthURLParams map[string]string
@@ -952,13 +959,22 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
952959
}
953960
}
954961

962+
// The 'name' is an optional property in Coder. If not specified,
963+
// it will be left blank.
964+
var name string
965+
nameRaw, ok := mergedClaims[api.OIDCConfig.NameField]
966+
if ok {
967+
name, _ = nameRaw.(string)
968+
name = httpapi.NormalizeRealUsername(name)
969+
}
970+
955971
var picture string
956972
pictureRaw, ok := mergedClaims["picture"]
957973
if ok {
958974
picture, _ = pictureRaw.(string)
959975
}
960976

961-
ctx = slog.With(ctx, slog.F("email", email), slog.F("username", username))
977+
ctx = slog.With(ctx, slog.F("email", email), slog.F("username", username), slog.F("name", name))
962978
usingGroups, groups, groupErr := api.oidcGroups(ctx, mergedClaims)
963979
if groupErr != nil {
964980
groupErr.Write(rw, r)
@@ -996,6 +1012,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
9961012
AllowSignups: api.OIDCConfig.AllowSignups,
9971013
Email: email,
9981014
Username: username,
1015+
Name: name,
9991016
AvatarURL: picture,
10001017
UsingRoles: api.OIDCConfig.RoleSyncEnabled(),
10011018
Roles: roles,
@@ -1222,6 +1239,7 @@ type oauthLoginParams struct {
12221239
AllowSignups bool
12231240
Email string
12241241
Username string
1242+
Name string
12251243
AvatarURL string
12261244
// Is UsingGroups is true, then the user will be assigned
12271245
// to the Groups provided.
@@ -1544,6 +1562,10 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C
15441562
user.AvatarURL = params.AvatarURL
15451563
needsUpdate = true
15461564
}
1565+
if user.Name != params.Name {
1566+
user.Name = params.Name
1567+
needsUpdate = true
1568+
}
15471569

15481570
// If the upstream email or username has changed we should mirror
15491571
// that in Coder. Many enterprises use a user's email/username as

0 commit comments

Comments
 (0)