Implement passkey authentication and enhance security features by aalmada · Pull Request #31 · aalmada/BookStore

aalmada · 2026-02-15T12:15:38Z

Description

Implement passkey authentication and registration flow, enhancing security with tenant validation and conflict detection. Introduce critical security checks and logging for passkey operations.

Type of Change

New feature (non-breaking change which adds functionality)

Related Issues

Fixes #

Changes Made

Added passkey authentication and registration flow.
Enhanced security features for passkey operations.
Implemented logging for security violations and tenant validation.

Testing

Unit tests pass (dotnet test)
Build succeeds (dotnet build)
Code formatting is correct (dotnet format --verify-no-changes)
No new analyzer warnings
Manual testing completed

Checklist

My code follows the project's coding standards
I have performed a self-review of my code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published

Screenshots (if applicable)

Additional Notes

- Added login page structure with navigation and buttons for login and registration. - Enhanced PasskeyEndpoints to handle passkey registration and login with improved error handling and tenant isolation. - Updated ApplicationServicesExtensions to configure passkey options including user verification and attestation preferences. - Modified Login.razor and Register.razor to support passkey login and registration, including validation and error handling. - Implemented ManagePasskeys.razor to manage user passkeys with proper credential handling. - Introduced PasskeyService to manage passkey creation and login options, returning structured payloads. - Updated JavaScript functions for WebAuthn to handle passkey creation and login with improved compatibility. - Added unit tests for passkey tenant isolation and validation of passkey operations.

- Implemented critical security checks for cross-tenant token theft and cloned authenticators in JwtAuthenticationEndpoints and PasskeyEndpoints. - Added logging for security violations including cross-tenant token theft and passkey counter mismatches. - Updated JWT token service to include security stamp in tokens and validate it on each request. - Introduced new error codes for security violations and passkey counter mismatches. - Created integration tests for passkey security features, including account lockout on cloned authenticator detection and token invalidation on security stamp changes. - Refactored passkey-related test helpers for better code reuse and clarity. - Ensured that refresh tokens are cleared upon passkey login to enhance security.

…d conflict detection

…process

github-actions · 2026-02-15T12:17:18Z

Package	Line Rate	Branch Rate	Health
JasperFx	0%	0%	❌
BookStore.ApiService	11%	13%	❌
BookStore.Shared	16%	10%	❌
BookStore.Shared	17%	38%	❌
Summary	11% (1368 / 22305)	22% (590 / 10003)	❌

Copilot

Pull request overview

Adds and hardens passkey (WebAuthn) flows across the API, Blazor web app, and integration tests, including tenant isolation, token revocation via security-stamp validation, and security-violation handling.

Changes:

Implement/adjust passkey registration + login flows, including discoverable-credential login options and frontend JSON handling.
Add security controls: tenant validation, passkey counter mismatch handling, cross-tenant refresh-token theft detection, and security-stamp claim/validation.
Add/expand integration tests for tenant isolation and security behaviors; refactor test helpers for Marten setup and passkey manipulation.

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 14 comments.

Show a summary per file

File	Description
tests/BookStore.AppHost.Tests/TestHelpers.cs	Adds Marten store/session helpers and register+login helper for tests.
tests/BookStore.AppHost.Tests/PasswordManagementTests.cs	Adjusts password removal verification to query DB directly (token invalidation).
tests/BookStore.AppHost.Tests/PasskeyTests.cs	Updates expectation for assertion-options endpoint to always return options (anti-enumeration).
tests/BookStore.AppHost.Tests/PasskeyTestHelpers.cs	New shared helpers for adding/updating passkeys in DB for integration tests.
tests/BookStore.AppHost.Tests/PasskeyTenantIsolationTests.cs	New tenant-scoping tests for passkey list/delete and creation options.
tests/BookStore.AppHost.Tests/PasskeySecurityTests.cs	New tests for security-stamp revocation, token theft detection, refresh-token clearing, sign-count scenarios.
tests/BookStore.AppHost.Tests/PasskeyRegistrationSecurityTests.cs	New tests for concurrent registration + user-id conflict masking.
tests/BookStore.AppHost.Tests/PasskeyDeletionTests.cs	Refactors DB setup using new helpers; verifies unsafe-id deletion.
src/BookStore.Web/wwwroot/js/passkeys.js	Uses `PublicKeyCredential.parse*FromJSON` when available; improves wrapped-response handling and transports capture.
src/BookStore.Web/Services/PasskeyService.cs	Returns typed payload for creation options (options JSON + userId), normalizes login options JSON serialization.
src/BookStore.Web/Program.cs	Custom DI for `IPasskeyClient` with cookie persistence for ceremony state.
src/BookStore.Web/Components/Pages/Register.razor	Switches to typed creation-options payload (no manual JSON parsing).
src/BookStore.Web/Components/Pages/ManagePasskeys.razor	Switches to typed creation-options payload.
src/BookStore.Web/Components/Pages/Login.razor	Passkey login UX tweaks (optional email, better aria label, error handling).
src/BookStore.Shared/Models/ErrorCodes.cs	Adds security-violation and counter-mismatch error codes.
src/BookStore.ApiService/Services/JwtTokenService.cs	Adds `security_stamp` claim to access tokens.
src/BookStore.ApiService/Infrastructure/Logging/Log.Users.cs	Adds structured logs for passkey and cross-tenant security events.
src/BookStore.ApiService/Infrastructure/Identity/MartenUserStore.cs	Refactors some lookups; adds (currently ineffective) tenant-isolation asserts.
src/BookStore.ApiService/Infrastructure/Extensions/ApplicationServicesExtensions.cs	Tightens passkey options, origin validation, JWT secret validation, and adds security-stamp validation hook.
src/BookStore.ApiService/Endpoints/PasskeyEndpoints.cs	Adds tenant validation, conflict detection before attestation, counter checks, refresh-token clearing, user-profile stream initialization.
src/BookStore.ApiService/Endpoints/JwtAuthenticationEndpoints.cs	Locks account + clears tokens on cross-tenant refresh-token usage.
login-page.md	Adds captured login-page accessibility snapshot/output.
AGENTS.md	Updates repo structure notes + requires `dotnet format`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-15T12:23:46Z

src/BookStore.ApiService/Infrastructure/Identity/MartenUserStore.cs

+        // Defense-in-depth: Validate tenant isolation
+        // Marten's session should already filter by tenant_id, but verify for security
+        if (user != null)
+        {
+            System.Diagnostics.Debug.Assert(
+                _session.TenantId == _session.TenantId,
+                "Tenant isolation violation detected in FindByEmailAsync");
+        }


This assert has the same issue (_session.TenantId == _session.TenantId): it can never fail and will emit a warning that breaks the build. Remove it or implement a real tenant-isolation check.

Suggested change

// Defense-in-depth: Validate tenant isolation

// Marten's session should already filter by tenant_id, but verify for security

if (user != null)

{

System.Diagnostics.Debug.Assert(

_session.TenantId == _session.TenantId,

"Tenant isolation violation detected in FindByEmailAsync");

}

// Defense-in-depth: tenant isolation is enforced by Marten's session tenant filtering.

// If additional checks are needed, they should compare the user's tenant to the session tenant.

Copilot · 2026-02-15T12:23:46Z

src/BookStore.ApiService/Infrastructure/Extensions/ApplicationServicesExtensions.cs

+        var secretKey = jwtSettings["SecretKey"];
+        var environment = services.BuildServiceProvider().GetRequiredService<Microsoft.AspNetCore.Hosting.IWebHostEnvironment>();
+


Avoid calling services.BuildServiceProvider() during service registration; it creates a second container (duplicated singletons, wrong scoped lifetimes, potential leaks). Prefer passing IWebHostEnvironment into this method (from the caller) or performing this validation via options validation/hosted service after the real provider is built.

Copilot · 2026-02-15T12:23:46Z

src/BookStore.ApiService/Infrastructure/Identity/MartenUserStore.cs

+        if (user != null)
+        {
+            System.Diagnostics.Debug.Assert(
+                _session.TenantId == _session.TenantId,


Same issue here: _session.TenantId == _session.TenantId is always true and will generate a warning (treated as error). Remove it or replace it with an actual tenant-isolation validation for passkey lookups.

Suggested change

_session.TenantId == _session.TenantId,

user.TenantId == _session.TenantId,

Copilot · 2026-02-15T12:23:47Z

tests/BookStore.AppHost.Tests/PasskeyRegistrationSecurityTests.cs

+
+        // Act - Simulate concurrent registration attempts with the SAME user ID
+        // This simulates a race condition where two clients try to register at the same time
+        var registrationResults = new List<(HttpStatusCode StatusCode, string Content)>();


registrationResults is declared but never used. With TreatWarningsAsErrors=true this will fail the build; remove it or use it (e.g., to capture results instead of relying only on results).

Suggested change

var registrationResults = new List<(HttpStatusCode StatusCode, string Content)>();

Copilot · 2026-02-15T12:23:47Z

tests/BookStore.AppHost.Tests/PasskeyTenantIsolationTests.cs

+using System.IdentityModel.Tokens.Jwt;
+using System.Linq;
+using System.Net;
+using System.Net.Http.Json;
+using BookStore.ApiService.Models;


Unused using directives here (e.g., System.IdentityModel.Tokens.Jwt and BookStore.ApiService.Models) will produce warnings that fail the build under TreatWarningsAsErrors=true. Remove unused usings or stop fully-qualifying JwtSecurityTokenHandler if you keep the import.

Copilot · 2026-02-15T12:23:48Z

tests/BookStore.AppHost.Tests/PasskeyTestHelpers.cs

+using JasperFx;
+using Marten;
+using Microsoft.AspNetCore.Identity;
+using Weasel.Core;


Unused using directives here (JasperFx, Microsoft.AspNetCore.Identity, Weasel.Core) will produce warnings that fail the build under TreatWarningsAsErrors=true. Remove any usings that aren’t needed.

Suggested change

using JasperFx;

using Marten;

using Microsoft.AspNetCore.Identity;

using Weasel.Core;

using Marten;

Copilot · 2026-02-15T12:23:48Z

tests/BookStore.AppHost.Tests/PasskeySecurityTests.cs

+        var loginResponse1 = await login1.Content.ReadFromJsonAsync<TestHelpers.LoginResponse>();
+
+        // Manually add the same refresh token with a DIFFERENT tenant ID to simulate cross-tenant token theft
+        // In a real scenario, this would require a security breach or bug that allows token reuse across tenants


loginResponse1 is assigned but never used, which will fail the build under TreatWarningsAsErrors=true. Either assert it’s not null / use it in the test, or remove the variable entirely.

Suggested change

var loginResponse1 = await login1.Content.ReadFromJsonAsync<TestHelpers.LoginResponse>();

// Manually add the same refresh token with a DIFFERENT tenant ID to simulate cross-tenant token theft

// In a real scenario, this would require a security breach or bug that allows token reuse across tenants

// Manually add the same refresh token with a DIFFERENT tenant ID to simulate cross-tenant token theft

// In a real scenario, this would require a security breach or bug that allows token reuse across tenants

// In a real scenario, this would require a security breach or bug that allows token reuse across tenants

Copilot · 2026-02-15T12:23:48Z

tests/BookStore.AppHost.Tests/PasskeyTenantIsolationTests.cs

+using Marten;
+using Refit;
+using Weasel.Core;


using Marten; / using Weasel.Core; appear unused in this test file and will fail the build with warnings-as-errors. Please remove unused usings.

Suggested change

using Marten;

using Refit;

using Weasel.Core;

using Refit;

Copilot · 2026-02-15T12:23:48Z

tests/BookStore.AppHost.Tests/PasskeySecurityTests.cs

+
+        // Act - Try to use a passkey with a DECREASING counter (cloned authenticator)
+        var client = TestHelpers.GetUnauthenticatedClient(tenantId);
+        var response = await client.PostAsJsonAsync("/account/assertion/result", new


response is assigned but never used; with warnings treated as errors this will break the build. Either assert on response (e.g., status code) or discard it.

Suggested change

var response = await client.PostAsJsonAsync("/account/assertion/result", new

await client.PostAsJsonAsync("/account/assertion/result", new

Copilot · 2026-02-15T12:23:49Z

tests/BookStore.AppHost.Tests/PasskeySecurityTests.cs

+using BookStore.ApiService.Models;
+using BookStore.Client;
+using BookStore.Shared.Models;
+using JasperFx;
+using Marten;


At least BookStore.ApiService.Models and JasperFx are unused here and will fail the build under warnings-as-errors. Please remove unused usings.

Suggested change

using BookStore.ApiService.Models;

using BookStore.Client;

using BookStore.Shared.Models;

using JasperFx;

using Marten;

using BookStore.Client;

using BookStore.Shared.Models;

aalmada added 4 commits February 14, 2026 22:11

feat: Enhance passkey registration security with tenant validation an…

fd7a941

…d conflict detection

fix: Refactor password visibility toggle and error handling in login …

ac816f1

…process

Copilot AI review requested due to automatic review settings February 15, 2026 12:15

Copilot started reviewing on behalf of aalmada February 15, 2026 12:16 View session

aalmada merged commit aee3d9f into main Feb 15, 2026
8 checks passed

aalmada deleted the fix/passkey branch February 15, 2026 12:20

Copilot AI reviewed Feb 15, 2026

View reviewed changes

		var secretKey = jwtSettings["SecretKey"];
		var environment = services.BuildServiceProvider().GetRequiredService<Microsoft.AspNetCore.Hosting.IWebHostEnvironment>();

	_session.TenantId == _session.TenantId,
	user.TenantId == _session.TenantId,

	var response = await client.PostAsJsonAsync("/account/assertion/result", new
	await client.PostAsJsonAsync("/account/assertion/result", new

Comments

Conversation

aalmada commented Feb 15, 2026

Description

Type of Change

Related Issues

Changes Made

Testing

Checklist

Screenshots (if applicable)

Additional Notes

Uh oh!

github-actions bot commented Feb 15, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant