diff --git a/cli/usercreate.go b/cli/usercreate.go index 78bb396916926..d1ae2baf85a43 100644 --- a/cli/usercreate.go +++ b/cli/usercreate.go @@ -11,7 +11,6 @@ import ( "github.com/coder/pretty" "github.com/coder/coder/v2/cli/cliui" - "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/cryptorand" "github.com/coder/serpent" @@ -72,7 +71,7 @@ func (r *RootCmd) userCreate() *serpent.Command { if err != nil { return err } - name = httpapi.NormalizeRealUsername(rawName) + name = codersdk.NormalizeRealUsername(rawName) if !strings.EqualFold(rawName, name) { cliui.Warnf(inv.Stderr, "Normalized name to %q", name) } diff --git a/coderd/externalauth/externalauth.go b/coderd/externalauth/externalauth.go index d93120fc5da14..2ad2761e80b46 100644 --- a/coderd/externalauth/externalauth.go +++ b/coderd/externalauth/externalauth.go @@ -23,7 +23,6 @@ import ( "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbtime" - "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/coderd/promoauth" "github.com/coder/coder/v2/codersdk" "github.com/coder/retry" @@ -486,7 +485,7 @@ func ConvertConfig(instrument *promoauth.Factory, entries []codersdk.ExternalAut // apply their client secret and ID, and have the UI appear nicely. applyDefaultsToConfig(&entry) - valid := httpapi.NameValid(entry.ID) + valid := codersdk.NameValid(entry.ID) if valid != nil { return nil, xerrors.Errorf("external auth provider %q doesn't have a valid id: %w", entry.ID, valid) } diff --git a/coderd/httpapi/httpapi.go b/coderd/httpapi/httpapi.go index a83f4b6d19cfc..1e6f8c7323d93 100644 --- a/coderd/httpapi/httpapi.go +++ b/coderd/httpapi/httpapi.go @@ -43,7 +43,7 @@ func init() { if !ok { return false } - valid := NameValid(str) + valid := codersdk.NameValid(str) return valid == nil } for _, tag := range []string{"username", "organization_name", "template_name", "workspace_name", "oauth2_app_name"} { @@ -59,7 +59,7 @@ func init() { if !ok { return false } - valid := DisplayNameValid(str) + valid := codersdk.DisplayNameValid(str) return valid == nil } for _, displayNameTag := range []string{"organization_display_name", "template_display_name", "group_display_name"} { @@ -75,7 +75,7 @@ func init() { if !ok { return false } - valid := TemplateVersionNameValid(str) + valid := codersdk.TemplateVersionNameValid(str) return valid == nil } err := Validate.RegisterValidation("template_version_name", templateVersionNameValidator) @@ -89,7 +89,7 @@ func init() { if !ok { return false } - valid := UserRealNameValid(str) + valid := codersdk.UserRealNameValid(str) return valid == nil } err = Validate.RegisterValidation("user_real_name", userRealNameValidator) @@ -103,7 +103,7 @@ func init() { if !ok { return false } - valid := GroupNameValid(str) + valid := codersdk.GroupNameValid(str) return valid == nil } err = Validate.RegisterValidation("group_name", groupNameValidator) diff --git a/coderd/httpapi/name.go b/coderd/httpapi/name.go index 98bbf50f46861..a40542fe2dd21 100644 --- a/coderd/httpapi/name.go +++ b/coderd/httpapi/name.go @@ -38,7 +38,7 @@ func UsernameFrom(str string) string { } // NameValid returns whether the input string is a valid name. -// It is a generic validator for any name (user, workspace, template, role name, etc.). +// It is a generic validator for any name that doesn't have it's own validator. func NameValid(str string) error { if len(str) > 32 { return xerrors.New("must be <= 32 characters") diff --git a/coderd/userauth.go b/coderd/userauth.go index 9c72af249cf3a..bb149d9d07379 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -602,7 +602,7 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) { } ghName := ghUser.GetName() - normName := httpapi.NormalizeRealUsername(ghName) + normName := codersdk.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. @@ -951,7 +951,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { // 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. - usernameValid := httpapi.NameValid(username) + usernameValid := codersdk.NameValid(username) if usernameValid != nil { // If no username is provided, we can default to use the email address. // This will be converted in the from function below, so it's safe @@ -959,7 +959,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { if username == "" { username = email } - username = httpapi.UsernameFrom(username) + username = codersdk.UsernameFrom(username) } if len(api.OIDCConfig.EmailDomain) > 0 { @@ -994,7 +994,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { nameRaw, ok := mergedClaims[api.OIDCConfig.NameField] if ok { name, _ = nameRaw.(string) - name = httpapi.NormalizeRealUsername(name) + name = codersdk.NormalizeRealUsername(name) } var picture string @@ -1389,7 +1389,7 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C for i := 0; i < 10; i++ { alternate := fmt.Sprintf("%s-%s", original, namesgenerator.GetRandomName(1)) - params.Username = httpapi.UsernameFrom(alternate) + params.Username = codersdk.UsernameFrom(alternate) //nolint:gocritic _, err := tx.GetUserByEmailOrUsername(dbauthz.AsSystemRestricted(ctx), database.GetUserByEmailOrUsernameParams{ diff --git a/coderd/users.go b/coderd/users.go index 38dcc1bd82f6e..aed325c6fe735 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -1287,7 +1287,7 @@ type CreateUserRequest struct { func (api *API) CreateUser(ctx context.Context, store database.Store, req CreateUserRequest) (database.User, error) { // Ensure the username is valid. It's the caller's responsibility to ensure // the username is valid and unique. - if usernameValid := httpapi.NameValid(req.Username); usernameValid != nil { + if usernameValid := codersdk.NameValid(req.Username); usernameValid != nil { return database.User{}, xerrors.Errorf("invalid username %q: %w", req.Username, usernameValid) } @@ -1299,7 +1299,7 @@ func (api *API) CreateUser(ctx context.Context, store database.Store, req Create ID: uuid.New(), Email: req.Email, Username: req.Username, - Name: httpapi.NormalizeRealUsername(req.Name), + Name: codersdk.NormalizeRealUsername(req.Name), CreatedAt: dbtime.Now(), UpdatedAt: dbtime.Now(), HashedPassword: []byte{}, diff --git a/codersdk/name.go b/codersdk/name.go new file mode 100644 index 0000000000000..064c2175bcb49 --- /dev/null +++ b/codersdk/name.go @@ -0,0 +1,125 @@ +package codersdk + +import ( + "regexp" + "strings" + + "github.com/moby/moby/pkg/namesgenerator" + "golang.org/x/xerrors" +) + +var ( + UsernameValidRegex = regexp.MustCompile("^[a-zA-Z0-9]+(?:-[a-zA-Z0-9]+)*$") + usernameReplace = regexp.MustCompile("[^a-zA-Z0-9-]*") + + templateVersionName = regexp.MustCompile(`^[a-zA-Z0-9]+(?:[_.-]{1}[a-zA-Z0-9]+)*$`) + templateDisplayName = regexp.MustCompile(`^[^\s](.*[^\s])?$`) +) + +// UsernameFrom returns a best-effort username from the provided string. +// +// It first attempts to validate the incoming string, which will +// be returned if it is valid. It then will attempt to extract +// the username from an email address. If no success happens during +// these steps, a random username will be returned. +func UsernameFrom(str string) string { + if valid := NameValid(str); valid == nil { + return str + } + emailAt := strings.LastIndex(str, "@") + if emailAt >= 0 { + str = str[:emailAt] + } + str = usernameReplace.ReplaceAllString(str, "") + if valid := NameValid(str); valid == nil { + return str + } + return strings.ReplaceAll(namesgenerator.GetRandomName(1), "_", "-") +} + +// NameValid returns whether the input string is a valid name. +// It is a generic validator for any name (user, workspace, template, role name, etc.). +func NameValid(str string) error { + if len(str) > 32 { + return xerrors.New("must be <= 32 characters") + } + if len(str) < 1 { + return xerrors.New("must be >= 1 character") + } + // Avoid conflicts with routes like /templates/new and /groups/create. + if str == "new" || str == "create" { + return xerrors.Errorf("cannot use %q as a name", str) + } + matched := UsernameValidRegex.MatchString(str) + if !matched { + return xerrors.New("must be alphanumeric with hyphens") + } + return nil +} + +// TemplateVersionNameValid returns whether the input string is a valid template version name. +func TemplateVersionNameValid(str string) error { + if len(str) > 64 { + return xerrors.New("must be <= 64 characters") + } + matched := templateVersionName.MatchString(str) + if !matched { + return xerrors.New("must be alphanumeric with underscores and dots") + } + return nil +} + +// DisplayNameValid returns whether the input string is a valid template display name. +func DisplayNameValid(str string) error { + if len(str) == 0 { + return nil // empty display_name is correct + } + if len(str) > 64 { + return xerrors.New("must be <= 64 characters") + } + matched := templateDisplayName.MatchString(str) + if !matched { + return xerrors.New("must be alphanumeric with spaces") + } + return nil +} + +// UserRealNameValid returns whether the input string is a valid real user name. +func UserRealNameValid(str string) error { + if len(str) > 128 { + return xerrors.New("must be <= 128 characters") + } + + if strings.TrimSpace(str) != str { + return xerrors.New("must not have leading or trailing whitespace") + } + return nil +} + +// GroupNameValid returns whether the input string is a valid group name. +func GroupNameValid(str string) error { + // 36 is to support using UUIDs as the group name. + if len(str) > 36 { + return xerrors.New("must be <= 36 characters") + } + // Avoid conflicts with routes like /groups/new and /groups/create. + if str == "new" || str == "create" { + return xerrors.Errorf("cannot use %q as a name", str) + } + matched := UsernameValidRegex.MatchString(str) + if !matched { + return xerrors.New("must be alphanumeric with hyphens") + } + 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/codersdk/name_test.go similarity index 91% rename from coderd/httpapi/name_test.go rename to codersdk/name_test.go index 4edd816af1671..11ce797f78023 100644 --- a/coderd/httpapi/name_test.go +++ b/codersdk/name_test.go @@ -1,4 +1,4 @@ -package httpapi_test +package codersdk_test import ( "strings" @@ -7,7 +7,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/coder/coder/v2/coderd/httpapi" + "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/testutil" ) @@ -62,7 +62,7 @@ func TestUsernameValid(t *testing.T) { testCase := testCase t.Run(testCase.Username, func(t *testing.T) { t.Parallel() - valid := httpapi.NameValid(testCase.Username) + valid := codersdk.NameValid(testCase.Username) require.Equal(t, testCase.Valid, valid == nil) }) } @@ -117,7 +117,7 @@ func TestTemplateDisplayNameValid(t *testing.T) { testCase := testCase t.Run(testCase.Name, func(t *testing.T) { t.Parallel() - valid := httpapi.DisplayNameValid(testCase.Name) + valid := codersdk.DisplayNameValid(testCase.Name) require.Equal(t, testCase.Valid, valid == nil) }) } @@ -158,7 +158,7 @@ func TestTemplateVersionNameValid(t *testing.T) { testCase := testCase t.Run(testCase.Name, func(t *testing.T) { t.Parallel() - valid := httpapi.TemplateVersionNameValid(testCase.Name) + valid := codersdk.TemplateVersionNameValid(testCase.Name) require.Equal(t, testCase.Valid, valid == nil) }) } @@ -169,7 +169,7 @@ func TestGeneratedTemplateVersionNameValid(t *testing.T) { for i := 0; i < 1000; i++ { name := testutil.GetRandomName(t) - err := httpapi.TemplateVersionNameValid(name) + err := codersdk.TemplateVersionNameValid(name) require.NoError(t, err, "invalid template version name: %s", name) } } @@ -199,9 +199,9 @@ func TestFrom(t *testing.T) { testCase := testCase t.Run(testCase.From, func(t *testing.T) { t.Parallel() - converted := httpapi.UsernameFrom(testCase.From) + converted := codersdk.UsernameFrom(testCase.From) t.Log(converted) - valid := httpapi.NameValid(converted) + valid := codersdk.NameValid(converted) require.True(t, valid == nil) if testCase.Match == "" { require.NotEqual(t, testCase.From, converted) @@ -245,9 +245,9 @@ func TestUserRealNameValid(t *testing.T) { testCase := testCase t.Run(testCase.Name, func(t *testing.T) { t.Parallel() - err := httpapi.UserRealNameValid(testCase.Name) - norm := httpapi.NormalizeRealUsername(testCase.Name) - normErr := httpapi.UserRealNameValid(norm) + err := codersdk.UserRealNameValid(testCase.Name) + norm := codersdk.NormalizeRealUsername(testCase.Name) + normErr := codersdk.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/enterprise/coderd/roles.go b/enterprise/coderd/roles.go index 50d57e73cae80..227be3e4ce39e 100644 --- a/enterprise/coderd/roles.go +++ b/enterprise/coderd/roles.go @@ -266,7 +266,7 @@ func validOrganizationRoleRequest(ctx context.Context, req codersdk.CustomRoleRe return false } - if err := httpapi.NameValid(req.Name); err != nil { + if err := codersdk.NameValid(req.Name); err != nil { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ Message: "Invalid role name", Detail: err.Error(), diff --git a/enterprise/coderd/scim.go b/enterprise/coderd/scim.go index 0e777111819b9..b6733bbd4c052 100644 --- a/enterprise/coderd/scim.go +++ b/enterprise/coderd/scim.go @@ -206,7 +206,7 @@ func (api *API) scimPostUser(rw http.ResponseWriter, r *http.Request) { // 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. - usernameValid := httpapi.NameValid(sUser.UserName) + usernameValid := codersdk.NameValid(sUser.UserName) if usernameValid != nil { // If no username is provided, we can default to use the email address. // This will be converted in the from function below, so it's safe @@ -214,7 +214,7 @@ func (api *API) scimPostUser(rw http.ResponseWriter, r *http.Request) { if sUser.UserName == "" { sUser.UserName = email } - sUser.UserName = httpapi.UsernameFrom(sUser.UserName) + sUser.UserName = codersdk.UsernameFrom(sUser.UserName) } // TODO: This is a temporary solution that does not support multi-org