Skip to content

Commit

Permalink
feat: add start time (abcxyz#23)
Browse files Browse the repository at this point in the history
  • Loading branch information
sqin2019 authored and verbanicm committed May 23, 2023
1 parent de07c5d commit 8eab5c5
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 19 deletions.
4 changes: 4 additions & 0 deletions apis/v1alpha1/iam_request_wrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,8 @@ type IAMRequestWrapper struct {
// Duration feild used as IAM binding condition to specify expiration.
// This will not override role bindings with no conditions.
Duration time.Duration

// Start time of the IAM permission lifecycle, StartTime + Duration is when
// the permission will expire.
StartTime time.Time
}
33 changes: 31 additions & 2 deletions pkg/cli/iam_handle.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ type IAMHandleCommand struct {

flagDuration time.Duration

// TODO(#13): add start time flag.
flagStartTime time.Time

// testHandler is used for testing only.
testHandler iamHandler
Expand All @@ -58,7 +58,7 @@ Usage: {{ COMMAND }} [options]
Handle the IAM request YAML file in the given path:
aod iam handle -path "/path/to/file.yaml" -duration "2h"
aod iam handle -path "/path/to/file.yaml" -duration "2h" -start-time "2009-11-10T23:00:00Z"
`
}

Expand All @@ -82,9 +82,33 @@ func (c *IAMHandleCommand) Flags() *cli.FlagSet {
Usage: `The IAM permission lifecycle, as a duration.`,
})

cli.Flag(f, &cli.Var[time.Time]{
Name: "start-time",
Usage: `The start time of the IAM permission lifecycle in RFC3339 format. Default is current UTC time.`,
Example: "2009-11-10T23:00:00Z",
Default: time.Now().UTC(),
Target: &c.flagStartTime,
Parser: timeParser,
Printer: timePrinter,
})

return set
}

// Parse the string representation of time, it must be in RFC3339 format.
func timeParser(s string) (time.Time, error) {
t, err := time.Parse(time.RFC3339, s)
if err != nil {
return t, fmt.Errorf("failed to parse %q with RFC3339 layout: %w", s, err)
}
return t, nil
}

// Print time in RFC3339 format.
func timePrinter(t time.Time) string {
return t.Format(time.RFC3339)
}

func (c *IAMHandleCommand) Run(ctx context.Context, args []string) error {
f := c.Flags()
if err := f.Parse(args); err != nil {
Expand All @@ -103,6 +127,10 @@ func (c *IAMHandleCommand) Run(ctx context.Context, args []string) error {
return fmt.Errorf("a positive duration is required")
}

if c.flagStartTime.Add(c.flagDuration).Before(time.Now()) {
return fmt.Errorf("expiry (start time + duration) already passed")
}

return c.handleIAM(ctx)
}

Expand Down Expand Up @@ -153,6 +181,7 @@ func (c *IAMHandleCommand) handleIAM(ctx context.Context) error {
reqWrapper := &v1alpha1.IAMRequestWrapper{
IAMRequest: req,
Duration: c.flagDuration,
StartTime: c.flagStartTime,
}
// TODO(#15): add a log level to output handler response.
if _, err := h.Do(ctx, reqWrapper); err != nil {
Expand Down
23 changes: 19 additions & 4 deletions pkg/cli/iam_handle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
"github.com/abcxyz/pkg/logging"
"github.com/abcxyz/pkg/testutil"
"github.com/google/go-cmp/cmp"
"google.golang.org/protobuf/testing/protocmp"
)

func TestIAMHandleCommand(t *testing.T) {
Expand Down Expand Up @@ -115,6 +114,8 @@ policies:
},
}

st := time.Now().UTC().Round(time.Second)

cases := []struct {
name string
args []string
Expand All @@ -126,12 +127,13 @@ policies:
}{
{
name: "success",
args: []string{"-path", filepath.Join(dir, "valid.yaml"), "-duration", "2h"},
args: []string{"-path", filepath.Join(dir, "valid.yaml"), "-duration", "2h", "-start-time", st.Format(time.RFC3339)},
handler: &fakeIAMHandler{},
expOut: "Successfully handled IAM request",
expReq: &v1alpha1.IAMRequestWrapper{
IAMRequest: validRequest,
Duration: 2 * time.Hour,
StartTime: st,
},
},
{
Expand All @@ -158,6 +160,18 @@ policies:
handler: &fakeIAMHandler{},
expErr: "a positive duration is required",
},
{
name: "invalid_start_time",
args: []string{"-path", filepath.Join(dir, "valid.yaml"), "-duration", "2h", "-start-time", "2009"},
handler: &fakeIAMHandler{},
expErr: `failed to parse "2009" with RFC3339 layout`,
},
{
name: "expiry_passed",
args: []string{"-path", filepath.Join(dir, "valid.yaml"), "-duration", "2h", "-start-time", "2009-11-10T23:00:00Z"},
handler: &fakeIAMHandler{},
expErr: "expiry (start time + duration) already passed",
},
{
name: "invalid_yaml",
args: []string{"-path", filepath.Join(dir, "invalid.yaml"), "-duration", "2h"},
Expand All @@ -166,14 +180,15 @@ policies:
},
{
name: "handler_failure",
args: []string{"-path", filepath.Join(dir, "valid.yaml"), "-duration", "1h"},
args: []string{"-path", filepath.Join(dir, "valid.yaml"), "-duration", "1h", "-start-time", st.Format(time.RFC3339)},
handler: &fakeIAMHandler{
injectErr: fmt.Errorf("injected error"),
},
expErr: "injected error",
expReq: &v1alpha1.IAMRequestWrapper{
IAMRequest: validRequest,
Duration: 1 * time.Hour,
StartTime: st,
},
},
}
Expand All @@ -199,7 +214,7 @@ policies:
if diff := cmp.Diff(strings.TrimSpace(tc.expOut), strings.TrimSpace(stdout.String())); diff != "" {
t.Errorf("Process(%+v) got output diff (-want, +got):\n%s", tc.name, diff)
}
if diff := cmp.Diff(tc.expReq, tc.handler.gotReq, protocmp.Transform()); diff != "" {
if diff := cmp.Diff(tc.expReq, tc.handler.gotReq); diff != "" {
t.Errorf("Process(%+v) got request diff (-want, +got):\n%s", tc.name, diff)
}
})
Expand Down
11 changes: 6 additions & 5 deletions pkg/handler/iam_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,9 @@ func NewIAMHandler(ctx context.Context, organizationsClient, foldersClient, proj

// Do removes expired or duplicative IAM bindings added by AOD and adds requested IAM bindings to current IAM policy.
func (h *IAMHandler) Do(ctx context.Context, r *v1alpha1.IAMRequestWrapper) (nps []*v1alpha1.IAMResponse, retErr error) {
expiry := r.StartTime.Add(r.Duration)
for _, p := range r.ResourcePolicies {
np, err := h.handlePolicy(ctx, p, r.Duration)
np, err := h.handlePolicy(ctx, p, expiry)
if err != nil {
retErr = errors.Join(
retErr,
Expand All @@ -99,7 +100,7 @@ func (h *IAMHandler) Do(ctx context.Context, r *v1alpha1.IAMRequestWrapper) (nps
return
}

func (h *IAMHandler) handlePolicy(ctx context.Context, p *v1alpha1.ResourcePolicy, ttl time.Duration) (*v1alpha1.IAMResponse, error) {
func (h *IAMHandler) handlePolicy(ctx context.Context, p *v1alpha1.ResourcePolicy, expiry time.Time) (*v1alpha1.IAMResponse, error) {
var iamC IAMClient
switch strings.Split(p.Resource, "/")[0] {
case "organizations":
Expand All @@ -121,7 +122,7 @@ func (h *IAMHandler) handlePolicy(ctx context.Context, p *v1alpha1.ResourcePolic
}

// Update the policy with new IAM binding additions.
updatePolicy(cp, p.Bindings, ttl)
updatePolicy(cp, p.Bindings, expiry)

// Set the new policy.
setIamPolicyRequest := &iampb.SetIamPolicyRequest{
Expand All @@ -143,7 +144,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, ttl time.Duration) {
func updatePolicy(p *iampb.Policy, bs []*v1alpha1.Binding, expiry time.Time) {
// Convert new bindings to a role to unique bindings map.
bsMap := toBindingsMap(bs)
// Clean up current policy bindings.
Expand Down Expand Up @@ -174,7 +175,7 @@ func updatePolicy(p *iampb.Policy, bs []*v1alpha1.Binding, ttl time.Duration) {
p.Bindings = result

// Add new bindings with expiration condition.
t := time.Now().UTC().Add(ttl).Format(time.RFC3339)
t := expiry.Format(time.RFC3339)
for r, ms := range bsMap {
newBinding := &iampb.Binding{
Condition: &expr.Expr{
Expand Down
18 changes: 12 additions & 6 deletions pkg/handler/iam_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ func TestDo(t *testing.T) {
},
},
},
Duration: 2 * time.Hour,
Duration: 2 * time.Hour,
StartTime: now,
},
wantPolicies: []*v1alpha1.IAMResponse{
{
Expand Down Expand Up @@ -262,7 +263,8 @@ func TestDo(t *testing.T) {
},
},
},
Duration: 2 * time.Hour,
Duration: 2 * time.Hour,
StartTime: now,
},
wantPolicies: []*v1alpha1.IAMResponse{
{
Expand Down Expand Up @@ -359,7 +361,8 @@ func TestDo(t *testing.T) {
},
},
},
Duration: 2 * time.Hour,
Duration: 2 * time.Hour,
StartTime: now,
},
wantPolicies: []*v1alpha1.IAMResponse{
{
Expand Down Expand Up @@ -450,7 +453,8 @@ func TestDo(t *testing.T) {
},
},
},
Duration: 2 * time.Hour,
Duration: 2 * time.Hour,
StartTime: now,
},
wantPolicies: []*v1alpha1.IAMResponse{
{
Expand Down Expand Up @@ -548,7 +552,8 @@ func TestDo(t *testing.T) {
},
},
},
Duration: 2 * time.Hour,
Duration: 2 * time.Hour,
StartTime: now,
},
wantPolicies: []*v1alpha1.IAMResponse{
{
Expand Down Expand Up @@ -626,7 +631,8 @@ func TestDo(t *testing.T) {
},
},
},
Duration: 1 * time.Hour,
Duration: 1 * time.Hour,
StartTime: now,
},
wantPolicies: []*v1alpha1.IAMResponse{
{
Expand Down
3 changes: 1 addition & 2 deletions pkg/requestutil/request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"github.com/abcxyz/access-on-demand/apis/v1alpha1"
"github.com/abcxyz/pkg/testutil"
"github.com/google/go-cmp/cmp"
"google.golang.org/protobuf/testing/protocmp"
)

func TestIAMValidateCommand(t *testing.T) {
Expand Down Expand Up @@ -139,7 +138,7 @@ policies:
t.Errorf("Process(%+v) got error diff (-want, +got):\n%s", tc.name, diff)
}

if diff := cmp.Diff(tc.expReq, req, protocmp.Transform()); diff != "" {
if diff := cmp.Diff(tc.expReq, req); diff != "" {
t.Errorf("Process(%+v) got request diff (-want, +got): %v", tc.name, diff)
}
})
Expand Down

0 comments on commit 8eab5c5

Please sign in to comment.