diff --git a/api/types/accesslist/accesslist.go b/api/types/accesslist/accesslist.go index 16fde6c36eeb9..e214cfc36a3cd 100644 --- a/api/types/accesslist/accesslist.go +++ b/api/types/accesslist/accesslist.go @@ -22,6 +22,7 @@ import ( "time" "github.com/gravitational/trace" + "github.com/jonboulle/clockwork" "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/api/types/header" @@ -311,10 +312,6 @@ func (a *AccessList) CheckAndSetDefaults() error { return trace.BadParameter("owners are missing") } - if a.Spec.Audit.NextAuditDate.IsZero() { - return trace.BadParameter("next audit date is missing") - } - if a.Spec.Audit.Recurrence.Frequency == 0 { a.Spec.Audit.Recurrence.Frequency = SixMonths } @@ -335,6 +332,10 @@ func (a *AccessList) CheckAndSetDefaults() error { return trace.BadParameter("recurrence day of month is an invalid value") } + if a.Spec.Audit.NextAuditDate.IsZero() { + a.setInitialAuditDate(clockwork.NewRealClock()) + } + if a.Spec.Audit.Notifications.Start == 0 { a.Spec.Audit.Notifications.Start = twoWeeks } @@ -527,3 +528,32 @@ func (n Notifications) MarshalJSON() ([]byte, error) { Start: n.Start.String(), }) } + +// SelectNextReviewDate will select the next review date for the access list. +func (a *AccessList) SelectNextReviewDate() time.Time { + numMonths := int(a.Spec.Audit.Recurrence.Frequency) + dayOfMonth := int(a.Spec.Audit.Recurrence.DayOfMonth) + + // If the last day of the month has been specified, use the 0 day of the + // next month, which will result in the last day of the target month. + if dayOfMonth == int(LastDayOfMonth) { + numMonths += 1 + dayOfMonth = 0 + } + + currentReviewDate := a.Spec.Audit.NextAuditDate + nextDate := time.Date(currentReviewDate.Year(), currentReviewDate.Month()+time.Month(numMonths), dayOfMonth, + 0, 0, 0, 0, time.UTC) + + return nextDate +} + +// setInitialAuditDate sets the NextAuditDate for a newly created AccessList. +// The function is extracted from CheckAndSetDefaults for the sake of testing +// (we need to pass a fake clock). +func (a *AccessList) setInitialAuditDate(clock clockwork.Clock) { + // We act as if the AccessList just got reviewed (we just created it, so + // we're pretty sure of what it does) and pick the next review date. + a.Spec.Audit.NextAuditDate = clock.Now() + a.Spec.Audit.NextAuditDate = a.SelectNextReviewDate() +} diff --git a/api/types/accesslist/accesslist_test.go b/api/types/accesslist/accesslist_test.go index 9ad04826ef4a4..64c1498309e55 100644 --- a/api/types/accesslist/accesslist_test.go +++ b/api/types/accesslist/accesslist_test.go @@ -21,6 +21,7 @@ import ( "testing" "time" + "github.com/jonboulle/clockwork" "github.com/stretchr/testify/require" "github.com/gravitational/teleport/api/types/header" @@ -278,3 +279,113 @@ func TestAccessListDefaults(t *testing.T) { require.NoError(t, err) }) } + +func TestSelectNextReviewDate(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + frequency ReviewFrequency + dayOfMonth ReviewDayOfMonth + currentReviewDate time.Time + expected time.Time + }{ + { + name: "one month, first day", + frequency: OneMonth, + dayOfMonth: FirstDayOfMonth, + currentReviewDate: time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC), + expected: time.Date(2023, 2, 1, 0, 0, 0, 0, time.UTC), + }, + { + name: "one month, fifteenth day", + frequency: OneMonth, + dayOfMonth: FifteenthDayOfMonth, + currentReviewDate: time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC), + expected: time.Date(2023, 2, 15, 0, 0, 0, 0, time.UTC), + }, + { + name: "one month, last day", + frequency: OneMonth, + dayOfMonth: LastDayOfMonth, + currentReviewDate: time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC), + expected: time.Date(2023, 2, 28, 0, 0, 0, 0, time.UTC), + }, + { + name: "six months, last day", + frequency: SixMonths, + dayOfMonth: LastDayOfMonth, + currentReviewDate: time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC), + expected: time.Date(2023, 7, 31, 0, 0, 0, 0, time.UTC), + }, + } + + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + accessList := AccessList{} + accessList.Spec.Audit.NextAuditDate = test.currentReviewDate + accessList.Spec.Audit.Recurrence = Recurrence{ + Frequency: test.frequency, + DayOfMonth: test.dayOfMonth, + } + require.Equal(t, test.expected, accessList.SelectNextReviewDate()) + }) + } +} + +func TestAccessList_setInitialReviewDate(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + frequency ReviewFrequency + dayOfMonth ReviewDayOfMonth + currentReviewDate time.Time + expected time.Time + }{ + { + name: "one month, first day", + frequency: OneMonth, + dayOfMonth: FirstDayOfMonth, + currentReviewDate: time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC), + expected: time.Date(2023, 2, 1, 0, 0, 0, 0, time.UTC), + }, + { + name: "one month, fifteenth day", + frequency: OneMonth, + dayOfMonth: FifteenthDayOfMonth, + currentReviewDate: time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC), + expected: time.Date(2023, 2, 15, 0, 0, 0, 0, time.UTC), + }, + { + name: "one month, last day", + frequency: OneMonth, + dayOfMonth: LastDayOfMonth, + currentReviewDate: time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC), + expected: time.Date(2023, 2, 28, 0, 0, 0, 0, time.UTC), + }, + { + name: "six months, last day", + frequency: SixMonths, + dayOfMonth: LastDayOfMonth, + currentReviewDate: time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC), + expected: time.Date(2023, 7, 31, 0, 0, 0, 0, time.UTC), + }, + } + + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + accessList := AccessList{} + accessList.Spec.Audit.Recurrence = Recurrence{ + Frequency: test.frequency, + DayOfMonth: test.dayOfMonth, + } + accessList.setInitialAuditDate(clockwork.NewFakeClockAt(test.currentReviewDate)) + require.Equal(t, test.expected, accessList.Spec.Audit.NextAuditDate) + }) + } +} diff --git a/lib/services/access_list.go b/lib/services/access_list.go index ac294548b270a..57d1bf1e6ce7b 100644 --- a/lib/services/access_list.go +++ b/lib/services/access_list.go @@ -345,25 +345,6 @@ func UserMeetsRequirements(identity tlsca.Identity, requires accesslist.Requires return true } -// SelectNextReviewDate will select the next review date for the access list. -func SelectNextReviewDate(accessList *accesslist.AccessList) time.Time { - numMonths := int(accessList.Spec.Audit.Recurrence.Frequency) - dayOfMonth := int(accessList.Spec.Audit.Recurrence.DayOfMonth) - - // If the last day of the month has been specified, use the 0 day of the - // next month, which will result in the last day of the target month. - if dayOfMonth == int(accesslist.LastDayOfMonth) { - numMonths += 1 - dayOfMonth = 0 - } - - currentReviewDate := accessList.Spec.Audit.NextAuditDate - nextDate := time.Date(currentReviewDate.Year(), currentReviewDate.Month()+time.Month(numMonths), dayOfMonth, - 0, 0, 0, 0, time.UTC) - - return nextDate -} - // AccessListReviews defines an interface for managing Access List reviews. type AccessListReviews interface { // ListAccessListReviews will list access list reviews for a particular access list. diff --git a/lib/services/access_list_test.go b/lib/services/access_list_test.go index 1aaeec143cd08..a144f33625e8a 100644 --- a/lib/services/access_list_test.go +++ b/lib/services/access_list_test.go @@ -571,61 +571,6 @@ func TestIsAccessListMemberChecker(t *testing.T) { } } -func TestSelectNextReviewDate(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - frequency accesslist.ReviewFrequency - dayOfMonth accesslist.ReviewDayOfMonth - currentReviewDate time.Time - expected time.Time - }{ - { - name: "one month, first day", - frequency: accesslist.OneMonth, - dayOfMonth: accesslist.FirstDayOfMonth, - currentReviewDate: time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC), - expected: time.Date(2023, 2, 1, 0, 0, 0, 0, time.UTC), - }, - { - name: "one month, fifteenth day", - frequency: accesslist.OneMonth, - dayOfMonth: accesslist.FifteenthDayOfMonth, - currentReviewDate: time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC), - expected: time.Date(2023, 2, 15, 0, 0, 0, 0, time.UTC), - }, - { - name: "one month, last day", - frequency: accesslist.OneMonth, - dayOfMonth: accesslist.LastDayOfMonth, - currentReviewDate: time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC), - expected: time.Date(2023, 2, 28, 0, 0, 0, 0, time.UTC), - }, - { - name: "six months, last day", - frequency: accesslist.SixMonths, - dayOfMonth: accesslist.LastDayOfMonth, - currentReviewDate: time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC), - expected: time.Date(2023, 7, 31, 0, 0, 0, 0, time.UTC), - }, - } - - for _, test := range tests { - test := test - t.Run(test.name, func(t *testing.T) { - t.Parallel() - accessList := newAccessList(t) - accessList.Spec.Audit.NextAuditDate = test.currentReviewDate - accessList.Spec.Audit.Recurrence = accesslist.Recurrence{ - Frequency: test.frequency, - DayOfMonth: test.dayOfMonth, - } - require.Equal(t, test.expected, SelectNextReviewDate(accessList)) - }) - } -} - // TestAccessListReviewUnmarshal verifies an access list review resource can be unmarshaled. func TestAccessListReviewUnmarshal(t *testing.T) { expected, err := accesslist.NewReview( diff --git a/lib/services/local/access_list.go b/lib/services/local/access_list.go index 0fb38dac52ccb..285dc0b172a7d 100644 --- a/lib/services/local/access_list.go +++ b/lib/services/local/access_list.go @@ -514,7 +514,7 @@ func (a *AccessListService) CreateAccessListReview(ctx context.Context, review * return trace.Wrap(err) } - nextAuditDate = services.SelectNextReviewDate(accessList) + nextAuditDate = accessList.SelectNextReviewDate() accessList.Spec.Audit.NextAuditDate = nextAuditDate for _, removedMember := range review.Spec.Changes.RemovedMembers {