auth: Allow '.' in NormalizeUsername (#4690)

* auth: Allow '.' in NormalizeUsername

Fixes #4674

* Update CHANGELOG

* Update CHANGELOG.md

Co-Authored-By: Quinn Slack <qslack@qslack.com>

* fixup! Fix TestMiddleware

CHANGELOG.md

@@ -18,6 +18,10 @@ All notable changes to Sourcegraph are documented in this file.
 
 ### Changed
 
+- Updating or creating an external service will no longer block until the service is synced.
+- The GraphQL fields `Repository.createdAt` and `Repository.updatedAt` are deprecated and will be removed in 3.8. Now `createdAt` is always the current time and updatedAt is always null.
+- Usernames can now contain the `.` character (#4690).
+
 ### Fixed
 
 ### Removed

cmd/frontend/auth/auth.go

@@ -58,28 +58,34 @@ func composeMiddleware(middlewares ...*Middleware) *Middleware {
 }
 
 // NormalizeUsername normalizes a proposed username into a format that meets Sourcegraph's
-// username formatting rules (consistent with
+// username formatting rules (based on, but not identical to
 // https://help.github.com/enterprise/2.11/admin/guides/user-management/using-ldap/#username-considerations-with-ldap):
 //
 // - Any portion of the username after a '@' character is removed
-// - Any characters not in `[a-zA-Z0-9-]` are replaced with `-`
-// - Usernames with consecutive '-' characters are not allowed
-// - Usernames that start or end with '-' are not allowed
+// - Any characters not in `[a-zA-Z0-9-.]` are replaced with `-`
+// - Usernames with consecutive '-' or '.' characters are not allowed
+// - Usernames that start or end with '-' or '.' are not allowed
 //
 // Usernames that could not be converted return an error.
 func NormalizeUsername(name string) (string, error) {
 	origName := name
 	if i := strings.Index(name, "@"); i != -1 && i == strings.LastIndex(name, "@") {
 		name = name[:i]
 	}
+
 	name = disallowedCharacter.ReplaceAllString(name, "-")
-	if strings.HasPrefix(name, "-") || strings.HasSuffix(name, "-") || strings.Contains(name, "--") {
+	if disallowedSymbols.MatchString(name) {
 		return "", fmt.Errorf("username %q could not be normalized to acceptable format", origName)
 	}
+
 	if err := suspiciousnames.CheckNameAllowedForUserOrOrganization(name); err != nil {
 		return "", err
 	}
+
 	return name, nil
 }
 
-var disallowedCharacter = regexp.MustCompile(`[^a-zA-Z0-9\-]`)
+var (
+	disallowedSymbols   = regexp.MustCompile(`(^[\-\.])|([\-\.]$)|([\-\.]{2,})`)
+	disallowedCharacter = regexp.MustCompile(`[^a-zA-Z0-9\-\.]`)
+)

cmd/frontend/auth/auth_test.go

@@ -10,15 +10,20 @@ func TestNormalizeUsername(t *testing.T) {
 	}{
 		{in: "username", out: "username"},
 		{in: "john@gmail.com", out: "john"},
-		{in: "john.appleseed@gmail.com", out: "john-appleseed"},
+		{in: "john.appleseed@gmail.com", out: "john.appleseed"},
 		{in: "john+test@gmail.com", out: "john-test"},
 		{in: "this@is@not-an-email", out: "this-is-not-an-email"},
+		{in: "user.na$e", out: "user.na-e"},
 		{in: "2039f0923f0", out: "2039f0923f0"},
 		{in: "john(test)@gmail.com", hasErr: true},
 		{in: "bob!", hasErr: true},
 		{in: "bob.!bob", hasErr: true},
 		{in: "bob@@bob", hasErr: true},
 		{in: "username-", hasErr: true},
+		{in: "username.", hasErr: true},
+		{in: ".username", hasErr: true},
+		{in: "user..name", hasErr: true},
+		{in: "user.-name", hasErr: true},
 	}
 
 	for _, tc := range testCases {

doc/admin/auth/index.md

@@ -313,12 +313,12 @@ Some proxies add a prefix to the username header value. For example, Google IAP
 Usernames on Sourcegraph are normalized according to the following rules.
 
 - Any portion of the username after a '@' character is removed
-- Any characters not in `[a-zA-Z0-9-]` are replaced with `-`
-- Usernames with consecutive '-' characters are not allowed
-- Usernames that start or end with '-' are not allowed
+- Any characters not in `[a-zA-Z0-9-.]` are replaced with `-`
+- Usernames with consecutive '-' or '.' characters are not allowed
+- Usernames that start or end with '-' or '.' are not allowed
 
 Usernames from authentication providers are normalized before being used in Sourcegraph. Usernames chosen by users are rejected if they do not meet these criteria.
 
-For example, a user whose external username (according the authentication provider) is `alice.smith@example.com` would have the Sourcegraph username `alice-smith`.
+For example, a user whose external username (according the authentication provider) is `alice_smith@example.com` would have the Sourcegraph username `alice-smith`.
 
 If multiple accounts normalize into the same username, only the first user account is created. Other users won't be able to sign in. This is a rare occurrence; contact support if this is a blocker.

enterprise/cmd/frontend/auth/httpheader/middleware_test.go

@@ -91,15 +91,15 @@ func TestMiddleware(t *testing.T) {
 	t.Run("sent, with un-normalized username", func(t *testing.T) {
 		rr := httptest.NewRecorder()
 		req, _ := http.NewRequest("GET", "/", nil)
-		req.Header.Set(headerName, "alice.zhao")
+		req.Header.Set(headerName, "alice_zhao")
 		const wantNormalizedUsername = "alice-zhao"
 		var calledMock bool
 		auth.MockGetAndSaveUser = func(ctx context.Context, op auth.GetAndSaveUserOp) (userID int32, safeErrMsg string, err error) {
 			calledMock = true
 			if op.UserProps.Username != wantNormalizedUsername {
 				t.Errorf("got %q, want %q", op.UserProps.Username, wantNormalizedUsername)
 			}
-			if op.ExternalAccount.ServiceType == "http-header" && op.ExternalAccount.ServiceID == "" && op.ExternalAccount.ClientID == "" && op.ExternalAccount.AccountID == "alice.zhao" {
+			if op.ExternalAccount.ServiceType == "http-header" && op.ExternalAccount.ServiceID == "" && op.ExternalAccount.ClientID == "" && op.ExternalAccount.AccountID == "alice_zhao" {
 				return 1, "", nil
 			}
 			return 0, "safeErr", fmt.Errorf("account %v not found in mock", op.ExternalAccount)