Merge pull request #1043 from unreality/oidc-missing-email

Allow missing Email claim in OIDC tokens

authority/provisioner/oidc.go

@@ -376,31 +376,46 @@ func (o *OIDC) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOption
 	if err != nil {
 		return nil, errs.Wrap(http.StatusInternalServerError, err, "oidc.AuthorizeSSHSign")
 	}
-	// Enforce an email claim
+
+	if claims.Subject == "" {
+		return nil, errs.Unauthorized("oidc.AuthorizeSSHSign: failed to validate oidc token payload: subject not found")
+	}
+
+	var data sshutil.TemplateData
+	var principals []string
+
 	if claims.Email == "" {
-		return nil, errs.Unauthorized("oidc.AuthorizeSSHSign: failed to validate oidc token payload: email not found")
+		// If email is empty, use the Subject claim instead to create minimal data for the template to use
-	}
+		data = sshutil.CreateTemplateData(sshutil.UserCert, claims.Subject, nil)
+		if v, err := unsafeParseSigned(token); err == nil {
+			data.SetToken(v)
+		}
 
-	// Get the identity using either the default identityFunc or one injected
+		principals = nil
-	// externally. Note that the PreferredUsername might be empty.
+	} else {
-	// TBD: Would preferred_username present a safety issue here?
+		// Get the identity using either the default identityFunc or one injected
-	iden, err := o.ctl.GetIdentity(ctx, claims.Email)
+		// externally. Note that the PreferredUsername might be empty.
-	if err != nil {
+		// TBD: Would preferred_username present a safety issue here?
-		return nil, errs.Wrap(http.StatusInternalServerError, err, "oidc.AuthorizeSSHSign")
+		iden, err := o.ctl.GetIdentity(ctx, claims.Email)
-	}
+		if err != nil {
+			return nil, errs.Wrap(http.StatusInternalServerError, err, "oidc.AuthorizeSSHSign")
+		}
 
-	// Certificate templates.
+		// Certificate templates.
-	data := sshutil.CreateTemplateData(sshutil.UserCert, claims.Email, iden.Usernames)
+		data = sshutil.CreateTemplateData(sshutil.UserCert, claims.Email, iden.Usernames)
-	if v, err := unsafeParseSigned(token); err == nil {
+		if v, err := unsafeParseSigned(token); err == nil {
-		data.SetToken(v)
+			data.SetToken(v)
-	}
+		}
-	// Add custom extensions added in the identity function.
+		// Add custom extensions added in the identity function.
-	for k, v := range iden.Permissions.Extensions {
+		for k, v := range iden.Permissions.Extensions {
-		data.AddExtension(k, v)
+			data.AddExtension(k, v)
-	}
+		}
-	// Add custom critical options added in the identity function.
+		// Add custom critical options added in the identity function.
-	for k, v := range iden.Permissions.CriticalOptions {
+		for k, v := range iden.Permissions.CriticalOptions {
-		data.AddCriticalOption(k, v)
+			data.AddCriticalOption(k, v)
+		}
+
+		principals = iden.Usernames
 	}
 
 	// Use the default template unless no-templates are configured and email is
@@ -429,7 +444,7 @@ func (o *OIDC) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOption
 	} else {
 		signOptions = append(signOptions, sshCertOptionsValidator(SignSSHOptions{
 			CertType:   SSHUserCert,
-			Principals: iden.Usernames,
+			Principals: principals,
 		}))
 	}
 

authority/provisioner/oidc_test.go

@@ -523,7 +523,12 @@ func TestOIDC_AuthorizeSSHSign(t *testing.T) {
 	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])
+	emptyEmail, err := generateToken("subject", "the-issuer", p1.ClientID, "", []string{}, time.Now(), &keys.Keys[0])
+	expectemptyEmailOptions := &SignSSHOptions{
+		CertType:   "user",
+		Principals: []string{},
+		ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(p1.ctl.Claimer.DefaultUserSSHCertDuration())),
+	}
 	assert.FatalError(t, err)
 
 	key, err := generateJSONWebKey()
@@ -571,6 +576,7 @@ func TestOIDC_AuthorizeSSHSign(t *testing.T) {
 		{"ok", p1, args{t1, SignSSHOptions{}, pub}, expectedUserOptions, http.StatusOK, false, false},
 		{"ok-rsa2048", p1, args{t1, SignSSHOptions{}, rsa2048.Public()}, expectedUserOptions, http.StatusOK, false, false},
 		{"ok-user", p1, args{t1, SignSSHOptions{CertType: "user"}, pub}, expectedUserOptions, http.StatusOK, false, false},
+		{"ok-empty-email", p1, args{emptyEmail, SignSSHOptions{CertType: "user"}, pub}, expectemptyEmailOptions, http.StatusOK, false, false},
 		{"ok-principals", p1, args{t1, SignSSHOptions{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},
@@ -593,7 +599,6 @@ func TestOIDC_AuthorizeSSHSign(t *testing.T) {
 		{"fail-rsa1024", p1, args{t1, SignSSHOptions{}, rsa1024.Public()}, expectedUserOptions, http.StatusOK, false, true},
 		{"fail-user-host", p1, args{t1, SignSSHOptions{CertType: "host"}, pub}, nil, http.StatusOK, false, true},
 		{"fail-user-principals", p1, args{t1, SignSSHOptions{Principals: []string{"root"}}, pub}, nil, http.StatusOK, false, true},
-		{"fail-email", p3, args{failEmail, SignSSHOptions{}, pub}, nil, http.StatusUnauthorized, true, false},
 		{"fail-getIdentity", p5, args{failGetIdentityToken, SignSSHOptions{}, pub}, nil, http.StatusInternalServerError, true, false},
 		{"fail-sshCA-disabled", p6, args{"foo", SignSSHOptions{}, pub}, nil, http.StatusUnauthorized, true, false},
 		// Missing parametrs

authority/provisioner/ssh_test.go

@@ -20,7 +20,7 @@ func validateSSHCertificate(cert *ssh.Certificate, opts *SignSSHOptions) error {
 		return fmt.Errorf("certificate signature is nil")
 	case cert.SignatureKey == nil:
 		return fmt.Errorf("certificate signature is nil")
-	case !reflect.DeepEqual(cert.ValidPrincipals, opts.Principals):
+	case !reflect.DeepEqual(cert.ValidPrincipals, opts.Principals) && (len(opts.Principals) > 0 || len(cert.ValidPrincipals) > 0):
 		return fmt.Errorf("certificate principals are not equal, want %v, got %v", opts.Principals, cert.ValidPrincipals)
 	case cert.CertType != ssh.UserCert && cert.CertType != ssh.HostCert:
 		return fmt.Errorf("certificate type %v is not valid", cert.CertType)