From a50654b46895d9aefdf26ebeac105dcef4f41c24 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 23 Sep 2021 15:49:28 -0700 Subject: [PATCH] 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) + } + }) + } +}