Make SCIM Access List Reviewable (#62824)

* make scim access list reviewable

* fix tests

* fix tests due to SCIM list becoming reviewable

* fix regressed tests

* update e

* update e

* update e

* Reset e submodule to master
This commit is contained in:
Lion Chen
2026-01-26 16:07:14 +00:00
committed by GitHub
parent f4ed5de156
commit eac340f888
5 changed files with 19 additions and 28 deletions
+2 -3
View File
@@ -190,8 +190,7 @@ const (
// Static Access Lists are supposed to be managed with the IaC tools like Terraform. Audit
// reviews are not supported for them and the ownership is optional.
Static Type = "static"
// SCIM Access Lists are created with the SCIM integration. Audit reviews are not supported
// for them and the ownership is optional.
// SCIM Access Lists are created with the SCIM integration. Ownership is optional.
SCIM Type = "scim"
)
@@ -201,7 +200,7 @@ var AllTypes = []Type{DeprecatedDynamic, Default, Static, SCIM}
// IsReviewable returns true if the AccessList type supports the audit reviews in the web UI.
func (t Type) IsReviewable() bool {
switch t {
case DeprecatedDynamic, Default:
case DeprecatedDynamic, Default, SCIM:
return true
default:
return false
+6 -6
View File
@@ -262,7 +262,7 @@ func TestSelectNextReviewDate(t *testing.T) {
}{
{
name: "one month, first day",
accessListTypes: []Type{Default, DeprecatedDynamic},
accessListTypes: []Type{Default, DeprecatedDynamic, SCIM},
frequency: OneMonth,
dayOfMonth: FirstDayOfMonth,
currentReviewDate: time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC),
@@ -271,7 +271,7 @@ func TestSelectNextReviewDate(t *testing.T) {
},
{
name: "one month, fifteenth day",
accessListTypes: []Type{Default, DeprecatedDynamic},
accessListTypes: []Type{Default, DeprecatedDynamic, SCIM},
frequency: OneMonth,
dayOfMonth: FifteenthDayOfMonth,
currentReviewDate: time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC),
@@ -280,7 +280,7 @@ func TestSelectNextReviewDate(t *testing.T) {
},
{
name: "one month, last day",
accessListTypes: []Type{Default, DeprecatedDynamic},
accessListTypes: []Type{Default, DeprecatedDynamic, SCIM},
frequency: OneMonth,
dayOfMonth: LastDayOfMonth,
currentReviewDate: time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC),
@@ -289,7 +289,7 @@ func TestSelectNextReviewDate(t *testing.T) {
},
{
name: "six months, last day",
accessListTypes: []Type{Default, DeprecatedDynamic},
accessListTypes: []Type{Default, DeprecatedDynamic, SCIM},
frequency: SixMonths,
dayOfMonth: LastDayOfMonth,
currentReviewDate: time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC),
@@ -298,7 +298,7 @@ func TestSelectNextReviewDate(t *testing.T) {
},
{
name: "six months, last day",
accessListTypes: []Type{Static, SCIM, "__test_unknown__"},
accessListTypes: []Type{Static, "__test_unknown__"},
frequency: SixMonths,
dayOfMonth: LastDayOfMonth,
currentReviewDate: time.Time{},
@@ -307,7 +307,7 @@ func TestSelectNextReviewDate(t *testing.T) {
},
{
name: "six months, last day",
accessListTypes: []Type{Static, SCIM, "__test_unknown__"},
accessListTypes: []Type{Static, "__test_unknown__"},
frequency: SixMonths,
dayOfMonth: LastDayOfMonth,
currentReviewDate: time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC),
@@ -395,7 +395,7 @@ func TestConvAccessList(t *testing.T) {
{
name: "audit with only Recurrence.DayOfMonth set",
input: newAccessList(func(al *accesslistv1.AccessList) {
al.Spec.Type = string(accesslist.SCIM)
al.Spec.Type = string(accesslist.Static)
al.Spec.Audit = &accesslistv1.AccessListAudit{
Recurrence: &accesslistv1.Recurrence{
DayOfMonth: accesslistv1.ReviewDayOfMonth_REVIEW_DAY_OF_MONTH_LAST,
@@ -411,7 +411,7 @@ func TestConvAccessList(t *testing.T) {
{
name: "audit with only Recurrence.Frequency and Notifications.Start set",
input: newAccessList(func(al *accesslistv1.AccessList) {
al.Spec.Type = string(accesslist.SCIM)
al.Spec.Type = string(accesslist.Static)
al.Spec.Audit = &accesslistv1.AccessListAudit{
Recurrence: &accesslistv1.Recurrence{
Frequency: accesslistv1.ReviewFrequency_REVIEW_FREQUENCY_ONE_YEAR,
@@ -431,13 +431,13 @@ func TestConvAccessList(t *testing.T) {
{
name: "static-type",
input: newAccessList(func(al *accesslistv1.AccessList) {
al.Spec.Type = string(accesslist.SCIM)
al.Spec.Type = string(accesslist.Static)
}),
},
{
name: "scim-type and zero audit",
name: "static-type and zero audit",
input: newAccessList(func(al *accesslistv1.AccessList) {
al.Spec.Type = string(accesslist.SCIM)
al.Spec.Type = string(accesslist.Static)
al.Spec.Audit = &accesslistv1.AccessListAudit{
NextAuditDate: &timestamppb.Timestamp{},
Recurrence: &accesslistv1.Recurrence{
+4 -4
View File
@@ -387,7 +387,7 @@ func Test_ValidateAccessListWithMembers_audit(t *testing.T) {
t.Run("audit frequency", func(t *testing.T) {
accessList = newAccessList(t, accessListName, clockwork.NewFakeClockAt(time.Now()))
t.Run("must be non-zero for reviewable access lists", func(t *testing.T) {
for _, typ := range []accesslist.Type{accesslist.Default} {
for _, typ := range []accesslist.Type{accesslist.Default, accesslist.SCIM} {
t.Run(string(typ), func(t *testing.T) {
accessList.Spec.Type = typ
accessList.Spec.Audit.Recurrence.Frequency = 0
@@ -397,7 +397,7 @@ func Test_ValidateAccessListWithMembers_audit(t *testing.T) {
}
})
t.Run("can be zero for non-reviewable access lists", func(t *testing.T) {
for _, typ := range []accesslist.Type{accesslist.SCIM, accesslist.Static} {
for _, typ := range []accesslist.Type{accesslist.Static} {
t.Run(string(typ), func(t *testing.T) {
accessList.Spec.Type = typ
accessList.Spec.Audit.Recurrence.Frequency = 0
@@ -425,7 +425,7 @@ func Test_ValidateAccessListWithMembers_audit(t *testing.T) {
t.Run("audit day_of_month", func(t *testing.T) {
accessList = newAccessList(t, accessListName, clockwork.NewFakeClockAt(time.Now()))
t.Run("must be non-zero for reviewable access lists", func(t *testing.T) {
for _, typ := range []accesslist.Type{accesslist.Default} {
for _, typ := range []accesslist.Type{accesslist.Default, accesslist.SCIM} {
t.Run(string(typ), func(t *testing.T) {
accessList.Spec.Type = typ
accessList.Spec.Audit.Recurrence.DayOfMonth = 0
@@ -435,7 +435,7 @@ func Test_ValidateAccessListWithMembers_audit(t *testing.T) {
}
})
t.Run("can be zero for non-reviewable access lists", func(t *testing.T) {
for _, typ := range []accesslist.Type{accesslist.SCIM, accesslist.Static} {
for _, typ := range []accesslist.Type{accesslist.Static} {
t.Run(string(typ), func(t *testing.T) {
accessList.Spec.Type = typ
accessList.Spec.Audit.Recurrence.DayOfMonth = 0
+2 -10
View File
@@ -1544,28 +1544,20 @@ func Test_CreateAccessListReview_FailForNonReviewable(t *testing.T) {
require.NoError(t, err)
service := newAccessListService(t, mem, modulestest.EnterpriseModules())
// Create a couple access lists.
// Create an access lists.
accessList1 := newAccessList(t, "accessList1", clock, withType(accesslist.Static))
accessList2 := newAccessList(t, "accessList2", clock, withType(accesslist.SCIM))
// Create both access lists.
_, err = service.UpsertAccessList(ctx, accessList1)
require.NoError(t, err)
_, err = service.UpsertAccessList(ctx, accessList2)
require.NoError(t, err)
accessList1Review := newAccessListReview(t, accessList1.GetName(), "al1-review")
accessList2Review := newAccessListReview(t, accessList2.GetName(), "al2-review")
// Add access list review.
_, _, err = service.CreateAccessListReview(ctx, accessList1Review)
require.Error(t, err)
require.ErrorContains(t, err, "is not reviewable")
require.True(t, trace.IsBadParameter(err))
_, _, err = service.CreateAccessListReview(ctx, accessList2Review)
require.Error(t, err)
require.ErrorContains(t, err, "is not reviewable")
require.True(t, trace.IsBadParameter(err))
}
func TestAccessListRequiresEqual(t *testing.T) {