CR

api/types/semaphore.go

@@ -51,6 +51,10 @@ const SemaphoreKindAccessMonitoringLimiter = "access_monitoring_limiter"
 // session recordings backend.
 const SemaphoreKindUploadCompleter = "upload_completer"
 
+// SemaphoreKindAccessListReminderLimiter is the semaphore kind used by
+// the periodic check which creates access list reminder notifications.
+const SemaphoreKindAccessListReminderLimiter = "access_monitoring_limiter"
+
 // Semaphore represents distributed semaphore concept
 type Semaphore interface {
 	// Resource contains common resource values

lib/auth/auth.go

@@ -6165,8 +6165,32 @@ func (a *Server) CleanupNotifications(ctx context.Context) {
 	}
 }
 
+const (
+	accessListReminderSemaphoreName      = "access-list-reminder-check"
+	accessListReminderSemaphoreMaxLeases = 1
+)
+
 // CreateAccessListReminderNotifications checks if there are any access lists expiring soon and creates notifications to remind their owners if so.
 func (a *Server) CreateAccessListReminderNotifications(ctx context.Context) {
+	// Ensure only one auth server is running this check at a time.
+	_, err := services.AcquireSemaphoreLock(ctx, services.SemaphoreLockConfig{
+		Service: a,
+		Clock:   a.clock,
+		Expiry:  5 * time.Minute,
+		Params: types.AcquireSemaphoreRequest{
+			SemaphoreKind: types.SemaphoreKindAccessListReminderLimiter,
+			SemaphoreName: accessListReminderSemaphoreName,
+			MaxLeases:     accessListReminderSemaphoreMaxLeases,
+			Holder:        a.ServerID,
+		},
+	})
+	if err != nil {
+		a.logger.WarnContext(ctx, "unable to acquire semaphore, will skip this access list reminder check", "server_id", a.ServerID)
+		return
+	}
+
+	now := time.Now()
+
 	// Fetch all access lists
 	var accessLists []*accesslist.AccessList
 	var accessListsPageKey string
@@ -6182,15 +6206,21 @@ func (a *Server) CreateAccessListReminderNotifications(ctx context.Context) {
 		if err != nil {
 			a.logger.WarnContext(ctx, "failed to list access lists for periodic reminder notification check", "error", err)
 		}
-		accessLists = append(accessLists, response...)
+
+		for _, al := range response {
+			daysDiff := int(al.Spec.Audit.NextAuditDate.Sub(now).Hours() / 24)
+			// Only keep access lists that fall within our thresholds in memory
+			if daysDiff <= 15 && daysDiff >= -8 {
+				accessLists = append(accessLists, al)
+			}
+		}
+
 		if nextKey == "" {
 			break
 		}
 		accessListsPageKey = nextKey
 	}
 
-	now := time.Now()
-
 	reminderThresholds := []struct {
 		days                int
 		prefix              string
@@ -6230,7 +6260,7 @@ func (a *Server) CreateAccessListReminderNotifications(ctx context.Context) {
 			continue
 		}
 
-		// Fetch all identifiers for this treshold p refix.
+		// Fetch all identifiers for this treshold prefix.
 		var identifiers []*notificationsv1.UniqueNotificationIdentifier
 		var nextKey string
 		for {
@@ -6246,10 +6276,10 @@ func (a *Server) CreateAccessListReminderNotifications(ctx context.Context) {
 		}
 
 		// Create a map of identifiers for quick lookup
-		identifiersMap := make(map[string]bool)
+		identifiersMap := make(map[string]struct{})
 		for _, id := range identifiers {
 			// id.Spec.UniqueIdentifier is the access list ID
-			identifiersMap[id.Spec.UniqueIdentifier] = true
+			identifiersMap[id.Spec.UniqueIdentifier] = struct{}{}
 		}
 
 		// owners is the combined list of owners for relevant access lists we are creating the notification for.
@@ -6258,7 +6288,7 @@ func (a *Server) CreateAccessListReminderNotifications(ctx context.Context) {
 		// Check for access lists which haven't already been accounted for in a notification
 		var needsNotification bool
 		for _, accessList := range relevantLists {
-			if !identifiersMap[accessList.GetName()] {
+			if _, exists := identifiersMap[accessList.GetName()]; !exists {
 				needsNotification = true
 				// Create a unique identifier for this access list so that we know it has been accounted for.
 				if _, err := a.CreateUniqueNotificationIdentifier(ctx, threshold.prefix, accessList.GetName()); err != nil {
@@ -6271,63 +6301,74 @@ func (a *Server) CreateAccessListReminderNotifications(ctx context.Context) {
 			}
 		}
 		// Deduplicate the owners list.
+		slices.Sort(owners)
 		owners = slices.Compact(owners)
 
 		// Create the notification for this reminder treshold for all relevant owners.
 		if needsNotification {
-			_, err := a.Services.CreateGlobalNotification(ctx, &notificationsv1.GlobalNotification{
-				Spec: &notificationsv1.GlobalNotificationSpec{
-					Matcher: &notificationsv1.GlobalNotificationSpec_ByUsers{
-						ByUsers: &notificationsv1.ByUsers{
-							Users: owners,
-						},
-					},
-					Notification: &notificationsv1.Notification{
-						Spec:    &notificationsv1.NotificationSpec{},
-						SubKind: threshold.notificationSubkind,
-						Metadata: &headerv1.Metadata{
-							Labels: map[string]string{types.NotificationTitleLabel: threshold.title},
-						},
-					},
-				},
-			})
+			err := a.createAccessListReminderNotification(ctx, owners, threshold.notificationSubkind, threshold.title)
 			if err != nil {
 				a.logger.WarnContext(ctx, "Failed to create access list reminder notification", "error", err)
 			}
+		}
+	}
+}
 
-			// Also create a notification for users who have CRUD permissions for access lists. This is because they can also review access lists.
-			_, err = a.Services.CreateGlobalNotification(ctx, &notificationsv1.GlobalNotification{
-				Spec: &notificationsv1.GlobalNotificationSpec{
-					Matcher: &notificationsv1.GlobalNotificationSpec_ByPermissions{
-						ByPermissions: &notificationsv1.ByPermissions{
-							RoleConditions: []*types.RoleConditions{
+// createAccessListReminderNotification is a helper function to create a notification for an access list reminder.
+func (a *Server) createAccessListReminderNotification(ctx context.Context, owners []string, subkind string, title string) error {
+	_, err := a.Services.CreateGlobalNotification(ctx, &notificationsv1.GlobalNotification{
+		Spec: &notificationsv1.GlobalNotificationSpec{
+			Matcher: &notificationsv1.GlobalNotificationSpec_ByUsers{
+				ByUsers: &notificationsv1.ByUsers{
+					Users: owners,
+				},
+			},
+			Notification: &notificationsv1.Notification{
+				Spec:    &notificationsv1.NotificationSpec{},
+				SubKind: subkind,
+				Metadata: &headerv1.Metadata{
+					Labels: map[string]string{types.NotificationTitleLabel: title},
+				},
+			},
+		},
+	})
+	if err != nil {
+		return err
+	}
+
+	// Also create a notification for users who have CRUD permissions for access lists. This is because they can also review access lists.
+	_, err = a.Services.CreateGlobalNotification(ctx, &notificationsv1.GlobalNotification{
+		Spec: &notificationsv1.GlobalNotificationSpec{
+			Matcher: &notificationsv1.GlobalNotificationSpec_ByPermissions{
+				ByPermissions: &notificationsv1.ByPermissions{
+					RoleConditions: []*types.RoleConditions{
+						{
+							Rules: []types.Rule{
 								{
-									Rules: []types.Rule{
-										{
-											Resources: []string{types.KindAccessList},
-											Verbs:     services.RW(),
-										},
-									},
+									Resources: []string{types.KindAccessList},
+									Verbs:     services.RW(),
 								},
 							},
 						},
 					},
-					// Exclude the list of owners so that they don't get a duplicate notification, since we already created a notification for them.
-					ExcludeUsers: owners,
-					Notification: &notificationsv1.Notification{
-						Spec:    &notificationsv1.NotificationSpec{},
-						SubKind: threshold.notificationSubkind,
-						Metadata: &headerv1.Metadata{
-							Labels: map[string]string{types.NotificationTitleLabel: threshold.title},
-						},
-					},
 				},
-			})
-			if err != nil {
-				a.logger.WarnContext(ctx, "Failed to create access list reminder notification", "error", err)
-			}
-		}
+			},
+			// Exclude the list of owners so that they don't get a duplicate notification, since we already created a notification for them.
+			ExcludeUsers: owners,
+			Notification: &notificationsv1.Notification{
+				Spec:    &notificationsv1.NotificationSpec{},
+				SubKind: subkind,
+				Metadata: &headerv1.Metadata{
+					Labels: map[string]string{types.NotificationTitleLabel: title},
+				},
+			},
+		},
+	})
+	if err != nil {
+		return err
 	}
+
+	return nil
 }
 
 // GenerateCertAuthorityCRL generates an empty CRL for the local CA of a given type.

lib/auth/auth_test.go

@@ -4418,8 +4418,8 @@ func TestCreateAccessListReminderNotifications(t *testing.T) {
 		{name: "al-due-2d", dueInDays: 2},
 		{name: "al-overdue-4d", dueInDays: -4},
 		{name: "al-overdue-8d", dueInDays: -8},
-		{name: "al-overdue-2d", dueInDays: -2},
 		{name: "al-due-60d", dueInDays: 60}, // there should be no notification for this one
+		{name: "al-overdue-today", dueInDays: 0},
 	}
 
 	for _, al := range accessLists {