graph: Allow provisioning users with legacy names (#5255)

Via configuration you can now configure to skip the validation of username and
instead decide to trust the upstream system that is adding users.

services/graph/pkg/config/config.go

@@ -92,7 +92,8 @@ type Identity struct {
 
 // API represents API configuration parameters.
 type API struct {
-	GroupMembersPatchLimit int `yaml:"group_members_patch_limit" env:"GRAPH_GROUP_MEMBERS_PATCH_LIMIT" desc:"The amount of group members allowed to be added with a single patch request."`
+	GroupMembersPatchLimit int    `yaml:"group_members_patch_limit" env:"GRAPH_GROUP_MEMBERS_PATCH_LIMIT" desc:"The amount of group members allowed to be added with a single patch request."`
+	UsernameMatch          string `yaml:"graph_username_match" env:"GRAPH_USERNAME_MATCH" desc:"Option to allow legacy usernames. Supported options are 'default' and 'none'."`
 }
 
 // Events combines the configuration options for the event bus.

services/graph/pkg/config/defaults/defaultconfig.go

@@ -36,6 +36,7 @@ func DefaultConfig() *config.Config {
 		},
 		API: config.API{
 			GroupMembersPatchLimit: 20,
+			UsernameMatch:          "default",
 		},
 		Reva: shared.DefaultRevaConfig(),
 		Spaces: config.Spaces{

services/graph/pkg/config/parser/parse.go

@@ -52,5 +52,15 @@ func Validate(cfg *config.Config) error {
 			"graph", defaults2.BaseConfigPath())
 	}
 
+	switch cfg.API.UsernameMatch {
+	case "default", "none":
+	default:
+		return fmt.Errorf("The username match validator is invalid for %s. "+
+			"Make sure your %s config contains the proper values "+
+			"(e.g. by running ocis init or setting it manually in "+
+			"the config/corresponding environment variable).",
+			"graph", defaults2.BaseConfigPath())
+	}
+
 	return nil
 }

services/graph/pkg/service/v0/educationuser.go

@@ -112,7 +112,7 @@ func (g Graph) PostEducationUser(w http.ResponseWriter, r *http.Request) {
 	}
 
 	if accountName, ok := u.GetOnPremisesSamAccountNameOk(); ok {
-		if !isValidUsername(*accountName) {
+		if !g.isValidUsername(*accountName) {
 			logger.Debug().Str("username", *accountName).Msg("could not create education user: username must be at least the local part of an email")
 			errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, fmt.Sprintf("username %s must be at least the local part of an email", *u.OnPremisesSamAccountName))
 			return

services/graph/pkg/service/v0/users.go

@@ -159,7 +159,7 @@ func (g Graph) PostUser(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	if accountName, ok := u.GetOnPremisesSamAccountNameOk(); ok {
-		if !isValidUsername(*accountName) {
+		if !g.isValidUsername(*accountName) {
 			logger.Debug().Str("username", *accountName).Msg("could not create user: username must be at least the local part of an email")
 			errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, fmt.Sprintf("username %s must be at least the local part of an email", *u.OnPremisesSamAccountName))
 			return
@@ -530,16 +530,28 @@ func (g Graph) PatchUser(w http.ResponseWriter, r *http.Request) {
 
 }
 
-// We want to allow email addresses as usernames so they show up when using them in ACLs on storages that allow integration with our glauth LDAP service
-// so we are adding a few restrictions from https://stackoverflow.com/questions/6949667/what-are-the-real-rules-for-linux-usernames-on-centos-6-and-rhel-6
-// names should not start with numbers
-var usernameRegex = regexp.MustCompile("^[a-zA-Z_][a-zA-Z0-9.!#$%&'*+/=?^_`{|}~-]*(@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*)*$")
+const (
+	usernameMatchDefault = "default"
+	usernameMatchNone    = "none"
+)
 
-func isValidUsername(e string) bool {
+var usernameRegexes = map[string]*regexp.Regexp{
+	// We want to allow email addresses as usernames so they show up when using them in ACLs on storages that allow integration with our glauth LDAP service
+	// so we are adding a few restrictions from https://stackoverflow.com/questions/6949667/what-are-the-real-rules-for-linux-usernames-on-centos-6-and-rhel-6
+	// names should not start with numbers
+	usernameMatchDefault: regexp.MustCompile("^[a-zA-Z_][a-zA-Z0-9.!#$%&'*+/=?^_`{|}~-]*(@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*)*$"),
+
+	// In some cases users will be provisioned from an existing system, which may or may not have strange usernames. Because of this we want to "trust" the
+	// upstream system and allow weird usernames, so relying on the used identity provider to complain if a username is violating its restrictions.
+	usernameMatchNone: regexp.MustCompile(".*"),
+}
+
+func (g Graph) isValidUsername(e string) bool {
 	if len(e) < 1 && len(e) > 254 {
 		return false
 	}
-	return usernameRegex.MatchString(e)
+
+	return usernameRegexes[g.config.API.UsernameMatch].MatchString(e)
 }
 
 // regex from https://www.w3.org/TR/2016/REC-html51-20161101/sec-forms.html#valid-e-mail-address

services/graph/pkg/service/v0/users_test.go

@@ -539,6 +539,63 @@ var _ = Describe("Users", func() {
 
 			Expect(rr.Code).To(Equal(http.StatusOK))
 		})
+
+		Describe("Handling usernames with spaces", func() {
+			var (
+				newSvc = func(usernameMatch string) service.Service {
+					localCfg := defaults.FullDefaultConfig()
+					localCfg.Identity.LDAP.CACert = "" // skip the startup checks, we don't use LDAP at all in this tests
+					localCfg.TokenManager.JWTSecret = "loremipsum"
+					localCfg.Commons = &shared.Commons{}
+					localCfg.GRPCClientTLS = &shared.GRPCClientTLS{}
+
+					localCfg.API.UsernameMatch = usernameMatch
+
+					_ = ogrpc.Configure(ogrpc.GetClientOptions(cfg.GRPCClientTLS)...)
+					localSvc, _ := service.NewService(
+						service.Config(localCfg),
+						service.WithGatewayClient(gatewayClient),
+						service.EventsPublisher(&eventsPublisher),
+						service.WithIdentityBackend(identityBackend),
+						service.WithRoleService(roleService),
+					)
+
+					return localSvc
+				}
+			)
+
+			BeforeEach(func() {
+				user.SetOnPremisesSamAccountName("username with spaces")
+			})
+
+			It("rejects a username with spaces if match regex is default", func() {
+				userJson, err := json.Marshal(user)
+				Expect(err).ToNot(HaveOccurred())
+
+				r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/me/users", bytes.NewBuffer(userJson))
+				r = r.WithContext(revactx.ContextSetUser(ctx, currentUser))
+
+				newSvc("default").PostUser(rr, r)
+				Expect(rr.Code).To(Equal(http.StatusBadRequest))
+			})
+
+			It("creates a user with spaces in username if regex is none", func() {
+				roleService.On("AssignRoleToUser", mock.Anything, mock.Anything).Return(&settings.AssignRoleToUserResponse{}, nil)
+				identityBackend.On("CreateUser", mock.Anything, mock.Anything).Return(func(ctx context.Context, user libregraph.User) *libregraph.User {
+					user.SetId("/users/user")
+					return &user
+				}, nil)
+				userJson, err := json.Marshal(user)
+				Expect(err).ToNot(HaveOccurred())
+
+				r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/me/users", bytes.NewBuffer(userJson))
+				r = r.WithContext(revactx.ContextSetUser(ctx, currentUser))
+				newSvc("none").PostUser(rr, r)
+
+				Expect(rr.Code).To(Equal(http.StatusOK))
+			})
+		})
+
 	})
 
 	Describe("DeleteUser", func() {