From a50654b46895d9aefdf26ebeac105dcef4f41c24 Mon Sep 17 00:00:00 2001
From: Mariano Cano <mariano.cano@gmail.com>
Date: Thu, 23 Sep 2021 15:49:28 -0700
Subject: [PATCH 1/3] Check for admins in both emails and groups.
---
authority/provisioner/oidc.go | 71 ++++++++++++++----------------
authority/provisioner/oidc_test.go | 36 +++++++++++++++
2 files changed, 68 insertions(+), 39 deletions(-)
diff --git a/authority/provisioner/oidc.go b/authority/provisioner/oidc.go
index b6bca872..3786f54b 100644
--- a/authority/provisioner/oidc.go
+++ b/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)
- if !isAdmin && len(claims.Groups) > 0 {
- isAdmin = o.IsAdminGroup(claims.Groups)
- }
+ isAdmin := claims.IsAdmin(o.Admins)
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) {
- return errs.Unauthorized("oidc.AuthorizeSSHRevoke; cannot revoke with non-admin oidc token")
+ if claims.IsAdmin(o.Admins) {
+ return nil
}
- return nil
+
+ return errs.Unauthorized("oidc.AuthorizeSSHRevoke; cannot revoke with non-admin oidc token")
}
func getAndDecode(uri string, v interface{}) error {
diff --git a/authority/provisioner/oidc_test.go b/authority/provisioner/oidc_test.go
index 48f879a8..532bd2e0 100644
--- a/authority/provisioner/oidc_test.go
+++ b/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)
+ }
+ })
+ }
+}
From 9eb757797ec4e514a80199d666dd66e9e1f29ff2 Mon Sep 17 00:00:00 2001
From: Mariano Cano <mariano.cano@gmail.com>
Date: Fri, 24 Sep 2021 13:50:10 -0700
Subject: [PATCH 2/3] Add line to changelog.
---
CHANGELOG.md | 1 +
1 file changed, 1 insertion(+)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index a6e67397..0ec765d3 100644
--- a/CHANGELOG.md
+++ b/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.
+- Support for group admins in OIDC provisioners for X509 certificates.
### Changed
- Using go 1.17 for binaries
### Deprecated
From 963eaf8882cee1b3d881df7f969b220a8b6b4a83 Mon Sep 17 00:00:00 2001
From: Mariano Cano <mariano.cano@gmail.com>
Date: Fri, 24 Sep 2021 13:50:47 -0700
Subject: [PATCH 3/3] Fix line in changelog
---
CHANGELOG.md | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0ec765d3..60b36b9b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,7 +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.
-- Support for group admins in OIDC provisioners for X509 certificates.
+- Global support for group admins in the OIDC provisioner.
### Changed
- Using go 1.17 for binaries
### Deprecated