diff --git a/jans-fido2/server/src/main/java/io/jans/fido2/service/metric/Fido2MetricsService.java b/jans-fido2/server/src/main/java/io/jans/fido2/service/metric/Fido2MetricsService.java index cd0a45a1436..2c9dc912f67 100644 --- a/jans-fido2/server/src/main/java/io/jans/fido2/service/metric/Fido2MetricsService.java +++ b/jans-fido2/server/src/main/java/io/jans/fido2/service/metric/Fido2MetricsService.java @@ -14,6 +14,7 @@ import io.jans.fido2.model.trust.AttestationTrustDiagnostic; import io.jans.as.common.service.common.ApplicationFactory; import io.jans.orm.PersistenceEntryManager; +import io.jans.orm.model.SearchScope; import io.jans.orm.search.filter.Filter; import jakarta.enterprise.context.ApplicationScoped; import jakarta.inject.Inject; @@ -64,6 +65,9 @@ public class Fido2MetricsService { private static final String METRICS_ENTRY_BASE_DN = "ou=fido2-metrics,o=jans"; private static final String METRICS_AGGREGATION_BASE_DN = "ou=fido2-aggregations,o=jans"; + /** Page size for the paged prior-adopters lookup in {@link #getUsersRegisteredBefore}. */ + private static final int PRIOR_ADOPTERS_CHUNK_SIZE = 1000; + // ========== METRICS ENTRY OPERATIONS ========== /** @@ -360,41 +364,97 @@ public void cleanupOldData(int retentionDays) { // ========== ANALYTICS AND REPORTING ========== /** - * Get user adoption metrics + * Get user adoption metrics. + *

+ * "New" and "returning" are decided against every successful registration on record, not just the + * rows inside {@code [startTime, endTime]}: a user only counts as new the first time their + * registration succeeds, and as returning if they had already registered before the window began. + * {@code adoptionRate} is newUsers against the cumulative population of everyone who has ever + * registered as of {@code endTime} — a self-contained figure bounded by how long metrics entries + * are retained, not a rate against the full identity directory. */ public Map getUserAdoptionMetrics(LocalDateTime startTime, LocalDateTime endTime) { List entries = getMetricsEntries(startTime, endTime); - + Map metrics = new HashMap<>(); - - // Total unique users + + // Every user with any activity in this window, regardless of operation or outcome Set uniqueUsers = entries.stream() .map(Fido2MetricsEntry::getUserId) .filter(Objects::nonNull) .collect(Collectors.toSet()); metrics.put(Fido2MetricsConstants.TOTAL_UNIQUE_USERS, uniqueUsers.size()); - // New users (first registration) - Set newUsers = entries.stream() - .filter(e -> Fido2MetricsConstants.REGISTRATION.equals(e.getOperationType()) && Fido2MetricsConstants.SUCCESS.equals(e.getStatus())) + // Users already known to have registered successfully before this window began + Set priorAdopters = getUsersRegisteredBefore(startTime); + + // Registration successes recorded inside this window + Set registeredInWindow = entries.stream() + .filter(e -> Fido2MetricsConstants.REGISTRATION.equals(e.getOperationType()) + && Fido2MetricsConstants.SUCCESS.equals(e.getStatus())) .map(Fido2MetricsEntry::getUserId) .filter(Objects::nonNull) .collect(Collectors.toSet()); + + // New users: this is the first time their registration ever succeeded + Set newUsers = new HashSet<>(registeredInWindow); + newUsers.removeAll(priorAdopters); metrics.put(Fido2MetricsConstants.NEW_USERS, newUsers.size()); - // Returning users + // Returning users: active this window, and already an adopter before it began. + // Computed directly against priorAdopters rather than as uniqueUsers minus newUsers, so a user + // who registers a second passkey and signs in within the same window is still counted here. Set returningUsers = new HashSet<>(uniqueUsers); - returningUsers.removeAll(newUsers); + returningUsers.retainAll(priorAdopters); metrics.put(Fido2MetricsConstants.RETURNING_USERS, returningUsers.size()); - // Adoption rate - if (!uniqueUsers.isEmpty()) { - metrics.put(Fido2MetricsConstants.ADOPTION_RATE, (double) newUsers.size() / uniqueUsers.size()); + // Adoption rate: new users against the cumulative population of everyone who has ever + // registered as of endTime (priorAdopters and newUsers are disjoint by construction). + long cumulativeAdopters = (long) priorAdopters.size() + newUsers.size(); + if (cumulativeAdopters > 0) { + metrics.put(Fido2MetricsConstants.ADOPTION_RATE, (double) newUsers.size() / cumulativeAdopters); + } else { + metrics.put(Fido2MetricsConstants.ADOPTION_RATE, null); } return metrics; } + /** + * Distinct users whose registration succeeded at any point before {@code beforeTime}, searched + * directly against the metrics store rather than derived from the {@code [startTime, endTime]} + * window. Bounded by the metrics retention policy: a user whose only prior registration entry has + * already been cleaned up by {@link #cleanupOldData} will not appear here, and will be reported as + * new again. + */ + private Set getUsersRegisteredBefore(LocalDateTime beforeTime) { + try { + // Strictly before beforeTime, so a registration timestamped exactly at the window's start + // is not counted both as a prior adopter and as part of this window. + Date exclusiveUpperBound = new Date(convertToDate(beforeTime).getTime() - 1); + + Filter filter = Filter.createANDFilter( + Filter.createEqualityFilter("jansFido2MetricsOperationType", Fido2MetricsConstants.REGISTRATION), + Filter.createEqualityFilter("jansFido2MetricsStatus", Fido2MetricsConstants.SUCCESS), + Filter.createLessOrEqualFilter(Fido2MetricsConstants.JANS_TIMESTAMP, exclusiveUpperBound) + ); + + // Paged retrieval: the unpaged findEntries(filter) overload issues a single search that a + // persistence backend enforcing a result-size limit can reject outright, which would + // misclassify every in-window registration as new. Paging in PRIOR_ADOPTERS_CHUNK_SIZE + // batches keeps this working past that limit. + return persistenceEntryManager.findEntries(METRICS_ENTRY_BASE_DN, Fido2MetricsEntry.class, filter, + SearchScope.SUB, null, 0, 0, PRIOR_ADOPTERS_CHUNK_SIZE) + .stream() + .map(Fido2MetricsEntry::getUserId) + .filter(Objects::nonNull) + .collect(Collectors.toSet()); + } catch (Exception e) { + log.error("Failed to retrieve prior registrations before {}: {}", beforeTime, e.getMessage(), e); + return Collections.emptySet(); + } + } + /** * Whether this entry records a ceremony a user was actually present for. *

diff --git a/jans-fido2/server/src/test/java/io/jans/fido2/service/metric/Fido2MetricsServiceTest.java b/jans-fido2/server/src/test/java/io/jans/fido2/service/metric/Fido2MetricsServiceTest.java index 6d93b184010..c069c472386 100644 --- a/jans-fido2/server/src/test/java/io/jans/fido2/service/metric/Fido2MetricsServiceTest.java +++ b/jans-fido2/server/src/test/java/io/jans/fido2/service/metric/Fido2MetricsServiceTest.java @@ -12,6 +12,7 @@ import io.jans.fido2.model.metric.Fido2MetricsData; import io.jans.fido2.model.metric.Fido2MetricsEntry; import io.jans.orm.PersistenceEntryManager; +import io.jans.orm.model.SearchScope; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -32,7 +33,9 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.never; import static org.mockito.Mockito.timeout; @@ -373,6 +376,115 @@ void aggregation_countsDeviceTypesPerCompletedCeremony() { assertEquals(Map.of("PLATFORM", 1L), metrics.get(Fido2MetricsConstants.DEVICE_TYPES)); } + /** + * A user who already registered before the window and only signs in during it must be reported as + * returning, never as new — the prior definition only ever looked at rows inside the window, so an + * established user with no registration activity here fell out of both buckets. + */ + @Test + void getUserAdoptionMetrics_ifUserRegisteredBeforeWindowAndSignsInDuringIt_isReturningNotNew() { + Fido2MetricsEntry signIn = statusEntry(Fido2MetricsConstants.AUTHENTICATION, Fido2MetricsConstants.SUCCESS); + signIn.setUserId("user-1"); + stubAdoptionQueries(List.of(signIn), List.of("user-1")); + + Map metrics = adoption(); + + assertEquals(1, metrics.get(Fido2MetricsConstants.TOTAL_UNIQUE_USERS)); + assertEquals(0, metrics.get(Fido2MetricsConstants.NEW_USERS)); + assertEquals(1, metrics.get(Fido2MetricsConstants.RETURNING_USERS)); + } + + /** + * The previous formula measured adoption against uniqueUsers active in the window, which shrinks + * as new registrations taper off — so it reported near-zero adoption exactly when adoption + * finished. A user with no prior registration record is new the first time they register, + * regardless of what else happens in the window. + */ + @Test + void getUserAdoptionMetrics_ifUserHasNoPriorRegistration_firstSuccessIsNew() { + Fido2MetricsEntry registration = statusEntry(Fido2MetricsConstants.REGISTRATION, Fido2MetricsConstants.SUCCESS); + registration.setUserId("user-2"); + stubAdoptionQueries(List.of(registration), List.of()); + + Map metrics = adoption(); + + assertEquals(1, metrics.get(Fido2MetricsConstants.NEW_USERS)); + assertEquals(0, metrics.get(Fido2MetricsConstants.RETURNING_USERS)); + assertEquals(1.0, (Double) metrics.get(Fido2MetricsConstants.ADOPTION_RATE), 0.0001); + } + + /** + * Enrolling a second passkey must not make an already-adopted user look new again — the old + * "newUsers" filter only checked REGISTRATION + SUCCESS inside the window, with no check for a + * prior registration, so this changed meaning with the date picker. + */ + @Test + void getUserAdoptionMetrics_ifUserEnrolsSecondPasskey_isNotCountedAsNewAgain() { + Fido2MetricsEntry secondRegistration = statusEntry(Fido2MetricsConstants.REGISTRATION, Fido2MetricsConstants.SUCCESS); + secondRegistration.setUserId("user-3"); + stubAdoptionQueries(List.of(secondRegistration), List.of("user-3")); + + Map metrics = adoption(); + + assertEquals(0, metrics.get(Fido2MetricsConstants.NEW_USERS)); + assertEquals(1, metrics.get(Fido2MetricsConstants.RETURNING_USERS)); + } + + /** + * adoptionRate is newUsers against everyone who has ever registered as of the end of the window + * (prior adopters plus this window's new ones) rather than against uniqueUsers active in the + * window, so it keeps meaning "share of all adopters that are new" instead of falling toward zero + * as adoption succeeds. + */ + @Test + void getUserAdoptionMetrics_adoptionRateIsAgainstCumulativeAdoptersNotWindowActivity() { + Fido2MetricsEntry newUser = statusEntry(Fido2MetricsConstants.REGISTRATION, Fido2MetricsConstants.SUCCESS); + newUser.setUserId("user-new"); + stubAdoptionQueries(List.of(newUser), List.of("user-old-1", "user-old-2", "user-old-3")); + + Map metrics = adoption(); + + // 1 new user out of 4 cumulative adopters (3 prior + this 1 new) + assertEquals(0.25, (Double) metrics.get(Fido2MetricsConstants.ADOPTION_RATE), 0.0001); + } + + /** + * With nobody ever having registered, there is no population to measure adoption against — the + * rate must be left unknown rather than published as a misleading 0.0. + */ + @Test + void getUserAdoptionMetrics_ifNoOneHasEverRegistered_adoptionRateIsNull() { + stubAdoptionQueries(List.of(), List.of()); + + assertNull(adoption().get(Fido2MetricsConstants.ADOPTION_RATE)); + } + + private Map adoption() { + return fido2MetricsService.getUserAdoptionMetrics(LocalDateTime.now().minusDays(1), LocalDateTime.now()); + } + + /** + * getUserAdoptionMetrics issues two distinct queries against the same store: one for activity + * inside the window, and one for registrations that succeeded before it began. The window query + * carries no status filter, so that is what distinguishes the two for stubbing purposes. + */ + private void stubAdoptionQueries(List windowEntries, List priorAdopterUserIds) { + when(persistenceEntryManager.findEntries(any(String.class), eq(Fido2MetricsEntry.class), + argThat(filter -> filter == null || !filter.toString().contains("jansFido2MetricsStatus")))) + .thenReturn(windowEntries); + + List priorEntries = priorAdopterUserIds.stream().map(userId -> { + Fido2MetricsEntry entry = statusEntry(Fido2MetricsConstants.REGISTRATION, Fido2MetricsConstants.SUCCESS); + entry.setUserId(userId); + return entry; + }).collect(java.util.stream.Collectors.toList()); + + when(persistenceEntryManager.findEntries(any(String.class), eq(Fido2MetricsEntry.class), + argThat(filter -> filter != null && filter.toString().contains("jansFido2MetricsStatus")), + any(SearchScope.class), any(), anyInt(), anyInt(), anyInt())) + .thenReturn(priorEntries); + } + private Map performance() { return fido2MetricsService.getPerformanceMetrics(LocalDateTime.now().minusDays(1), LocalDateTime.now()); }