Skip to content

Commit

Permalink
[YUNIKORN-2656] Validate user name (#889)
Browse files Browse the repository at this point in the history
Closes: #889

Signed-off-by: Manikandan R <manirajv06@gmail.com>
  • Loading branch information
manirajv06 committed Jun 12, 2024
1 parent 070a678 commit d515fe0
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 2 deletions.
2 changes: 1 addition & 1 deletion pkg/common/configs/configvalidator.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ var QueueNameRegExp = regexp.MustCompile(`^[a-zA-Z0-9_-]{1,64}$`)

// User and group name check: systems allow different things POSIX is the base but we need to be lenient and allow more.
// allow upper and lower case, add the @ and . (dot) and officially no length.
var UserRegExp = regexp.MustCompile(`^[_a-zA-Z][a-zA-Z0-9:_.@-]*[$]?$`)
var UserRegExp = regexp.MustCompile(`^[_a-zA-Z][a-zA-Z0-9:#/_.@-]*[$]?$`)

// Groups should have a slightly more restrictive regexp (no @ . or $ at the end)
var GroupRegExp = regexp.MustCompile(`^[_a-zA-Z][a-zA-Z0-9:_.-]*$`)
Expand Down
1 change: 0 additions & 1 deletion pkg/common/configs/configvalidator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -742,7 +742,6 @@ func TestUserName(t *testing.T) {
rejectedUserNames := []string{
"username rejected",
"",
"rejected#",
"rejected!name",
"!rejected",
" rejected ",
Expand Down
6 changes: 6 additions & 0 deletions pkg/common/security/usergroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"go.uber.org/zap"

"github.com/apache/yunikorn-core/pkg/common"
"github.com/apache/yunikorn-core/pkg/common/configs"
"github.com/apache/yunikorn-core/pkg/locking"
"github.com/apache/yunikorn-core/pkg/log"
"github.com/apache/yunikorn-scheduler-interface/lib/go/si"
Expand Down Expand Up @@ -153,6 +154,11 @@ func (c *UserGroupCache) ConvertUGI(ugi *si.UserGroupInformation, force bool) (U
return ug, err
}
}

if !configs.UserRegExp.MatchString(ugi.User) {
return UserGroup{}, fmt.Errorf("invalid username, it contains invalid characters")
}

// If groups are already present we should just convert
newUG := UserGroup{User: ugi.User}
newUG.Groups = append(newUG.Groups, ugi.Groups...)
Expand Down
14 changes: 14 additions & 0 deletions pkg/common/security/usergroup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,4 +237,18 @@ func TestConvertUGI(t *testing.T) {
if ug.Groups[0] != group {
t.Errorf("groups not initialised correctly on convert: expected '%s' got '%s'", group, ug.Groups[0])
}
// try valid username with groups
ugi.User = "validuserABCD1234@://#"
ugi.Groups = []string{group}
ug, err = testCache.ConvertUGI(ugi, false)
if err != nil {
t.Errorf("valid username with groups, convert should not have failed: %v", err)
}
// try invalid username with groups
ugi.User = "invaliduser><+"
ugi.Groups = []string{group}
ug, err = testCache.ConvertUGI(ugi, false)
if err == nil {
t.Errorf("invalid username, convert should have failed: %v", err)
}
}

0 comments on commit d515fe0

Please sign in to comment.