Access list user requirements check before member expiry check. (#35057)

The user requirements check now happens before the member expiry check. Tests
have been added to verify this.

lib/services/access_list.go

@@ -255,18 +255,14 @@ func (a AccessListMembershipChecker) IsAccessListMember(ctx context.Context, ide
 		return trace.Wrap(err)
 	}
 
-	expires := member.Spec.Expires
-	if expires.IsZero() {
-		return nil
+	if !UserMeetsRequirements(identity, accessList.Spec.MembershipRequires) {
+		return trace.AccessDenied("user %s is a member, but does not have the roles or traits required to be a member of this list", username)
 	}
 
-	if !a.clock.Now().Before(expires) {
+	if !member.Spec.Expires.IsZero() && !a.clock.Now().Before(member.Spec.Expires) {
 		return trace.AccessDenied("user %s's membership has expired in the access list", username)
 	}
 
-	if !UserMeetsRequirements(identity, accessList.Spec.MembershipRequires) {
-		return trace.AccessDenied("user %s is a member, but does not have the roles or traits required to be a member of this list", username)
-	}
 	return nil
 }
 

lib/services/access_list_test.go

@@ -416,6 +416,21 @@ func TestIsAccessListMemberChecker(t *testing.T) {
 				require.True(t, trace.IsAccessDenied(err))
 			},
 		},
+		{
+			name: "is member with no expiration and missing roles",
+			identity: tlsca.Identity{
+				Username: member3,
+				Groups:   []string{"mrole1"},
+				Traits: map[string][]string{
+					"mtrait1": {"mvalue1", "mvalue2"},
+					"mtrait2": {"mvalue3", "mvalue4"},
+				},
+			},
+			currentTime: time.Date(2023, 2, 1, 0, 0, 0, 0, time.UTC),
+			errAssertionFunc: func(t require.TestingT, err error, i ...interface{}) {
+				require.True(t, trace.IsAccessDenied(err))
+			},
+		},
 		{
 			name: "is member with missing traits",
 			identity: tlsca.Identity{
@@ -431,6 +446,21 @@ func TestIsAccessListMemberChecker(t *testing.T) {
 				require.True(t, trace.IsAccessDenied(err))
 			},
 		},
+		{
+			name: "is member with no expiration and missing traits",
+			identity: tlsca.Identity{
+				Username: member3,
+				Groups:   []string{"mrole1", "mrole2"},
+				Traits: map[string][]string{
+					"mtrait1": {"mvalue1"},
+					"mtrait2": {"mvalue3"},
+				},
+			},
+			currentTime: time.Date(2023, 2, 1, 0, 0, 0, 0, time.UTC),
+			errAssertionFunc: func(t require.TestingT, err error, i ...interface{}) {
+				require.True(t, trace.IsAccessDenied(err))
+			},
+		},
 	}
 
 	for _, test := range tests {