feat(server): Support email for SSO+RBAC. Closes #4612 (#4644)

Signed-off-by: Alex Collins <alex_collins@intuit.com>
argoproj · Dec 11, 2020 · 433dc5b · 433dc5b
1 parent ae0c0bb
commit 433dc5b
Show file tree

Hide file tree

Showing 17 changed files with 188 additions and 51 deletions.
diff --git a/api/jsonschema/schema.json b/api/jsonschema/schema.json
@@ -760,6 +760,13 @@
     },
     "io.argoproj.workflow.v1alpha1.GetUserInfoResponse": {
       "properties": {
+        "email": {
+          "type": "string"
+        },
+        "emailVerified": {
+          "format": "boolean",
+          "type": "boolean"
+        },
         "groups": {
           "items": {
             "type": "string"

diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json
@@ -2813,6 +2813,13 @@
     "io.argoproj.workflow.v1alpha1.GetUserInfoResponse": {
       "type": "object",
       "properties": {
+        "email": {
+          "type": "string"
+        },
+        "emailVerified": {
+          "type": "boolean",
+          "format": "boolean"
+        },
         "groups": {
           "type": "array",
           "items": {

diff --git a/docs/workflow-controller-configmap.yaml b/docs/workflow-controller-configmap.yaml
@@ -239,6 +239,7 @@ data:
       # Additional scopes to request. Typically needed for SSO RBAC. >= v2.12
       scopes:
        - groups
+       - email
       # RBAC Config. >= v2.12
       rbac:
         enabled: false

diff --git a/docs/workflow-creator.md b/docs/workflow-creator.md
@@ -13,6 +13,8 @@ metadata:
   name: my-wf
   labels:
     workflows.argoproj.io/creator: admin
+    # labels must be DNS formatted, so the "@" is replaces by '.at.'  
+    workflows.argoproj.io/creator-email: admin.at.your.org
 ``` 
 
 !!! NOTE

diff --git a/manifests/quick-start/sso/overlays/argo-server-sa.yaml b/manifests/quick-start/sso/overlays/argo-server-sa.yaml
@@ -3,5 +3,5 @@ kind: ServiceAccount
 metadata:
   name: argo-server
   annotations:
-    workflows.argoproj.io/rbac-rule: "'authors' in groups"
+    workflows.argoproj.io/rbac-rule: "'authors' in groups && email == 'kilgore@kilgore.trout'"
     workflows.argoproj.io/rbac-rule-precedence: "1"
diff --git a/manifests/quick-start/sso/overlays/workflow-controller-configmap.yaml b/manifests/quick-start/sso/overlays/workflow-controller-configmap.yaml
@@ -11,6 +11,7 @@ data:
     redirectUrl: http://localhost:2746/oauth2/callback
     scopes:
     - groups
+    - email
     rbac:
       enabled: true
 kind: ConfigMap

diff --git a/pkg/apiclient/info/info.pb.go b/pkg/apiclient/info/info.pb.go
diff --git a/pkg/apiclient/info/info.proto b/pkg/apiclient/info/info.proto
@@ -24,6 +24,8 @@ message GetUserInfoResponse {
     string issuer = 1;
     string subject = 2;
     repeated string groups = 3;
+    string email = 4;
+    bool emailVerified = 5;
 }
 
 service InfoService {

diff --git a/server/auth/sso/sso.go b/server/auth/sso/sso.go
@@ -172,7 +172,7 @@ func newSso(
 	if err != nil {
 		return nil, fmt.Errorf("failed to create JWT encrpytor: %w", err)
 	}
-	log.WithFields(log.Fields{"redirectUrl": config.RedirectURL, "issuer": c.Issuer, "clientId": c.ClientID}).Info("SSO configuration")
+	log.WithFields(log.Fields{"redirectUrl": config.RedirectURL, "issuer": c.Issuer, "clientId": c.ClientID, "scopes": config.Scopes}).Info("SSO configuration")
 	return &sso{
 		config:          config,
 		idTokenVerifier: idTokenVerifier,
@@ -233,7 +233,14 @@ func (s *sso) HandleCallback(w http.ResponseWriter, r *http.Request) {
 		_, _ = w.Write([]byte(fmt.Sprintf("failed to get claims: %v", err)))
 		return
 	}
-	argoClaims := &types.Claims{Claims: jwt.Claims{Issuer: issuer, Subject: c.Subject, Expiry: jwt.NewNumericDate(time.Now().Add(s.expiry))}, Groups: c.Groups}
+	argoClaims := &types.Claims{Claims: jwt.Claims{
+		Issuer:  issuer,
+		Subject: c.Subject,
+		Expiry:  jwt.NewNumericDate(time.Now().Add(s.expiry))},
+		Groups:        c.Groups,
+		Email:         c.Email,
+		EmailVerified: c.EmailVerified,
+	}
 	raw, err := jwt.Encrypted(s.encrypter).Claims(argoClaims).CompactSerialize()
 	if err != nil {
 		panic(err)

diff --git a/server/auth/types/claims.go b/server/auth/types/claims.go
@@ -4,5 +4,7 @@ import "gopkg.in/square/go-jose.v2/jwt"
 
 type Claims struct {
 	jwt.Claims
-	Groups []string `json:"groups,omitempty"`
+	Groups        []string `json:"groups,omitempty"`
+	Email         string   `json:"email,omitempty"`
+	EmailVerified bool     `json:"email_verified,omitempty"`
 }
diff --git a/server/info/info_server.go b/server/info/info_server.go
@@ -17,7 +17,13 @@ type infoServer struct {
 func (i *infoServer) GetUserInfo(ctx context.Context, _ *infopkg.GetUserInfoRequest) (*infopkg.GetUserInfoResponse, error) {
 	claims := auth.GetClaims(ctx)
 	if claims != nil {
-		return &infopkg.GetUserInfoResponse{Subject: claims.Subject, Issuer: claims.Issuer, Groups: claims.Groups}, nil
+		return &infopkg.GetUserInfoResponse{
+			Subject:       claims.Subject,
+			Issuer:        claims.Issuer,
+			Groups:        claims.Groups,
+			Email:         claims.Email,
+			EmailVerified: claims.EmailVerified,
+		}, nil
 	}
 	return &infopkg.GetUserInfoResponse{}, nil
 }

diff --git a/server/info/info_server_test.go b/server/info/info_server_test.go
@@ -13,11 +13,13 @@ import (
 
 func Test_infoServer_GetUserInfo(t *testing.T) {
 	i := &infoServer{}
-	ctx := context.WithValue(context.TODO(), auth.ClaimsKey, &types.Claims{Claims: jwt.Claims{Issuer: "my-iss", Subject: "my-sub"}, Groups: []string{"my-group"}})
+	ctx := context.WithValue(context.TODO(), auth.ClaimsKey, &types.Claims{Claims: jwt.Claims{Issuer: "my-iss", Subject: "my-sub"}, Groups: []string{"my-group"}, Email: "my@email", EmailVerified: true})
 	info, err := i.GetUserInfo(ctx, nil)
 	if assert.NoError(t, err) {
 		assert.Equal(t, "my-iss", info.Issuer)
 		assert.Equal(t, "my-sub", info.Subject)
 		assert.Equal(t, []string{"my-group"}, info.Groups)
+		assert.Equal(t, "my@email", info.Email)
+		assert.True(t, info.EmailVerified)
 	}
 }
diff --git a/ui/src/app/userinfo/components/user-info.tsx b/ui/src/app/userinfo/components/user-info.tsx
@@ -40,6 +40,8 @@ export class UserInfo extends BasePage<RouteComponentProps<any>, State> {
                                 <p>Issuer: {this.state.userInfo.issuer || '-'}</p>
                                 <p>Subject: {this.state.userInfo.subject || '-'}</p>
                                 <p>Groups: {(this.state.userInfo.groups && this.state.userInfo.groups.length > 0 && this.state.userInfo.groups.join(', ')) || '-'}</p>
+                                <p>Email: {this.state.userInfo.email || '-'}</p>
+                                <p>Email Verified: {this.state.userInfo.emailVerified || '-'}</p>
                             </>
                         )}
                         <a className='argo-button argo-button--base-o' href={uiUrl('login')}>

diff --git a/ui/src/models/info.ts b/ui/src/models/info.ts
@@ -17,4 +17,6 @@ export interface GetUserInfoResponse {
     subject?: string;
     issuer?: string;
     groups?: string[];
+    email?: string;
+    emailVerified?: boolean;
 }
diff --git a/workflow/common/common.go b/workflow/common/common.go
@@ -55,7 +55,8 @@ const (
 	// for the purposes of workflow segregation
 	LabelKeyControllerInstanceID = workflow.WorkflowFullName + "/controller-instanceid"
 	// Who created this workflow.
-	LabelKeyCreator = workflow.WorkflowFullName + "/creator"
+	LabelKeyCreator      = workflow.WorkflowFullName + "/creator"
+	LabelKeyCreatorEmail = workflow.WorkflowFullName + "/creator-email"
 	// LabelKeyCompleted is the metadata label applied on worfklows and workflow pods to indicates if resource is completed
 	// Workflows and pods with a completed=true label will be ignored by the controller.
 	// See also `LabelKeyWorkflowArchivingStatus`.

diff --git a/workflow/creator/creator.go b/workflow/creator/creator.go
@@ -15,11 +15,18 @@ import (
 func Label(ctx context.Context, obj metav1.Object) {
 	claims := auth.GetClaims(ctx)
 	if claims != nil {
-		value := regexp.MustCompile("[^-_.a-z0-9A-Z]").ReplaceAllString(claims.Subject, "-")
-		if len(value) > 63 {
-			value = value[len(value)-63:]
+		labels.Label(obj, common.LabelKeyCreator, dnsFriendly(claims.Subject))
+		if claims.Email != "" {
+			labels.Label(obj, common.LabelKeyCreatorEmail, dnsFriendly(strings.Replace(claims.Email, "@", ".at.", 1)))
 		}
-		value = strings.TrimLeft(value, "-")
-		labels.Label(obj, common.LabelKeyCreator, value)
 	}
 }
+
+func dnsFriendly(s string) string {
+	value := regexp.MustCompile("[^-_.a-z0-9A-Z]").ReplaceAllString(s, "-")
+	if len(value) > 63 {
+		value = value[len(value)-63:]
+	}
+	value = strings.TrimLeft(value, "-")
+	return value
+}
diff --git a/workflow/creator/creator_test.go b/workflow/creator/creator_test.go
@@ -23,16 +23,10 @@ func TestLabel(t *testing.T) {
 	})
 	t.Run("NotEmpty", func(t *testing.T) {
 		wf := &wfv1.Workflow{}
-		Label(context.WithValue(context.TODO(), auth.ClaimsKey, &types.Claims{Claims: jwt.Claims{Subject: "my-sub"}}), wf)
+		Label(context.WithValue(context.TODO(), auth.ClaimsKey, &types.Claims{Claims: jwt.Claims{Subject: strings.Repeat("x", 63) + "y"}, Email: "my@email"}), wf)
 		if assert.NotEmpty(t, wf.Labels) {
-			assert.Contains(t, wf.Labels, common.LabelKeyCreator)
-		}
-	})
-	t.Run("TooLong", func(t *testing.T) {
-		wf := &wfv1.Workflow{}
-		Label(context.WithValue(context.TODO(), auth.ClaimsKey, &types.Claims{Claims: jwt.Claims{Subject: strings.Repeat("x", 63) + "y"}}), wf)
-		if assert.NotEmpty(t, wf.Labels) {
-			assert.Equal(t, strings.Repeat("x", 62)+"y", wf.Labels[common.LabelKeyCreator])
+			assert.Equal(t, strings.Repeat("x", 62)+"y", wf.Labels[common.LabelKeyCreator], "creator is truncated")
+			assert.Equal(t, "my.at.email", wf.Labels[common.LabelKeyCreatorEmail], "'@' is replaced by '.at.'")
 		}
 	})
 	t.Run("TooLongHyphen", func(t *testing.T) {