Skip to content

[v18] Add nested AL tests + change IsAccessListMember behaviour #61623

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
hugoShaka merged 1 commit into from
Nov 21, 2025
Merged

[v18] Add nested AL tests + change IsAccessListMember behaviour #61623

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 34 additions & 20 deletions lib/accesslists/hierarchy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,8 +263,9 @@ func IsUserLocked(err error) bool {
return errors.As(err, &userLockedError{})
}

// IsAccessListOwner checks if the given user is the Access List owner. It returns an error matched
// by [IsUserLocked] if the user is locked.
// IsAccessListOwner checks if the given user is the Access List owner.
// It returns an error matched by [IsUserLocked] if the user is locked.
// If the user is not an owner, it returns a trace.AccessDenied error.
func IsAccessListOwner(
ctx context.Context,
user types.User,
Expand Down Expand Up @@ -316,30 +317,34 @@ func IsAccessListOwner(
if owner.MembershipKind == accesslist.MembershipKindList {
ownerAccessList, err := g.GetAccessList(ctx, owner.Name)
if err != nil {
ownershipErr = trace.Wrap(err)
continue
return accesslistv1.AccessListUserAssignmentType_ACCESS_LIST_USER_ASSIGNMENT_TYPE_UNSPECIFIED, trace.Wrap(err)
}
// Since we already verified that the user is not locked, don't provide lockGetter here
membershipType, err := IsAccessListMember(ctx, user, ownerAccessList, g, nil, clock)
if err != nil {
ownershipErr = trace.Wrap(err)
continue
}
if membershipType != accesslistv1.AccessListUserAssignmentType_ACCESS_LIST_USER_ASSIGNMENT_TYPE_UNSPECIFIED {
if !UserMeetsRequirements(user, accessList.Spec.OwnershipRequires) {
ownershipErr = trace.AccessDenied("User '%s' does not meet the ownership requirements for Access List '%s'", user.GetName(), accessList.Spec.Title)
if _, err := IsAccessListMember(ctx, user, ownerAccessList, g, nil, clock); err != nil {
if trace.IsAccessDenied(err) {
ownershipErr = trace.Wrap(err)
continue
}
return accesslistv1.AccessListUserAssignmentType_ACCESS_LIST_USER_ASSIGNMENT_TYPE_INHERITED, nil
return accesslistv1.AccessListUserAssignmentType_ACCESS_LIST_USER_ASSIGNMENT_TYPE_UNSPECIFIED, trace.Wrap(err)
}
if !UserMeetsRequirements(user, accessList.Spec.OwnershipRequires) {
ownershipErr = trace.AccessDenied("User '%s' does not meet the ownership requirements for Access List '%s'", user.GetName(), accessList.Spec.Title)
continue
}
return accesslistv1.AccessListUserAssignmentType_ACCESS_LIST_USER_ASSIGNMENT_TYPE_INHERITED, nil
}
}

if ownershipErr == nil {
ownershipErr = trace.AccessDenied("no ownership path found")
}

return accesslistv1.AccessListUserAssignmentType_ACCESS_LIST_USER_ASSIGNMENT_TYPE_UNSPECIFIED, trace.Wrap(ownershipErr)
}

// IsAccessListMember checks if the given user is the Access List member. It returns an error
// matched by [IsUserLocked] if the user is locked.
// IsAccessListMember checks if the given user is the Access List member.
// It returns an error matched by [IsUserLocked] if the user is locked.
// If the user is not a member, it returns a trace.AccessDenied error.
func IsAccessListMember(
ctx context.Context,
user types.User,
Expand Down Expand Up @@ -375,7 +380,7 @@ func IsAccessListMember(

members, err := fetchMembers(ctx, accessList.GetName(), g)
if err != nil {
return accesslistv1.AccessListUserAssignmentType_ACCESS_LIST_USER_ASSIGNMENT_TYPE_UNSPECIFIED, trace.Wrap(err)
return accesslistv1.AccessListUserAssignmentType_ACCESS_LIST_USER_ASSIGNMENT_TYPE_UNSPECIFIED, trace.Wrap(err, "fetching access list %q members", accessList.GetName())
}

var membershipErr error
Expand All @@ -399,14 +404,19 @@ func IsAccessListMember(
if member.Spec.MembershipKind == accesslist.MembershipKindList {
memberAccessList, err := g.GetAccessList(ctx, member.GetName())
if err != nil {
membershipErr = trace.Wrap(err)
continue
if trace.IsNotFound(err) {
continue
}
return accesslistv1.AccessListUserAssignmentType_ACCESS_LIST_USER_ASSIGNMENT_TYPE_UNSPECIFIED, trace.Wrap(err, "getting access list %q", member.GetName())
}
// Since we already verified that the user is not locked, don't provide lockGetter here
membershipType, err := IsAccessListMember(ctx, user, memberAccessList, g, nil, clock)
if err != nil {
membershipErr = trace.Wrap(err)
continue
if trace.IsAccessDenied(err) {
membershipErr = err
continue
}
return accesslistv1.AccessListUserAssignmentType_ACCESS_LIST_USER_ASSIGNMENT_TYPE_UNSPECIFIED, trace.Wrap(err)
}
if membershipType != accesslistv1.AccessListUserAssignmentType_ACCESS_LIST_USER_ASSIGNMENT_TYPE_UNSPECIFIED {
if !UserMeetsRequirements(user, accessList.Spec.MembershipRequires) {
Expand All @@ -422,6 +432,10 @@ func IsAccessListMember(
}
}

if membershipErr == nil {
membershipErr = trace.AccessDenied("no access path found")
}

return accesslistv1.AccessListUserAssignmentType_ACCESS_LIST_USER_ASSIGNMENT_TYPE_UNSPECIFIED, trace.Wrap(membershipErr)
}

Expand Down
219 changes: 213 additions & 6 deletions lib/accesslists/hierarchy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package accesslists
import (
"context"
"iter"
"slices"
"sort"
"testing"
"time"
Expand Down Expand Up @@ -185,7 +186,7 @@ func TestAccessListHierarchyIsOwner(t *testing.T) {

ownershipType, err := IsAccessListOwner(ctx, stubUserNoRequires, acl4, accessListAndMembersGetter, nil, clock)
require.Error(t, err)
require.ErrorIs(t, err, trace.AccessDenied("User '%s' does not meet the membership requirements for Access List '%s'", member1, acl1.Spec.Title))
require.ErrorAs(t, err, new(*trace.AccessDeniedError))
// Should not have inherited ownership due to missing OwnershipRequires.
require.Equal(t, accesslistv1.AccessListUserAssignmentType_ACCESS_LIST_USER_ASSIGNMENT_TYPE_UNSPECIFIED, ownershipType)

Expand All @@ -200,7 +201,7 @@ func TestAccessListHierarchyIsOwner(t *testing.T) {

ownershipType, err = IsAccessListOwner(ctx, stubUserMeetsMemberRequires, acl4, accessListAndMembersGetter, nil, clock)
require.Error(t, err)
require.ErrorIs(t, err, trace.AccessDenied("User '%s' does not meet the ownership requirements for Access List '%s'", member1, acl4.Spec.Title))
require.ErrorAs(t, err, new(*trace.AccessDeniedError))
require.Equal(t, accesslistv1.AccessListUserAssignmentType_ACCESS_LIST_USER_ASSIGNMENT_TYPE_UNSPECIFIED, ownershipType)

// User which meets acl1's Membership and acl1's Ownership requirements.
Expand All @@ -221,8 +222,9 @@ func TestAccessListHierarchyIsOwner(t *testing.T) {

stubUserMeetsAllRequires.SetName(member2)
ownershipType, err = IsAccessListOwner(ctx, stubUserMeetsAllRequires, acl4, accessListAndMembersGetter, nil, clock)
require.NoError(t, err)
// Should not have ownership.
require.Error(t, err)
require.ErrorAs(t, err, new(*trace.AccessDeniedError))
require.Equal(t, accesslistv1.AccessListUserAssignmentType_ACCESS_LIST_USER_ASSIGNMENT_TYPE_UNSPECIFIED, ownershipType)
}

Expand Down Expand Up @@ -267,7 +269,8 @@ func TestAccessListIsMember(t *testing.T) {
locksGetter.targets[member1] = []types.Lock{lock}

membershipType, err = IsAccessListMember(ctx, stubMember1, acl1, accessListAndMembersGetter, locksGetter, clock)
require.ErrorIs(t, err, trace.AccessDenied("User %q is currently locked", member1))
require.Error(t, err)
require.ErrorAs(t, err, new(*trace.AccessDeniedError))
require.Equal(t, accesslistv1.AccessListUserAssignmentType_ACCESS_LIST_USER_ASSIGNMENT_TYPE_UNSPECIFIED, membershipType)
}

Expand All @@ -290,8 +293,9 @@ func TestAccessListIsMember_RequirementsAndExpiry(t *testing.T) {

// Missing membershipRequires should be AccessDenied
typ, err := IsAccessListMember(ctx, u, acl, aclGetter, locks, clock)
require.Error(t, err)
require.ErrorAs(t, err, new(*trace.AccessDeniedError))
require.Equal(t, accesslistv1.AccessListUserAssignmentType_ACCESS_LIST_USER_ASSIGNMENT_TYPE_UNSPECIFIED, typ)
require.ErrorIs(t, err, trace.AccessDenied("User '%s' does not meet the membership requirements for Access List '%s'", u.GetName(), acl.GetName()))

// Give correct traits/roles, but expire the membership
u.SetRoles([]string{"mrole1", "mrole2"})
Expand All @@ -304,7 +308,210 @@ func TestAccessListIsMember_RequirementsAndExpiry(t *testing.T) {

typ, err = IsAccessListMember(ctx, u, acl, aclGetter, locks, clock)
require.Equal(t, accesslistv1.AccessListUserAssignmentType_ACCESS_LIST_USER_ASSIGNMENT_TYPE_UNSPECIFIED, typ)
require.ErrorIs(t, err, trace.AccessDenied("User '%s's membership in Access List '%s' has expired", u.GetName(), acl.GetName()))
require.Error(t, err)
require.ErrorAs(t, err, new(*trace.AccessDeniedError))
}

func TestAccessListIsMember_NestedRequirements(t *testing.T) {
clock := clockwork.NewFakeClock()
ctx := t.Context()
locks := &mockLocksGetter{}

t.Run("nested lists with requirements at multiple levels", func(t *testing.T) {
rootList := newAccessList(t, "root", clock)
rootList.Spec.MembershipRequires = accesslist.Requires{
Roles: []string{"root-role"},
}

middleList := newAccessList(t, "middle", clock)
middleList.Spec.MembershipRequires = accesslist.Requires{
Roles: []string{"middle-role"},
}

leafList := newAccessList(t, "leaf", clock)
leafList.Spec.MembershipRequires = accesslist.Requires{
Roles: []string{"leaf-role"},
}

const userName = "alice"

userMember := newAccessListMember(t, leafList.GetName(), userName, accesslist.MembershipKindUser, clock)
leafInMiddle := newAccessListMember(t, middleList.GetName(), leafList.GetName(), accesslist.MembershipKindList, clock)
middleInRoot := newAccessListMember(t, rootList.GetName(), middleList.GetName(), accesslist.MembershipKindList, clock)

aclGetter := &mockAccessListAndMembersGetter{
accessLists: map[string]*accesslist.AccessList{
"root": rootList,
"middle": middleList,
"leaf": leafList,
},
members: map[string][]*accesslist.AccessListMember{
"root": {middleInRoot},
"middle": {leafInMiddle},
"leaf": {userMember},
},
}

user, err := types.NewUser(userName)
require.NoError(t, err)
allRoles := slices.Concat(
rootList.Spec.MembershipRequires.Roles,
middleList.Spec.MembershipRequires.Roles,
leafList.Spec.MembershipRequires.Roles,
)
user.SetRoles(allRoles)

typ, err := IsAccessListMember(ctx, user, rootList, aclGetter, locks, clock)
require.NoError(t, err)
require.Equal(t, accesslistv1.AccessListUserAssignmentType_ACCESS_LIST_USER_ASSIGNMENT_TYPE_INHERITED, typ)

typ, err = IsAccessListMember(ctx, user, middleList, aclGetter, locks, clock)
require.NoError(t, err)
require.Equal(t, accesslistv1.AccessListUserAssignmentType_ACCESS_LIST_USER_ASSIGNMENT_TYPE_INHERITED, typ)

typ, err = IsAccessListMember(ctx, user, leafList, aclGetter, locks, clock)
require.NoError(t, err)
require.Equal(t, accesslistv1.AccessListUserAssignmentType_ACCESS_LIST_USER_ASSIGNMENT_TYPE_EXPLICIT, typ)

// User missing middle role
missingMiddleRoles := slices.Concat(
rootList.Spec.MembershipRequires.Roles,
leafList.Spec.MembershipRequires.Roles,
)
user.SetRoles(missingMiddleRoles)

typ, err = IsAccessListMember(ctx, user, rootList, aclGetter, locks, clock)
require.Error(t, err)
require.ErrorAs(t, err, new(*trace.AccessDeniedError))
require.Equal(t, accesslistv1.AccessListUserAssignmentType_ACCESS_LIST_USER_ASSIGNMENT_TYPE_UNSPECIFIED, typ)
})

t.Run("nested lists with expired list membership in the middle", func(t *testing.T) {
rootList := newAccessList(t, "root", clock)
middleList := newAccessList(t, "middle", clock)
leafList := newAccessList(t, "leaf", clock)

const userName = "alice"

userMember := newAccessListMember(t, leafList.GetName(), userName, accesslist.MembershipKindUser, clock)
// middle -> leaf membership expires in 12 hours while the other memberships expire in 24h
leafInMiddle := newAccessListMember(t, middleList.GetName(), leafList.GetName(), accesslist.MembershipKindList, clockwork.NewFakeClockAt(clock.Now().Add(-12*time.Hour)))
middleInRoot := newAccessListMember(t, rootList.GetName(), middleList.GetName(), accesslist.MembershipKindList, clock)

aclGetter := &mockAccessListAndMembersGetter{
accessLists: map[string]*accesslist.AccessList{
"root": rootList,
"middle": middleList,
"leaf": leafList,
},
members: map[string][]*accesslist.AccessListMember{
"root": {middleInRoot},
"middle": {leafInMiddle},
"leaf": {userMember},
},
}

user, err := types.NewUser(userName)
require.NoError(t, err)
user.SetRoles(rootList.Spec.MembershipRequires.Roles)
user.SetTraits(rootList.Spec.MembershipRequires.Traits)

typ, err := IsAccessListMember(ctx, user, rootList, aclGetter, locks, clock)
require.NoError(t, err)
require.Equal(t, accesslistv1.AccessListUserAssignmentType_ACCESS_LIST_USER_ASSIGNMENT_TYPE_INHERITED, typ)

// advancing the clock makes the middle -> leaf membership expire
clock.Advance(14 * time.Hour)

typ, err = IsAccessListMember(ctx, user, rootList, aclGetter, locks, clock)
require.Error(t, err)
require.ErrorAs(t, err, new(*trace.AccessDeniedError))
require.Equal(t, accesslistv1.AccessListUserAssignmentType_ACCESS_LIST_USER_ASSIGNMENT_TYPE_UNSPECIFIED, typ)
})

t.Run("cyclic graph, no membership", func(t *testing.T) {
t.Skip("cyclic graph not supported yet")
firstList := newAccessList(t, "first", clock)
secondList := newAccessList(t, "second", clock)
thirdList := newAccessList(t, "third", clock)

firstArc := newAccessListMember(t, firstList.GetName(), secondList.GetName(), accesslist.MembershipKindList, clock)
secondArc := newAccessListMember(t, secondList.GetName(), thirdList.GetName(), accesslist.MembershipKindList, clock)
thirdArc := newAccessListMember(t, thirdList.GetName(), firstList.GetName(), accesslist.MembershipKindList, clock)

aclGetter := &mockAccessListAndMembersGetter{
accessLists: map[string]*accesslist.AccessList{
firstList.GetName(): firstList,
secondList.GetName(): secondList,
thirdList.GetName(): thirdList,
},
members: map[string][]*accesslist.AccessListMember{
firstList.GetName(): {firstArc},
secondList.GetName(): {secondArc},
thirdList.GetName(): {thirdArc},
},
}

user, err := types.NewUser("alice")
require.NoError(t, err)
// Make sure the user meets the membership requirements.
user.SetRoles(firstList.Spec.MembershipRequires.Roles)
user.SetTraits(firstList.Spec.MembershipRequires.Traits)

typ, err := IsAccessListMember(ctx, user, firstList, aclGetter, locks, clock)
require.Error(t, err)
require.ErrorAs(t, err, new(*trace.AccessDeniedError))
require.Equal(t, accesslistv1.AccessListUserAssignmentType_ACCESS_LIST_USER_ASSIGNMENT_TYPE_UNSPECIFIED, typ)
typ, err = IsAccessListMember(ctx, user, secondList, aclGetter, locks, clock)
require.Error(t, err)
require.ErrorAs(t, err, new(*trace.AccessDeniedError))
require.Equal(t, accesslistv1.AccessListUserAssignmentType_ACCESS_LIST_USER_ASSIGNMENT_TYPE_UNSPECIFIED, typ)
typ, err = IsAccessListMember(ctx, user, thirdList, aclGetter, locks, clock)
require.Error(t, err)
require.ErrorAs(t, err, new(*trace.AccessDeniedError))
require.Equal(t, accesslistv1.AccessListUserAssignmentType_ACCESS_LIST_USER_ASSIGNMENT_TYPE_UNSPECIFIED, typ)
})

t.Run("cyclic graph, user membership", func(t *testing.T) {
t.Skip("cyclic graph not supported yet")
firstList := newAccessList(t, "first", clock)
secondList := newAccessList(t, "second", clock)
thirdList := newAccessList(t, "third", clock)

user, err := types.NewUser("alice")
require.NoError(t, err)
// Make sure the user meets the membership requirements.
user.SetRoles(firstList.Spec.MembershipRequires.Roles)
user.SetTraits(firstList.Spec.MembershipRequires.Traits)

firstArc := newAccessListMember(t, firstList.GetName(), secondList.GetName(), accesslist.MembershipKindList, clock)
secondArc := newAccessListMember(t, secondList.GetName(), thirdList.GetName(), accesslist.MembershipKindList, clock)
thirdArc := newAccessListMember(t, thirdList.GetName(), firstList.GetName(), accesslist.MembershipKindList, clock)
userMembership := newAccessListMember(t, thirdList.GetName(), user.GetName(), accesslist.MembershipKindUser, clock)

aclGetter := &mockAccessListAndMembersGetter{
accessLists: map[string]*accesslist.AccessList{
firstList.GetName(): firstList,
secondList.GetName(): secondList,
thirdList.GetName(): thirdList,
},
members: map[string][]*accesslist.AccessListMember{
firstList.GetName(): {firstArc},
secondList.GetName(): {secondArc},
thirdList.GetName(): {thirdArc, userMembership},
},
}

typ, err := IsAccessListMember(ctx, user, firstList, aclGetter, locks, clock)
require.NoError(t, err)
require.Equal(t, accesslistv1.AccessListUserAssignmentType_ACCESS_LIST_USER_ASSIGNMENT_TYPE_INHERITED, typ)
typ, err = IsAccessListMember(ctx, user, secondList, aclGetter, locks, clock)
require.NoError(t, err)
require.Equal(t, accesslistv1.AccessListUserAssignmentType_ACCESS_LIST_USER_ASSIGNMENT_TYPE_INHERITED, typ)
typ, err = IsAccessListMember(ctx, user, thirdList, aclGetter, locks, clock)
require.NoError(t, err)
require.Equal(t, accesslistv1.AccessListUserAssignmentType_ACCESS_LIST_USER_ASSIGNMENT_TYPE_EXPLICIT, typ)
})
}

func TestGetOwners(t *testing.T) {
Expand Down
Loading