From cb635fd2a2e87c3c43d722f4bee0290e4a198472 Mon Sep 17 00:00:00 2001 From: Suhong Qin <51539171+sqin2019@users.noreply.github.com> Date: Tue, 20 Jun 2023 11:33:11 -0700 Subject: [PATCH] feat: remove expired IAM bindings (#42) --- pkg/handler/iam_handler.go | 46 ++++++- pkg/handler/iam_handler_test.go | 204 ++++++++++++++++++++++++++++++++ 2 files changed, 244 insertions(+), 6 deletions(-) diff --git a/pkg/handler/iam_handler.go b/pkg/handler/iam_handler.go index 6eb9704..770986c 100644 --- a/pkg/handler/iam_handler.go +++ b/pkg/handler/iam_handler.go @@ -19,6 +19,7 @@ import ( "context" "errors" "fmt" + "regexp" "sort" "strings" "time" @@ -30,8 +31,14 @@ import ( "google.golang.org/genproto/googleapis/type/expr" ) -// ConditionTitle of IAM bindings added by AOD. -var ConditionTitle = "abcxyz-aod-expiry" +var ( + // ConditionTitle of IAM bindings added by AOD. + ConditionTitle = "abcxyz-aod-expiry" + // expirationExpression of IAM binding condition added by AOD. + expirationExpression = "request.time < timestamp('%s')" + // expirationRegex matching expirationExpression. + expirationRegex = regexp.MustCompile(`request.time < timestamp\('([^']+)'\)`) +) // IAMHandler updates IAM policies of GCP organizations, folders, and projects // based on the IAM request received. @@ -132,8 +139,12 @@ func (h *IAMHandler) handlePolicy(ctx context.Context, p *v1alpha1.ResourcePolic return fmt.Errorf("failed to get IAM policy: %w", err) } + // TODO (#44): Continue to handle policy and alert updatePolicy error + // differently. // Update the policy with new IAM binding additions. - updatePolicy(cp, p.Bindings, expiry) + if err := updatePolicy(cp, p.Bindings, expiry); err != nil { + return fmt.Errorf("failed to update IAM policy: %w", err) + } // Set the new policy. setIAMPolicyRequest := &iampb.SetIamPolicyRequest{ @@ -155,7 +166,7 @@ func (h *IAMHandler) handlePolicy(ctx context.Context, p *v1alpha1.ResourcePolic } // Remove expired bindings and add or update new bindings with expiration condition. -func updatePolicy(p *iampb.Policy, bs []*v1alpha1.Binding, expiry time.Time) { +func updatePolicy(p *iampb.Policy, bs []*v1alpha1.Binding, expiry time.Time) error { // Convert new bindings to a role to unique bindings map. bsMap := toBindingsMap(bs) // Clean up current policy bindings. @@ -166,7 +177,17 @@ func updatePolicy(p *iampb.Policy, bs []*v1alpha1.Binding, expiry time.Time) { result = append(result, cb) continue } - // TODO (#6): Remove expired bindings. + + // Skip expired bindings. + expired, err := expired(cb.Condition.Expression) + if err != nil { + // Return error immediately since we don't expect this to fail. + return fmt.Errorf("failed to check expiry: %w", err) + } + if expired { + continue + } + // Skip roles we are not interested in. if _, ok := bsMap[cb.Role]; !ok { result = append(result, cb) @@ -191,7 +212,7 @@ func updatePolicy(p *iampb.Policy, bs []*v1alpha1.Binding, expiry time.Time) { newBinding := &iampb.Binding{ Condition: &expr.Expr{ Title: ConditionTitle, - Expression: fmt.Sprintf("request.time < timestamp('%s')", t), + Expression: fmt.Sprintf(expirationExpression, t), }, Role: r, } @@ -205,6 +226,7 @@ func updatePolicy(p *iampb.Policy, bs []*v1alpha1.Binding, expiry time.Time) { // Set policy version to 3 to support conditional IAM bindings. // See details here: https://cloud.google.com/iam/docs/policies#specifying-version-set p.Version = 3 + return nil } func toBindingsMap(bs []*v1alpha1.Binding) map[string]map[string]struct{} { @@ -219,3 +241,15 @@ func toBindingsMap(bs []*v1alpha1.Binding) map[string]map[string]struct{} { } return result } + +func expired(exp string) (bool, error) { + matches := expirationRegex.FindStringSubmatch(exp) + if len(matches) < 2 { + return false, fmt.Errorf("expression %q does not match format %q", exp, "request.time < timestamp('YYYY-MM-DDTHH:MM:SSZ')") + } + t, err := time.Parse(time.RFC3339, matches[1]) + if err != nil { + return false, fmt.Errorf("failed to parse expiration %q: %w", exp, err) + } + return t.Before(time.Now()), nil +} diff --git a/pkg/handler/iam_handler_test.go b/pkg/handler/iam_handler_test.go index 0e7e26c..a6ee1d1 100644 --- a/pkg/handler/iam_handler_test.go +++ b/pkg/handler/iam_handler_test.go @@ -332,6 +332,210 @@ func TestDo(t *testing.T) { wantFoldersPolicy: &iampb.Policy{}, wantProjectsPolicy: &iampb.Policy{}, }, + { + name: "clean_up_expired_bindings", + organizationsServer: &fakeServer{ + policy: &iampb.Policy{ + Bindings: []*iampb.Binding{ + // Expired bindings to be removed. + { + Members: []string{ + "user:test-org-userB@example.com", + }, + Role: "roles/accessapproval.approver", + Condition: &expr.Expr{ + Title: ConditionTitle, + Expression: fmt.Sprintf("request.time < timestamp('%s')", now.Add(-1*time.Hour).Format(time.RFC3339)), + }, + }, + }, + }, + }, + foldersServer: &fakeServer{ + policy: &iampb.Policy{}, + }, + projectsServer: &fakeServer{ + policy: &iampb.Policy{}, + }, + request: &v1alpha1.IAMRequestWrapper{ + IAMRequest: &v1alpha1.IAMRequest{ + ResourcePolicies: []*v1alpha1.ResourcePolicy{ + { + Resource: "organizations/foo", + Bindings: []*v1alpha1.Binding{ + { + Members: []string{ + "user:test-org-userA@example.com", + }, + Role: "roles/bigquery.dataViewer", + }, + }, + }, + }, + }, + Duration: 2 * time.Hour, + StartTime: now, + }, + wantPolicies: []*v1alpha1.IAMResponse{ + { + Resource: "organizations/foo", + Policy: &iampb.Policy{ + Bindings: []*iampb.Binding{ + { + Members: []string{ + "user:test-org-userA@example.com", + }, + Role: "roles/bigquery.dataViewer", + Condition: &expr.Expr{ + Title: ConditionTitle, + Expression: fmt.Sprintf("request.time < timestamp('%s')", now.Add(2*time.Hour).Format(time.RFC3339)), + }, + }, + }, + Version: 3, + }, + }, + }, + wantOrganizationsPolicy: &iampb.Policy{ + Bindings: []*iampb.Binding{ + { + Members: []string{ + "user:test-org-userA@example.com", + }, + Role: "roles/bigquery.dataViewer", + Condition: &expr.Expr{ + Title: ConditionTitle, + Expression: fmt.Sprintf("request.time < timestamp('%s')", now.Add(2*time.Hour).Format(time.RFC3339)), + }, + }, + }, + Version: 3, + }, + wantFoldersPolicy: &iampb.Policy{}, + wantProjectsPolicy: &iampb.Policy{}, + }, + { + name: "failed_clean_up_expired_bindings_invalid_expression", + organizationsServer: &fakeServer{ + policy: &iampb.Policy{ + Bindings: []*iampb.Binding{ + { + Members: []string{ + "user:test-org-userB@example.com", + }, + Role: "roles/accessapproval.approver", + Condition: &expr.Expr{ + Title: ConditionTitle, + Expression: fmt.Sprintf("request.time <= timestamp('%s')", now.Add(-1*time.Hour).Format(time.RFC3339)), + }, + }, + }, + }, + }, + foldersServer: &fakeServer{ + policy: &iampb.Policy{}, + }, + projectsServer: &fakeServer{ + policy: &iampb.Policy{}, + }, + request: &v1alpha1.IAMRequestWrapper{ + IAMRequest: &v1alpha1.IAMRequest{ + ResourcePolicies: []*v1alpha1.ResourcePolicy{ + { + Resource: "organizations/foo", + Bindings: []*v1alpha1.Binding{ + { + Members: []string{ + "user:test-org-userA@example.com", + }, + Role: "roles/bigquery.dataViewer", + }, + }, + }, + }, + }, + Duration: 2 * time.Hour, + StartTime: now, + }, + wantErrSubstr: `does not match format "request.time < timestamp('YYYY-MM-DDTHH:MM:SSZ')"`, + wantOrganizationsPolicy: &iampb.Policy{ + Bindings: []*iampb.Binding{ + { + Members: []string{ + "user:test-org-userB@example.com", + }, + Role: "roles/accessapproval.approver", + Condition: &expr.Expr{ + Title: ConditionTitle, + Expression: fmt.Sprintf("request.time <= timestamp('%s')", now.Add(-1*time.Hour).Format(time.RFC3339)), + }, + }, + }, + }, + wantFoldersPolicy: &iampb.Policy{}, + wantProjectsPolicy: &iampb.Policy{}, + }, + { + name: "failed_clean_up_expired_bindings_wrong_expiration", + organizationsServer: &fakeServer{ + policy: &iampb.Policy{ + Bindings: []*iampb.Binding{ + { + Members: []string{ + "user:test-org-userB@example.com", + }, + Role: "roles/accessapproval.approver", + Condition: &expr.Expr{ + Title: ConditionTitle, + Expression: fmt.Sprintf("request.time < timestamp('%s')", now.Add(-1*time.Hour).Format(time.RFC850)), + }, + }, + }, + }, + }, + foldersServer: &fakeServer{ + policy: &iampb.Policy{}, + }, + projectsServer: &fakeServer{ + policy: &iampb.Policy{}, + }, + request: &v1alpha1.IAMRequestWrapper{ + IAMRequest: &v1alpha1.IAMRequest{ + ResourcePolicies: []*v1alpha1.ResourcePolicy{ + { + Resource: "organizations/foo", + Bindings: []*v1alpha1.Binding{ + { + Members: []string{ + "user:test-org-userA@example.com", + }, + Role: "roles/bigquery.dataViewer", + }, + }, + }, + }, + }, + Duration: 2 * time.Hour, + StartTime: now, + }, + wantErrSubstr: "failed to parse expiration", + wantOrganizationsPolicy: &iampb.Policy{ + Bindings: []*iampb.Binding{ + { + Members: []string{ + "user:test-org-userB@example.com", + }, + Role: "roles/accessapproval.approver", + Condition: &expr.Expr{ + Title: ConditionTitle, + Expression: fmt.Sprintf("request.time < timestamp('%s')", now.Add(-1*time.Hour).Format(time.RFC850)), + }, + }, + }, + }, + wantFoldersPolicy: &iampb.Policy{}, + wantProjectsPolicy: &iampb.Policy{}, + }, { name: "ignore_non-AOD_bindings", organizationsServer: &fakeServer{