feat(jans-fido2): honour forwarded end-user IP and user agent in passkey metrics - #14805
feat(jans-fido2): honour forwarded end-user IP and user agent in passkey metrics#14805imran-ishaq wants to merge 2 commits into
Conversation
…key metrics Signed-off-by: imran <imranishaq7071@gmail.com>
…rded client context Signed-off-by: imran <imranishaq7071@gmail.com>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|



Prepare
Description
Target issue
Server half only. Populating the headers at the call sites — the person-authentication script, Casa
and
passkeys.xhtml— is #14803, and cannot be in this PR: those modules build against thepublished
jans-fido2-clientartifact, so they cannot compile against the new overloads until thismerges and republishes.
closes #14801
Implementation Details
The FIDO2 endpoints are never called by the browser. The Authorization Server, Casa and the
person-authentication script relay to them over
Fido2ClientFactory, which propagates no headers, sothe only client the server can observe is the calling service. That is why
ipAddressis the VM'sown address and
userAgentisApache-HttpClient/4.5.14 (Java/17.0.20)on every row.Carrying the end user's details across the hop.
ClientContextHeaders(new, inmodelso clientand server share one definition) names
X-Jans-Client-IPandX-Jans-Client-User-Agent.AssertionServiceandAttestationServicegain overloads taking them as@HeaderParam. Theexisting signatures are untouched deliberately: changing them would force Casa, the person
authentication script and the jans-chip demo to move in lockstep with this merge, whereas overloads
let each migrate on its own schedule. Passing null omits the header.
Deciding whether to believe them. A header is a claim, so
MetricServicehonours these only froma caller listed in the new
fido2TrustedClientContextSources, which is empty by default — theheaders are ignored until an operator names a caller. A forwarded address is still run through
isValidIpAddressbefore use, and a malformed one falls back to the request's own address ratherthan being stored.
"*"trusts any caller, for deployments where callers have no stable address.Stopping the device breakdown being fabricated.
DeviceInfoExtractor.determineDeviceTypepreviously matched mobile and tablet markers and let everything else fall through to
DESKTOP,with no unknown branch — so every service-to-service call was recorded as a desktop and the chart
looked plausible rather than empty. It now tests for desktop markers explicitly and reports
UNKNOWNotherwise. The mobile test still runs first, so Android's shared Linux marker does not reclassify
phones as desktops. This is independent of the trust question above and worth having regardless.
There is also a new
extractDeviceInfo(String)overload, since the user agent to parse is now oftennot the one on the request.
Test and Document the changes
Tests — full
jans-fido2suite green, 411 tests, 14 of them new:DeviceInfoExtractorTest(new, 9 tests) — the service-to-service user agent now reportingUNKNOWNinstead ofDESKTOP, real desktop user agents stillDESKTOP, Android notreclassified, null user agent leaving fields unset.
MetricServiceTest(5 added) — forwarded headers honoured from a trusted caller, ignored whenunconfigured, ignored from an untrusted caller, wildcard trust, and a malformed forwarded address
falling back to the request's own.
Static analysis — SpotBugs run locally on
model,clientandserver. This change adds twofindings,
EI_EXPOSE_REP/EI_EXPOSE_REP2on the newListgetter and setter, which is the samefinding every other collection property in
AppConfigurationalready carries; left consistent withthe surrounding bean rather than making one property defensively copy. The module has 121
pre-existing findings of that class and the plugin is declared only in
pluginManagement, so it doesnot gate the build.
Docs — updated in a separate
docs:commit:passkey-telemetry.md(a "Whose device is recorded"section explaining the relay, the new property and the
UNKNOWNdevice type),fido2-server-properties-config.mdand the generatedfido2-properties.md.Please check the below before submitting your PR. The PR will not be merged if there are no commits that start with
docs:to indicate documentation changes or if the below checklist is not selected.