Merge pull request #561 from LecrisUT/master

Check admin privileges from group membership

authority/provisioner/oidc.go

@@ -86,6 +86,21 @@ func (o *OIDC) IsAdmin(email string) bool {
 	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:])
@@ -372,7 +387,8 @@ func (o *OIDC) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOption
 	}
 
 	// Get the identity using either the default identityFunc or one injected
-	// externally.
+	// externally. Note that the PreferredUsername might be empty.
+	// TBD: Would preferred_username present a safety issue here?
 	iden, err := o.getIdentityFunc(ctx, o, claims.Email)
 	if err != nil {
 		return nil, errs.Wrap(http.StatusInternalServerError, err, "oidc.AuthorizeSSHSign")
@@ -395,6 +411,9 @@ 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)
+	}
 	defaultTemplate := sshutil.DefaultTemplate
 	if isAdmin && !o.Options.GetSSHOptions().HasTemplate() {
 		defaultTemplate = sshutil.DefaultAdminTemplate

authority/provisioner/oidc_test.go

@@ -506,6 +506,7 @@ func TestOIDC_AuthorizeSSHSign(t *testing.T) {
 	p5.getIdentityFunc = func(ctx context.Context, p Interface, email string) (*Identity, error) {
 		return nil, errors.New("force")
 	}
+	// Additional test needed for empty usernames and duplicate email and usernames
 
 	t1, err := generateSimpleToken("the-issuer", p1.ClientID, &keys.Keys[0])
 	assert.FatalError(t, err)
@@ -514,7 +515,7 @@ func TestOIDC_AuthorizeSSHSign(t *testing.T) {
 	failGetIdentityToken, err := generateSimpleToken("the-issuer", p5.ClientID, &keys.Keys[0])
 	assert.FatalError(t, err)
 	// Admin email not in domains
-	okAdmin, err := generateToken("subject", "the-issuer", p3.ClientID, "root@example.com", []string{}, time.Now(), &keys.Keys[0])
+	okAdmin, err := generateOIDCToken("subject", "the-issuer", p3.ClientID, "root@example.com", "", time.Now(), &keys.Keys[0])
 	assert.FatalError(t, err)
 	// Empty email
 	failEmail, err := generateToken("subject", "the-issuer", p3.ClientID, "", []string{}, time.Now(), &keys.Keys[0])
@@ -576,11 +577,11 @@ func TestOIDC_AuthorizeSSHSign(t *testing.T) {
 		{"ok-options", p1, args{t1, SignSSHOptions{CertType: "user", Principals: []string{"name"}}, pub},
 			&SignSSHOptions{CertType: "user", Principals: []string{"name", "name@smallstep.com"},
 				ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(userDuration))}, http.StatusOK, false, false},
-		{"admin-user", p3, args{okAdmin, SignSSHOptions{CertType: "user", KeyID: "root@example.com", Principals: []string{"root", "root@example.com"}}, pub},
+		{"ok-admin-user", p3, args{okAdmin, SignSSHOptions{CertType: "user", KeyID: "root@example.com", Principals: []string{"root", "root@example.com"}}, pub},
 			expectedAdminOptions, http.StatusOK, false, false},
-		{"admin-host", p3, args{okAdmin, SignSSHOptions{CertType: "host", KeyID: "smallstep.com", Principals: []string{"smallstep.com"}}, pub},
+		{"ok-admin-host", p3, args{okAdmin, SignSSHOptions{CertType: "host", KeyID: "smallstep.com", Principals: []string{"smallstep.com"}}, pub},
 			expectedHostOptions, http.StatusOK, false, false},
-		{"admin-options", p3, args{okAdmin, SignSSHOptions{CertType: "user", KeyID: "name", Principals: []string{"name"}}, pub},
+		{"ok-admin-options", p3, args{okAdmin, SignSSHOptions{CertType: "user", KeyID: "name", Principals: []string{"name"}}, pub},
 			&SignSSHOptions{CertType: "user", Principals: []string{"name"},
 				ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(userDuration))}, http.StatusOK, false, false},
 		{"fail-rsa1024", p1, args{t1, SignSSHOptions{}, rsa1024.Public()}, expectedUserOptions, http.StatusOK, false, true},

authority/provisioner/provisioner.go

@@ -339,33 +339,50 @@ type Permissions struct {
 // GetIdentityFunc is a function that returns an identity.
 type GetIdentityFunc func(ctx context.Context, p Interface, email string) (*Identity, error)
 
-// DefaultIdentityFunc return a default identity depending on the provisioner type.
+// DefaultIdentityFunc return a default identity depending on the provisioner
+// type. For OIDC email is always present and the usernames might
+// contain empty strings.
 func DefaultIdentityFunc(ctx context.Context, p Interface, email string) (*Identity, error) {
 	switch k := p.(type) {
 	case *OIDC:
 		// OIDC principals would be:
-		// 1. Sanitized local.
+		// ~~1. Preferred usernames.~~ Note: Under discussion, currently disabled
-		// 2. Raw local (if different).
+		// 2. Sanitized local.
-		// 3. Email address.
+		// 3. Raw local (if different).
+		// 4. Email address.
 		name := SanitizeSSHUserPrincipal(email)
 		if !sshUserRegex.MatchString(name) {
 			return nil, errors.Errorf("invalid principal '%s' from email '%s'", name, email)
 		}
 		usernames := []string{name}
 		if i := strings.LastIndex(email, "@"); i >= 0 {
-			if local := email[:i]; !strings.EqualFold(local, name) {
+			usernames = append(usernames, email[:i])
-				usernames = append(usernames, local)
-			}
 		}
 		usernames = append(usernames, email)
 		return &Identity{
-			Usernames: usernames,
+			Usernames: SanitizeStringSlices(usernames),
 		}, nil
 	default:
 		return nil, errors.Errorf("provisioner type '%T' not supported by identity function", k)
 	}
 }
 
+// SanitizeStringSlices removes duplicated an empty strings.
+func SanitizeStringSlices(original []string) []string {
+	output := []string{}
+	seen := make(map[string]struct{})
+	for _, entry := range original {
+		if entry == "" {
+			continue
+		}
+		if _, value := seen[entry]; !value {
+			seen[entry] = struct{}{}
+			output = append(output, entry)
+		}
+	}
+	return output
+}
+
 // MockProvisioner for testing
 type MockProvisioner struct {
 	Mret1, Mret2, Mret3 interface{}

authority/provisioner/provisioner_test.go

@@ -62,10 +62,11 @@ func TestSanitizeSSHUserPrincipal(t *testing.T) {
 
 func TestDefaultIdentityFunc(t *testing.T) {
 	type test struct {
-		p        Interface
+		p         Interface
-		email    string
+		email     string
-		err      error
+		usernames []string
-		identity *Identity
+		err       error
+		identity  *Identity
 	}
 	tests := map[string]func(*testing.T) test{
 		"fail/unsupported-provisioner": func(t *testing.T) test {
@@ -106,7 +107,7 @@ func TestDefaultIdentityFunc(t *testing.T) {
 			return test{
 				p:        &OIDC{},
 				email:    "John@smallstep.com",
-				identity: &Identity{Usernames: []string{"john", "John@smallstep.com"}},
+				identity: &Identity{Usernames: []string{"john", "John", "John@smallstep.com"}},
 			}
 		},
 		"ok symbol": func(t *testing.T) test {
@@ -116,6 +117,30 @@ func TestDefaultIdentityFunc(t *testing.T) {
 				identity: &Identity{Usernames: []string{"john_doe", "John+Doe", "John+Doe@smallstep.com"}},
 			}
 		},
+		"ok username": func(t *testing.T) test {
+			return test{
+				p:         &OIDC{},
+				email:     "john@smallstep.com",
+				usernames: []string{"johnny"},
+				identity:  &Identity{Usernames: []string{"john", "john@smallstep.com"}},
+			}
+		},
+		"ok usernames": func(t *testing.T) test {
+			return test{
+				p:         &OIDC{},
+				email:     "john@smallstep.com",
+				usernames: []string{"johnny", "js", "", "johnny", ""},
+				identity:  &Identity{Usernames: []string{"john", "john@smallstep.com"}},
+			}
+		},
+		"ok empty username": func(t *testing.T) test {
+			return test{
+				p:         &OIDC{},
+				email:     "john@smallstep.com",
+				usernames: []string{""},
+				identity:  &Identity{Usernames: []string{"john", "john@smallstep.com"}},
+			}
+		},
 	}
 	for name, get := range tests {
 		t.Run(name, func(t *testing.T) {

authority/provisioner/utils_test.go

@@ -773,6 +773,47 @@ func generateToken(sub, iss, aud string, email string, sans []string, iat time.T
 	return jose.Signed(sig).Claims(claims).CompactSerialize()
 }
 
+func generateOIDCToken(sub, iss, aud string, email string, preferredUsername string, iat time.Time, jwk *jose.JSONWebKey, tokOpts ...tokOption) (string, error) {
+	so := new(jose.SignerOptions)
+	so.WithType("JWT")
+	so.WithHeader("kid", jwk.KeyID)
+
+	for _, o := range tokOpts {
+		if err := o(so); err != nil {
+			return "", err
+		}
+	}
+
+	sig, err := jose.NewSigner(jose.SigningKey{Algorithm: jose.ES256, Key: jwk.Key}, so)
+	if err != nil {
+		return "", err
+	}
+
+	id, err := randutil.ASCII(64)
+	if err != nil {
+		return "", err
+	}
+
+	claims := struct {
+		jose.Claims
+		Email             string `json:"email"`
+		PreferredUsername string `json:"preferred_username,omitempty"`
+	}{
+		Claims: jose.Claims{
+			ID:        id,
+			Subject:   sub,
+			Issuer:    iss,
+			IssuedAt:  jose.NewNumericDate(iat),
+			NotBefore: jose.NewNumericDate(iat),
+			Expiry:    jose.NewNumericDate(iat.Add(5 * time.Minute)),
+			Audience:  []string{aud},
+		},
+		Email:             email,
+		PreferredUsername: preferredUsername,
+	}
+	return jose.Signed(sig).Claims(claims).CompactSerialize()
+}
+
 func generateX5CSSHToken(jwk *jose.JSONWebKey, claims *x5cPayload, tokOpts ...tokOption) (string, error) {
 	so := new(jose.SignerOptions)
 	so.WithType("JWT")