diff --git a/api/types/accesslist/accesslist.go b/api/types/accesslist/accesslist.go index 9d2457df2eaad..a8b1be890f803 100644 --- a/api/types/accesslist/accesslist.go +++ b/api/types/accesslist/accesslist.go @@ -233,6 +233,22 @@ type Owner struct { MembershipKind string `json:"membership_kind" yaml:"membership_kind"` } +// IsMembershipKindUser returns true if the owner is of kind user. +// All types expect "MEMBERSHIP_KIND_LIST" are treated as "MEMBERSHIP_KIND_USER". +func (o *Owner) IsMembershipKindUser() bool { + return isMembershipKindUser(o.MembershipKind) +} + +func isMembershipKindUser(membershipKind string) bool { + switch membershipKind { + case MembershipKindUnspecified, MembershipKindUser, "": + return true + default: + // In case if MembershipKind was extended. + return false + } +} + // Audit describes the audit configuration for an access list. type Audit struct { // NextAuditDate is the date that the next audit should be performed. diff --git a/api/types/accesslist/member.go b/api/types/accesslist/member.go index eaa9f57807b68..376b1eca19068 100644 --- a/api/types/accesslist/member.go +++ b/api/types/accesslist/member.go @@ -137,3 +137,9 @@ func (a *AccessListMember) Clone() *AccessListMember { func (a *AccessListMember) IsExpired(t time.Time) bool { return !a.Spec.Expires.IsZero() && !t.Before(a.Spec.Expires) } + +// IsUser returns true if the membership kind is User +// All types expect "MEMBERSHIP_KIND_LIST" are treated as "MEMBERSHIP_KIND_USER". +func (a *AccessListMember) IsUser() bool { + return isMembershipKindUser(a.Spec.MembershipKind) +} diff --git a/lib/accesslists/hierarchy.go b/lib/accesslists/hierarchy.go index 4841ff5968c5f..b62e7ac6b2441 100644 --- a/lib/accesslists/hierarchy.go +++ b/lib/accesslists/hierarchy.go @@ -582,6 +582,11 @@ func (s *Hierarchy) validMembership(ctx context.Context, list *accesslist.Access } return false, trace.Wrap(err) } + // Only user membership are valid. It can happen that username overlaps with the access list name. + // So we need to ensure the membership kind always is verified. + if !m.IsUser() { + return false, nil + } if m.IsExpired(s.Clock.Now()) || !UserMeetsRequirements(user, list.GetMembershipRequires()) { return false, nil } @@ -620,6 +625,11 @@ func (s *Hierarchy) validDirectOwner(user types.User, acl *accesslist.AccessList return false } for _, v := range acl.Spec.Owners { + // Only user membership are valid. It can happen that username overlaps with the access list name. + // So we need to ensure the membership kind is verified as user. + if !v.IsMembershipKindUser() { + continue + } if v.Name == user.GetName() { return true } diff --git a/lib/accesslists/hierarchy_user_test.go b/lib/accesslists/hierarchy_user_test.go index 2fccb35bb30c5..a3a23a292f504 100644 --- a/lib/accesslists/hierarchy_user_test.go +++ b/lib/accesslists/hierarchy_user_test.go @@ -365,6 +365,28 @@ func TestGetHierarchyForUser(t *testing.T) { kind: accesslists.RelationshipKindOwner, want: []string{"level2", "root"}, }, + { + name: "user is excluded from owners when user is not an owner and username overlaps with owning Access List name", + state: state{ + mustMakeAccessList("root", withOwnerList("level1")): {}, + mustMakeAccessList("level1"): {}, + }, + user: makeUser("level1"), + start: "root", + kind: accesslists.RelationshipKindOwner, + want: nil, + }, + { + name: "member/access list name and username overlaps", + state: state{ + mustMakeAccessList("root"): {mustCreateMember("level1", withACLMemKind())}, + mustMakeAccessList("level1"): {}, + }, + user: makeUser("level1"), + start: "root", + kind: accesslists.RelationshipKindMember, + want: nil, + }, } for _, tt := range tests {