Merge pull request #711 from smallstep/oidc-admin-group

Check for admins in both emails and groups.

CHANGELOG.md

@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - go 1.17 to github action test matrix
 - Support for CloudKMS RSA-PSS signers without using templates.
 - Add flags to support individual passwords for the intermediate and SSH keys.
+- Global support for group admins in the OIDC provisioner.
 ### Changed
 - Using go 1.17 for binaries
 ### Deprecated

authority/provisioner/oidc.go

@@ -49,6 +49,29 @@ type openIDPayload struct {
 	Groups          []string `json:"groups"`
 }
 
+func (o *openIDPayload) IsAdmin(admins []string) bool {
+	if o.Email != "" {
+		email := sanitizeEmail(o.Email)
+		for _, e := range admins {
+			if email == sanitizeEmail(e) {
+				return true
+			}
+		}
+	}
+
+	// The groups and emails can be in the same array for now, but consider
+	// making a specialized option later.
+	for _, name := range o.Groups {
+		for _, admin := range admins {
+			if name == admin {
+				return true
+			}
+		}
+	}
+
+	return false
+}
+
 // OIDC represents an OAuth 2.0 OpenID Connect provider.
 //
 // ClientSecret is mandatory, but it can be an empty string.
@@ -73,35 +96,6 @@ type OIDC struct {
 	getIdentityFunc       GetIdentityFunc
 }
 
-// IsAdmin returns true if the given email is in the Admins allowlist, false
-// otherwise.
-func (o *OIDC) IsAdmin(email string) bool {
-	if email != "" {
-		email = sanitizeEmail(email)
-		for _, e := range o.Admins {
-			if email == sanitizeEmail(e) {
-				return true
-			}
-		}
-	}
-	return false
-}
-
-// IsAdminGroup returns true if the one group in the given list is in the Admins
-// allowlist, false otherwise.
-func (o *OIDC) IsAdminGroup(groups []string) bool {
-	for _, g := range groups {
-		// The groups and emails can be in the same array for now, but consider
-		// making a specialized option later.
-		for _, gadmin := range o.Admins {
-			if g == gadmin {
-				return true
-			}
-		}
-	}
-	return false
-}
-
 func sanitizeEmail(email string) string {
 	if i := strings.LastIndex(email, "@"); i >= 0 {
 		email = email[:i] + strings.ToLower(email[i:])
@@ -234,7 +228,7 @@ func (o *OIDC) ValidatePayload(p openIDPayload) error {
 	}
 
 	// Validate domains (case-insensitive)
-	if p.Email != "" && len(o.Domains) > 0 && !o.IsAdmin(p.Email) {
+	if p.Email != "" && len(o.Domains) > 0 && !p.IsAdmin(o.Admins) {
 		email := sanitizeEmail(p.Email)
 		var found bool
 		for _, d := range o.Domains {
@@ -313,9 +307,10 @@ func (o *OIDC) AuthorizeRevoke(ctx context.Context, token string) error {
 	}
 
 	// Only admins can revoke certificates.
-	if o.IsAdmin(claims.Email) {
+	if claims.IsAdmin(o.Admins) {
 		return nil
 	}
+
 	return errs.Unauthorized("oidc.AuthorizeRevoke; cannot revoke with non-admin oidc token")
 }
 
@@ -351,7 +346,7 @@ func (o *OIDC) AuthorizeSign(ctx context.Context, token string) ([]SignOption, e
 	// Use the default template unless no-templates are configured and email is
 	// an admin, in that case we will use the CR template.
 	defaultTemplate := x509util.DefaultLeafTemplate
-	if !o.Options.GetX509Options().HasTemplate() && o.IsAdmin(claims.Email) {
+	if !o.Options.GetX509Options().HasTemplate() && claims.IsAdmin(o.Admins) {
 		defaultTemplate = x509util.DefaultAdminLeafTemplate
 	}
 
@@ -420,10 +415,7 @@ func (o *OIDC) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOption
 
 	// Use the default template unless no-templates are configured and email is
 	// an admin, in that case we will use the parameters in the request.
-	isAdmin := o.IsAdmin(claims.Email)
+	isAdmin := claims.IsAdmin(o.Admins)
-	if !isAdmin && len(claims.Groups) > 0 {
-		isAdmin = o.IsAdminGroup(claims.Groups)
-	}
 	defaultTemplate := sshutil.DefaultTemplate
 	if isAdmin && !o.Options.GetSSHOptions().HasTemplate() {
 		defaultTemplate = sshutil.DefaultAdminTemplate
@@ -471,10 +463,11 @@ func (o *OIDC) AuthorizeSSHRevoke(ctx context.Context, token string) error {
 	}
 
 	// Only admins can revoke certificates.
-	if !o.IsAdmin(claims.Email) {
+	if claims.IsAdmin(o.Admins) {
-		return errs.Unauthorized("oidc.AuthorizeSSHRevoke; cannot revoke with non-admin oidc token")
+		return nil
 	}
-	return nil
+
+	return errs.Unauthorized("oidc.AuthorizeSSHRevoke; cannot revoke with non-admin oidc token")
 }
 
 func getAndDecode(uri string, v interface{}) error {

authority/provisioner/oidc_test.go

@@ -698,3 +698,39 @@ func Test_sanitizeEmail(t *testing.T) {
 		})
 	}
 }
+
+func Test_openIDPayload_IsAdmin(t *testing.T) {
+	type fields struct {
+		Email  string
+		Groups []string
+	}
+	type args struct {
+		admins []string
+	}
+	tests := []struct {
+		name   string
+		fields fields
+		args   args
+		want   bool
+	}{
+		{"ok email", fields{"admin@smallstep.com", nil}, args{[]string{"admin@smallstep.com"}}, true},
+		{"ok email multiple", fields{"admin@smallstep.com", []string{"admin", "eng"}}, args{[]string{"eng@smallstep.com", "admin@smallstep.com"}}, true},
+		{"ok email sanitized", fields{"admin@Smallstep.com", nil}, args{[]string{"admin@smallStep.com"}}, true},
+		{"ok group", fields{"", []string{"admin"}}, args{[]string{"admin"}}, true},
+		{"ok group multiple", fields{"admin@smallstep.com", []string{"engineering", "admin"}}, args{[]string{"admin"}}, true},
+		{"fail missing", fields{"eng@smallstep.com", []string{"admin"}}, args{[]string{"admin@smallstep.com"}}, false},
+		{"fail email letter case", fields{"Admin@smallstep.com", []string{}}, args{[]string{"admin@smallstep.com"}}, false},
+		{"fail group letter case", fields{"", []string{"Admin"}}, args{[]string{"admin"}}, false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			o := &openIDPayload{
+				Email:  tt.fields.Email,
+				Groups: tt.fields.Groups,
+			}
+			if got := o.IsAdmin(tt.args.admins); got != tt.want {
+				t.Errorf("openIDPayload.IsAdmin() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}