Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import java.io.Serializable;
import java.time.LocalDateTime;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Unused import after migration.

The LocalDateTime import appears unused after the migration to Date types. Consider removing it to keep the imports clean.

🔧 Proposed fix
 import java.io.Serializable;
-import java.time.LocalDateTime;
 import java.util.Collections;
 import java.util.Date;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import java.time.LocalDateTime;
import java.io.Serializable;
import java.util.Collections;
import java.util.Date;
🤖 Prompt for AI Agents
In
`@jans-fido2/model/src/main/java/io/jans/fido2/model/metric/Fido2MetricsAggregation.java`
at line 17, Remove the unused import of java.time.LocalDateTime from the
Fido2MetricsAggregation class; open the file containing class
Fido2MetricsAggregation and delete the import line referencing LocalDateTime
since the class now uses java.util.Date types and the LocalDateTime symbol is no
longer referenced.

import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
Expand All @@ -40,16 +41,16 @@ public class Fido2MetricsAggregation extends Entry implements Serializable {
private String aggregationType; // HOURLY, DAILY, WEEKLY, MONTHLY

@AttributeName(name = "jansStartTime")
private LocalDateTime startTime;
private Date startTime;

@AttributeName(name = "jansEndTime")
private LocalDateTime endTime;
private Date endTime;

@AttributeName(name = "jansUniqueUsers")
private Long uniqueUsers;

@AttributeName(name = "jansLastUpdated")
private LocalDateTime lastUpdated;
private Date lastUpdated;

/**
* All metrics data stored as JSON for flexibility
Expand All @@ -65,13 +66,13 @@ public Fido2MetricsAggregation() {
this.metricsData = new HashMap<>();
}

public Fido2MetricsAggregation(String aggregationType, String period, LocalDateTime startTime, LocalDateTime endTime) {
public Fido2MetricsAggregation(String aggregationType, String period, Date startTime, Date endTime) {
this();
this.aggregationType = aggregationType;
this.id = aggregationType + "_" + period;
this.startTime = startTime;
this.endTime = endTime;
this.lastUpdated = LocalDateTime.now();
this.lastUpdated = new Date();
}
Comment on lines +69 to 76
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider UTC consistency for lastUpdated initialization.

The constructor uses new Date() for lastUpdated, which uses the system default timezone. Other parts of the PR explicitly use UTC (e.g., ZonedDateTime.now(ZoneId.of("UTC"))). For consistency, consider initializing with an explicit UTC-based date.

♻️ Proposed fix
+import java.time.Instant;
...
 public Fido2MetricsAggregation(String aggregationType, String period, Date startTime, Date endTime) {
     this();
     this.aggregationType = aggregationType;
     this.id = aggregationType + "_" + period;
     this.startTime = startTime;
     this.endTime = endTime;
-    this.lastUpdated = new Date();
+    this.lastUpdated = Date.from(Instant.now()); // Instant.now() is always UTC
 }
🤖 Prompt for AI Agents
In
`@jans-fido2/model/src/main/java/io/jans/fido2/model/metric/Fido2MetricsAggregation.java`
around lines 69 - 76, The Fido2MetricsAggregation(String aggregationType, String
period, Date startTime, Date endTime) constructor sets lastUpdated via new
Date() which uses system default timezone; change it to create a UTC-based Date
(e.g., assign lastUpdated = Date.from(Instant.now()) or
Date.from(ZonedDateTime.now(ZoneId.of("UTC")).toInstant())) so lastUpdated is
consistently UTC; update imports as needed and keep the assignment located in
the same constructor.


// Core getters and setters
Expand Down Expand Up @@ -102,19 +103,19 @@ public String getPeriod() {
return id;
}

public LocalDateTime getStartTime() {
public Date getStartTime() {
return startTime;
}

public void setStartTime(LocalDateTime startTime) {
public void setStartTime(Date startTime) {
this.startTime = startTime;
}

public LocalDateTime getEndTime() {
public Date getEndTime() {
return endTime;
}

public void setEndTime(LocalDateTime endTime) {
public void setEndTime(Date endTime) {
this.endTime = endTime;
}

Expand All @@ -126,11 +127,11 @@ public void setUniqueUsers(Long uniqueUsers) {
this.uniqueUsers = uniqueUsers;
}

public LocalDateTime getLastUpdated() {
public Date getLastUpdated() {
return lastUpdated;
}

public void setLastUpdated(LocalDateTime lastUpdated) {
public void setLastUpdated(Date lastUpdated) {
this.lastUpdated = lastUpdated;
}

Expand Down Expand Up @@ -278,7 +279,16 @@ private Map<String, Long> getMapMetric(String key) {
}
Object value = metricsData.get(key);
if (value instanceof Map) {
return (Map<String, Long>) value;
Map<String, Object> rawMap = (Map<String, Object>) value;
// Convert Integer values to Long for JSON serialization compatibility
Map<String, Long> resultMap = new HashMap<>();
for (Map.Entry<String, Object> entry : rawMap.entrySet()) {
Object val = entry.getValue();
if (val instanceof Number) {
resultMap.put(entry.getKey(), ((Number) val).longValue());
}
}
return resultMap;
}
return Collections.emptyMap();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ private Fido2MetricsConstants() {
public static final String MATURE = "MATURE";

// Database Attributes
public static final String JANS_TIMESTAMP = "jansTimestamp";
public static final String JANS_TIMESTAMP = "jansFido2MetricsTimestamp";

// Service Names
public static final String METRICS_SERVICE = "metricsService";
Expand Down Expand Up @@ -108,6 +108,10 @@ private Fido2MetricsConstants() {
public static final String PREVIOUS_PERIOD = "previousPeriod";
public static final String COMPARISON = "comparison";

// Query parameter names
public static final String PARAM_START_TIME = "startTime";
public static final String PARAM_END_TIME = "endTime";

// Fallback method constants
public static final String FALLBACK_METHOD_PASSWORD = "PASSWORD";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

import java.io.Serializable;
import java.time.LocalDateTime;
import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.util.Map;

/**
Expand Down Expand Up @@ -100,7 +102,8 @@ public class Fido2MetricsData implements Serializable {
private Double cpuUsagePercent;

public Fido2MetricsData() {
this.timestamp = LocalDateTime.now();
// Use UTC timezone to align with existing FIDO2 services
this.timestamp = ZonedDateTime.now(ZoneId.of("UTC")).toLocalDateTime();
Comment on lines +105 to +106
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

UTC initialization is correct, but consider consistency with other models.

The UTC-aligned timestamp initialization is appropriate. However, there's an inconsistency: Fido2MetricsAggregation uses Date types for timestamps while this class uses LocalDateTime. The service layer's convertToDate() method handles the conversion, but consider aligning the timestamp types across models for long-term maintainability.

🤖 Prompt for AI Agents
In
`@jans-fido2/model/src/main/java/io/jans/fido2/model/metric/Fido2MetricsData.java`
around lines 105 - 106, Fido2MetricsData currently sets timestamp as
LocalDateTime (ZonedDateTime.now(ZoneId.of("UTC")).toLocalDateTime()), while
Fido2MetricsAggregation uses java.util.Date; to make models consistent, change
Fido2MetricsData to store timestamps as Date (UTC) instead of LocalDateTime and
initialize it using Date.from(Instant.now()) or equivalent in the constructor;
update any getters/setters in Fido2MetricsData to use Date and ensure the
service layer method convertToDate() is no longer required for this conversion
(or adjust its usage) so both Fido2MetricsData and Fido2MetricsAggregation share
the same timestamp type.

}

// Getters and Setters
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
package io.jans.fido2.model.metric;

import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonProperty;
import io.jans.orm.annotation.AttributeName;
import io.jans.orm.annotation.DataEntry;
Expand All @@ -15,7 +16,9 @@
import io.jans.orm.model.base.Entry;

import java.io.Serializable;
import java.time.LocalDateTime;
import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.util.Date;
import java.util.Map;
import java.util.Objects;

Expand All @@ -27,70 +30,71 @@
@DataEntry
@ObjectClass("jansFido2MetricsEntry")
@JsonIgnoreProperties(ignoreUnknown = true)
@JsonInclude(JsonInclude.Include.NON_NULL)
public class Fido2MetricsEntry extends Entry implements Serializable {

private static final long serialVersionUID = 1L;

@AttributeName(name = "jansId")
@AttributeName(name = "jansFido2MetricsId")
private String id;

@AttributeName(name = "jansMetricType")
@AttributeName(name = "jansFido2MetricsType")
private String metricType;

@AttributeName(name = "jansTimestamp")
private LocalDateTime timestamp;
@AttributeName(name = "jansFido2MetricsTimestamp")
private Date timestamp;

@AttributeName(name = "jansUserId")
@AttributeName(name = "jansFido2MetricsUserId")
private String userId;

@AttributeName(name = "jansUsername")
@AttributeName(name = "jansFido2MetricsUsername")
private String username;

@AttributeName(name = "jansOperationType")
@AttributeName(name = "jansFido2MetricsOperationType")
private String operationType; // REGISTRATION, AUTHENTICATION, FALLBACK

@AttributeName(name = "jansStatus")
@AttributeName(name = "jansFido2MetricsStatus")
private String status; // SUCCESS, FAILURE, ATTEMPT

@AttributeName(name = "jansDurationMs")
@AttributeName(name = "jansFido2MetricsDuration")
private Long durationMs;

@AttributeName(name = "jansAuthenticatorType")
@AttributeName(name = "jansFido2MetricsAuthenticatorType")
private String authenticatorType; // PLATFORM, CROSS_PLATFORM, SECURITY_KEY

@AttributeName(name = "jansDeviceInfo")
@AttributeName(name = "jansFido2MetricsDeviceInfo")
@JsonObject
private DeviceInfo deviceInfo;

@AttributeName(name = "jansErrorReason")
@AttributeName(name = "jansFido2MetricsErrorReason")
private String errorReason;

@AttributeName(name = "jansErrorCategory")
@AttributeName(name = "jansFido2MetricsErrorCategory")
private String errorCategory;

@AttributeName(name = "jansFallbackMethod")
@AttributeName(name = "jansFido2MetricsFallbackMethod")
private String fallbackMethod;

@AttributeName(name = "jansFallbackReason")
@AttributeName(name = "jansFido2MetricsFallbackReason")
private String fallbackReason;

@AttributeName(name = "jansUserAgent")
@AttributeName(name = "jansFido2MetricsUserAgent")
private String userAgent;

@AttributeName(name = "jansIpAddress")
@AttributeName(name = "jansFido2MetricsIpAddress")
private String ipAddress;

@AttributeName(name = "jansSessionId")
@AttributeName(name = "jansFido2MetricsSessionId")
private String sessionId;

@AttributeName(name = "jansAdditionalData")
@AttributeName(name = "jansFido2MetricsAdditionalData")
@JsonObject
private transient Map<String, Object> additionalData;
Comment on lines +90 to 92
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

transient field with @JsonObject won't persist.

Same issue as in Fido2UserMetrics - the additionalData field is marked transient but has @JsonObject for ORM. This combination will prevent persistence of this field.

🔧 Suggested fix
     `@AttributeName`(name = "jansFido2MetricsAdditionalData")
     `@JsonObject`
-    private transient Map<String, Object> additionalData;
+    private Map<String, Object> additionalData;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@AttributeName(name = "jansFido2MetricsAdditionalData")
@JsonObject
private transient Map<String, Object> additionalData;
`@AttributeName`(name = "jansFido2MetricsAdditionalData")
`@JsonObject`
private Map<String, Object> additionalData;
🤖 Prompt for AI Agents
In
`@jans-fido2/model/src/main/java/io/jans/fido2/model/metric/Fido2MetricsEntry.java`
around lines 90 - 92, The field additionalData in class Fido2MetricsEntry is
marked transient while annotated with `@JsonObject/`@AttributeName, so it will not
be persisted; remove the transient modifier (or otherwise make it non-transient)
on the additionalData field so the ORM can serialize it, ensuring the
`@JsonObject` annotation on additionalData is effective; update the declaration of
additionalData in Fido2MetricsEntry (matching the pattern used in
Fido2UserMetrics) and run tests to confirm persistence.


@AttributeName(name = "jansNodeId")
@AttributeName(name = "jansFido2MetricsNodeId")
private String nodeId;

@AttributeName(name = "jansApplicationType")
@AttributeName(name = "jansFido2MetricsApplicationType")
private String applicationType;
Comment on lines +38 to 98
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, check the file to see the actual attribute names being used
cat -n jans-fido2/model/src/main/java/io/jans/fido2/model/metric/Fido2MetricsEntry.java | sed -n '30,110p'

Repository: JanssenProject/jans

Length of output: 3270


🏁 Script executed:

# Search for LDAP schema or migration-related files
find . -type f \( -name "*.ldif" -o -name "*.schema" -o -name "*migration*" -o -name "*upgrade*" \) | grep -i fido 2>/dev/null | head -20

Repository: JanssenProject/jans

Length of output: 330


🏁 Script executed:

# Check for references to old Fido2Metrics attribute names in the codebase
rg "jansMetric|jansFido2Metric" --type java -c | head -20

Repository: JanssenProject/jans

Length of output: 561


🏁 Script executed:

# Look for git history or comments about migration in related files
find . -path "*fido2*" -name "*.java" | xargs grep -l "migration\|deprecated\|backwards.*compat" 2>/dev/null | head -10

Repository: JanssenProject/jans

Length of output: 45


🏁 Script executed:

# Check the broader LDAP schema context for Fido2 attributes
find . -type f \( -name "*.ldif" -o -name "*.schema" \) | xargs grep -l "fido" -i 2>/dev/null

Repository: JanssenProject/jans

Length of output: 532


🏁 Script executed:

# Check the LDIF schema file for jansFido2Metrics attributes
cat ./jans-linux-setup/jans_setup/templates/jans-fido2/fido2.ldif | grep -A 50 -i "jansFido2Metrics"

Repository: JanssenProject/jans

Length of output: 45


🏁 Script executed:

# Check the upgrade.py script for migration logic
cat ./docker-jans-fido2/scripts/upgrade.py | grep -i "metric\|attribute\|migration" -C 3

Repository: JanssenProject/jans

Length of output: 603


🏁 Script executed:

# Search for references to the old MetricEntry class to understand the old schema
cat -n ./jans-core/model/src/main/java/io/jans/model/metric/ldap/MetricEntry.java | head -60

Repository: JanssenProject/jans

Length of output: 1964


🏁 Script executed:

# Check if there are test data files that show the old vs new attribute names
rg "MetricsEntry|metricType|operationType" --type ldif | head -20

Repository: JanssenProject/jans

Length of output: 91


🏁 Script executed:

# Look for any comments or documentation about the metrics schema
find . -path "*/fido2*" -type f \( -name "*.md" -o -name "*.txt" -o -name "README*" \) | xargs grep -l "metric" -i 2>/dev/null

Repository: JanssenProject/jans

Length of output: 313


🏁 Script executed:

# Read the LDIF schema file for FIDO2
cat ./jans-linux-setup/jans_setup/templates/jans-fido2/fido2.ldif

Repository: JanssenProject/jans

Length of output: 300


🏁 Script executed:

# Search for any historical references or old names of Fido2MetricsEntry
rg "MetricsEntry|Fido2Metric" --type java -B 2 -A 5 | head -50

Repository: JanssenProject/jans

Length of output: 5567


🏁 Script executed:

# Check if the test data LDIF files reference the new attribute names
cat ./jans-linux-setup/jans_setup/templates/test/jans-fido2/data/fido2-device-registration-test-data.ldif | head -100

Repository: JanssenProject/jans

Length of output: 1103


🏁 Script executed:

# Look for any version or changelog information about Fido2MetricsEntry
find . -path "*/fido2*" -type f \( -name "CHANGELOG*" -o -name "*.md" -o -name "*.txt" \) | xargs grep -l "MetricsEntry\|metrics" 2>/dev/null | head -5

Repository: JanssenProject/jans

Length of output: 102


🏁 Script executed:

# Search for git history of the Fido2MetricsEntry file
git log --oneline --follow jans-fido2/model/src/main/java/io/jans/fido2/model/metric/Fido2MetricsEntry.java 2>/dev/null | head -10

Repository: JanssenProject/jans

Length of output: 125


🏁 Script executed:

# Check if there's a schema file that defines the jansFido2Metrics* object class
find . -type f \( -name "*.ldif" -o -name "*.schema" \) -exec grep -l "jansFido2MetricsEntry\|jansFido2Metrics" {} \;

Repository: JanssenProject/jans

Length of output: 45


🏁 Script executed:

# Search for any documentation about Fido2MetricsEntry
find . -type f -name "*.md" -exec grep -l "Fido2MetricsEntry\|Fido2Metrics" {} \; 2>/dev/null

Repository: JanssenProject/jans

Length of output: 45


🏁 Script executed:

# Check the Fido2MetricsService to see how it's being used
rg "Fido2MetricsEntry" jans-fido2/server/src/main/java/io/jans/fido2/service/metric/ -A 5 -B 5 | head -80

Repository: JanssenProject/jans

Length of output: 9526


🏁 Script executed:

# Look at the docuemts.ldif file (note the typo in the filename)
cat ./jans-linux-setup/jans_setup/templates/jans-fido2/docuemts.ldif

Repository: JanssenProject/jans

Length of output: 2437


LDAP schema definition required: Add jansFido2MetricsEntry object class to LDAP schema.

This is a new metrics collection feature, not a breaking change to existing attributes. The jansFido2Metrics* prefixed attributes are newly introduced and have no existing persisted data to migrate. However, the LDAP schema definition for the jansFido2MetricsEntry object class and its attributes needs to be added to the schema files (e.g., jans-linux-setup/jans_setup/templates/jans-fido2/fido2.ldif) before deployment to ensure the LDAP directory properly validates and stores these entries.

🤖 Prompt for AI Agents
In
`@jans-fido2/model/src/main/java/io/jans/fido2/model/metric/Fido2MetricsEntry.java`
around lines 38 - 98, Add an LDAP schema definition for the new Fido2 metrics
model: create an objectClass named jansFido2MetricsEntry and define all
jansFido2Metrics* attributes used by the Fido2MetricsEntry class (e.g.,
jansFido2MetricsId, jansFido2MetricsType, jansFido2MetricsTimestamp,
jansFido2MetricsUserId, jansFido2MetricsUsername, jansFido2MetricsOperationType,
jansFido2MetricsStatus, jansFido2MetricsDuration,
jansFido2MetricsAuthenticatorType, jansFido2MetricsDeviceInfo,
jansFido2MetricsErrorReason, jansFido2MetricsErrorCategory,
jansFido2MetricsFallbackMethod, jansFido2MetricsFallbackReason,
jansFido2MetricsUserAgent, jansFido2MetricsIpAddress, jansFido2MetricsSessionId,
jansFido2MetricsAdditionalData, jansFido2MetricsNodeId,
jansFido2MetricsApplicationType) in the LDAP LDIF schema (the same template
location used for other FIDO2 schemas); specify appropriate syntaxes (e.g.,
distinguishedName/string/Date/Integer/JSON where supported), single- vs
multi-valued constraints, and include the new jansFido2MetricsEntry objectClass
in the appropriate structural/auxiliary category so instances of the
Fido2MetricsEntry Java class can be created and validated by the directory.


// Constructors
Expand All @@ -102,7 +106,8 @@ public Fido2MetricsEntry(String metricType, String operationType, String status)
this.metricType = metricType;
this.operationType = operationType;
this.status = status;
this.timestamp = LocalDateTime.now();
// Use UTC timezone to align with FIDO2 services
this.timestamp = Date.from(ZonedDateTime.now(ZoneId.of("UTC")).toInstant());
}

// Getters and Setters
Expand All @@ -122,11 +127,11 @@ public void setMetricType(String metricType) {
this.metricType = metricType;
}

public LocalDateTime getTimestamp() {
public Date getTimestamp() {
return timestamp;
}

public void setTimestamp(LocalDateTime timestamp) {
public void setTimestamp(Date timestamp) {
this.timestamp = timestamp;
}

Expand Down Expand Up @@ -291,6 +296,7 @@ public String toString() {
* Device information extracted from User-Agent
*/
@JsonIgnoreProperties(ignoreUnknown = true)
@JsonInclude(JsonInclude.Include.NON_NULL)
public static class DeviceInfo implements Serializable {
private static final long serialVersionUID = 1L;

Expand Down
Loading