dotnet / dotnet/aspnetcore

Review passkey logging and instrumentation

Open
#69,048 1 comment 0 reactions 1 assignee Claimed by @rolandVi View on GitHub
area-identity
Dominant language
C#
Stars
38.4k
Forks
10.9k
Avg merge
2d 6h
Merged PRs (30d)
290

Description

Passkey telemetry has been added piece by piece rather than designed as a whole. I would like to go through it end to end and then decide what to add and what to leave out.

### What exists today

Two metrics cover passkeys, both on the shared `Microsoft.AspNetCore.Identity` meter:

- `aspnetcore.identity.sign_in.authenticate.duration`, tagged `sign_in.type = "passkey"`, recorded by `PasskeySignInAsync`
- `aspnetcore.identity.user.update.duration`, tagged `add_or_update_passkey` and `remove_passkey`

For logging there are four event IDs, two `LogDebug` calls in `SignInManager` for attestation and assertion failure.
`PasskeyHandler`, which does all the WebAuthn verification, logs nothing.

### Gaps

1. **The ceremony itself is not measured.** `PerformPasskeyAttestationAsync` and `PerformPasskeyAssertionAsync` record no metrics. Assertion is covered only indirectly because `PasskeySignInAsync` wraps it. Attestation has no such wrapper, so a failed passkey registration is invisible: `add_or_update_passkey` only fires once attestation has already succeeded.

2. **No failure reason is observable.** `PasskeyHandler` has around 40 distinct `PasskeyException` throw sites, and all of them collapse into one `LogDebug` with the message interpolated as a string. There is no bounded reason value to chart or alert on. `AuthenticationMetrics` and `AuthorizationMetrics` both use a bounded result tag plus `error.type` instead.

3. **Ceremony starts are not recorded.** `MakePasskeyCreationOptionsAsync` and `MakePasskeyRequestOptionsAsync` emit nothing, so there is no way to see how many passkey flows are started versus completed.

4. **The signals API is uninstrumented.**

### Things to check before adding anything

- Adding a ceremony-level metric would double count failed assertions, which already appear in `sign_in.authenticate.duration`. Any new metric has to avoid that.
- Failure messages interpolate user-controlled values such as origin and user handle into the log text. A bounded reason code with separate structured fields is probably a better shape.
- Passkey logs use the `SignInManager` category, so turning passkey logging up turns all sign-in logging up. `Microsoft.AspNetCore.Components.Lifecycle` is precedent for a separate category.
- Part of this review is deciding where extra telemetry is not worth it, and writing that down rather than leaving it implicit.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.