Skip to content

[webauthn] initial support - #32

Merged
capcom6 merged 1 commit into
masterfrom
auth/passkey-support
Jul 14, 2026
Merged

[webauthn] initial support#32
capcom6 merged 1 commit into
masterfrom
auth/passkey-support

Conversation

@capcom6

@capcom6 capcom6 commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • New Features
    • Added passkey/WebAuthn sign-in and registration support.
    • Added automatic passkey sign-in (conditional) when supported.
    • Added Security settings for viewing, adding, renaming, and deleting passkeys.
    • Added a Security settings entry on the profile page and a dedicated authenticated route.
  • Configuration
    • Updated environment template with cache and WebAuthn relying-party configuration variables.
  • Bug Fixes
    • Improved passkey error handling with clearer client-facing status responses.

@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@capcom6, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 57 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ca3ec30b-591c-432a-bae2-a5909aad75bc

📥 Commits

Reviewing files that changed from the base of the PR and between e20abfd and fda8d60.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (29)
  • .env.example
  • .golangci.yml
  • README.md
  • frontend/src/App.svelte
  • frontend/src/lib/api/passkey.ts
  • frontend/src/lib/pages/profile.svelte
  • frontend/src/lib/pages/settings/security.svelte
  • frontend/src/lib/stores/auth.svelte.ts
  • frontend/src/lib/types/api.ts
  • frontend/src/pages/login.svelte
  • go.mod
  • internal/commands/serve/serve.go
  • internal/config/config.go
  • internal/config/module.go
  • internal/db/migrations/20260710000000_webauthn_credentials.sql
  • internal/server/auth/dto.go
  • internal/server/auth/handler.go
  • internal/server/middlewares/jwtauth/jwtauth.go
  • internal/server/module.go
  • internal/server/passkey/dto.go
  • internal/server/passkey/handler.go
  • internal/webauthn/config.go
  • internal/webauthn/domain.go
  • internal/webauthn/errors.go
  • internal/webauthn/models.go
  • internal/webauthn/module.go
  • internal/webauthn/repository.go
  • internal/webauthn/service.go
  • internal/webauthn/sessions.go
📝 Walkthrough

Walkthrough

Adds end-to-end WebAuthn passkey support: configuration, cache-backed sessions, credential persistence, registration/login APIs, JWT integration, and frontend login and security-management interfaces.

Changes

WebAuthn / Passkey Authentication

Layer / File(s) Summary
Configuration and application wiring
.env.example, .golangci.yml, go.mod, internal/config/*, internal/commands/serve/serve.go, internal/webauthn/config.go, internal/webauthn/module.go
Adds cache and relying-party settings, updates dependencies and lint configuration, provides cache/WebAuthn configuration, and registers the required FX modules.
Domain and persistence
internal/webauthn/*, internal/db/migrations/*
Adds credential models, SQL serialization types, cache-backed sessions, repository operations, sentinel errors, and the webauthn_credentials table.
Service and HTTP routes
internal/webauthn/service.go, internal/server/passkey/*, internal/server/..., internal/server/middlewares/*
Implements registration, login, credential management, error-to-status mapping, JWT bypasses, token issuance, and handler registration.
Frontend API and authentication store
frontend/src/lib/api/passkey.ts, frontend/src/lib/stores/auth.svelte.ts, frontend/src/lib/types/api.ts
Adds WebAuthn payload conversion, credential serialization, passkey API calls, and explicit or conditional passkey login flows.
Login and security settings UI
frontend/src/pages/login.svelte, frontend/src/lib/pages/profile.svelte, frontend/src/lib/pages/settings/security.svelte, frontend/src/App.svelte
Adds passkey login controls, security navigation, an authenticated security route, and passkey registration, rename, deletion, and listing views.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

Suggested labels: codex

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 8.82% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title is related to the change, but "initial support" is too vague to convey the main passkey/WebAuthn work. Use a more specific title such as "Add WebAuthn/passkey authentication support" or similar.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot added the codex label Jun 24, 2026
@github-actions

github-actions Bot commented Jun 24, 2026

Copy link
Copy Markdown

🤖 Pull request artifacts

Platform File
🐳 Docker GitHub Container Registry
🍎 Darwin arm64 backend_Darwin_arm64.tar.gz
🍎 Darwin x86_64 backend_Darwin_x86_64.tar.gz
🐧 Linux arm64 backend_Linux_arm64.tar.gz
🐧 Linux i386 backend_Linux_i386.tar.gz
🐧 Linux x86_64 backend_Linux_x86_64.tar.gz
🪟 Windows arm64 backend_Windows_arm64.zip
🪟 Windows i386 backend_Windows_i386.zip
🪟 Windows x86_64 backend_Windows_x86_64.zip

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 11

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@frontend/src/lib/pages/settings/security.svelte`:
- Around line 163-170: The icon-only action buttons in the passkey edit flow are
missing accessible names, so screen readers announce them as unnamed controls.
Update the Button instances in the security.svelte passkey rename/delete
actions, including the save/cancel controls in the edit block and the
delete-related buttons elsewhere in the same flow, by adding clear accessible
labels or equivalent text for each icon-only action. Use the existing Button
component and the surrounding handlers like saveEdit and cancelEdit to locate
and update all affected icon-only controls.

In `@frontend/src/pages/login.svelte`:
- Around line 133-139: The passkey login control in the login form is missing an
explicit button type, so it may submit the surrounding form when clicked. Update
the Button used by handlePasskeyLogin to include type="button" while keeping the
existing onclick and disabled behavior, so WebAuthn starts without triggering a
form submit.

In `@go.mod`:
- Around line 102-103: The go.mod dependencies for go.opentelemetry.io/otel and
go.opentelemetry.io/otel/trace are still pinned to the vulnerable v1.40.0
release. Update both module requirements together to v1.41.0 or later so the
OpenTelemetry packages stay in sync and the patched version is used.

In `@internal/server/auth/handler.go`:
- Around line 224-231: Malformed WebAuthn request payloads from
FinishRegistration and FinishLogin are currently treated as internal errors
instead of bad requests. Update the handler flow in authHandler methods that
call h.waSvc.FinishRegistration and h.waSvc.FinishLogin to surface
request-parse/validation failures as a shared sentinel error, then teach
errorsHandler to recognize that sentinel and return fiber.StatusBadRequest. Keep
the existing empty-body check, but ensure malformed client payloads from the
WebAuthn library no longer fall through as 500s.
- Around line 311-319: The RenamePasskeyRequest validation in auth handler only
rejects empty names, so add a 255-character max check before calling
h.waSvc.RenameCredential. Either extend the existing request validation on
RenamePasskeyRequest with the shared validator max=255 rule or add an explicit
length guard alongside the current name required check, and return a bad request
when the limit is exceeded.
- Around line 312-313: The auth handler has shadowed inner err variables that
trip govet with shadow.strict enabled. In the relevant flow in handler.go,
rename each short-lived error variable used in the json.Unmarshal, rename, and
delete checks to unmarshalErr, renameErr, and deleteErr so they no longer shadow
the outer err from strconv.ParseInt, and update the corresponding conditionals
to use the new names.

In `@internal/webauthn/domain.go`:
- Around line 50-53: The Authenticator reconstruction in domain.go is using the
stored AAGUID string as raw bytes, which produces ASCII bytes instead of the
original 16-byte identifier. Update the credential rebuild logic in the code
that constructs webauthn.Authenticator inside the relevant domain-to-WebAuthn
conversion so it decodes the stored AAGUID string back into its binary UUID form
before assigning it, keeping the round-trip consistent with formatAAGUID and
newWebAuthnUser.

In `@internal/webauthn/module.go`:
- Around line 16-18: The fx provider wrapper is unnecessary and triggers the
unlambda lint; replace the inline anonymous function in module setup with a
direct reference to NewService. Keep the existing dependencies and return type
unchanged so fx.Provide continues to construct the Service through NewService
without the extra lambda.

In `@internal/webauthn/repository.go`:
- Around line 20-23: The Create method in Repository currently wraps all insert
failures as a generic error, so duplicate credential inserts are not mapped to
the package’s ErrDuplicateCredential. Update the insert error handling in
Repository.Create to detect unique-key violations from the credential_id
constraint and return ErrDuplicateCredential in that case, while preserving the
existing generic wrapped error path for all other failures.

In `@internal/webauthn/service.go`:
- Around line 150-162: The sign counter update in the passkey login flow is
incrementing the stored value instead of persisting the authenticator’s returned
counter. In the login path inside the WebAuthn service method that calls
ValidatePasskeyLogin and then UpdateSignCount, use
credential.Authenticator.SignCount from the successful assertion when updating
the repo record, and keep the existing error handling/logging around the repo
call unchanged.

In `@internal/webauthn/session.go`:
- Around line 10-13: The in-memory challenge store in sessionStore is unbounded
and can grow under request floods on the unauthenticated WebAuthn path; add a
fixed capacity/eviction bound to the sessions map and enforce it in the
sessionStore methods that insert and track entries. Update the sessionStore
handling in internal/webauthn/session.go (including the logic around the
sessionEntry lifecycle) so old or excess challenges are evicted or rejected
instead of accumulating indefinitely.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f85d7ceb-ffbc-4fcb-b4fe-fc6b4ebbff58

📥 Commits

Reviewing files that changed from the base of the PR and between a1d0b04 and 3b9d1a3.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (25)
  • .env.example
  • frontend/src/App.svelte
  • frontend/src/lib/api/passkey.ts
  • frontend/src/lib/pages/profile.svelte
  • frontend/src/lib/pages/settings/security.svelte
  • frontend/src/lib/stores/auth.svelte.ts
  • frontend/src/lib/types/api.ts
  • frontend/src/pages/login.svelte
  • go.mod
  • internal/commands/serve/serve.go
  • internal/config/config.go
  • internal/config/module.go
  • internal/db/migrations/20260623000000_webauthn_credentials.sql
  • internal/server/auth/dto.go
  • internal/server/auth/handler.go
  • internal/server/middlewares/jwtauth/jwtauth.go
  • internal/webauthn/config.go
  • internal/webauthn/domain.go
  • internal/webauthn/encoding.go
  • internal/webauthn/errors.go
  • internal/webauthn/models.go
  • internal/webauthn/module.go
  • internal/webauthn/repository.go
  • internal/webauthn/service.go
  • internal/webauthn/session.go

Comment thread frontend/src/lib/pages/settings/security.svelte Outdated
Comment thread frontend/src/pages/login.svelte
Comment thread go.mod Outdated
Comment thread internal/server/auth/handler.go Outdated
Comment thread internal/server/auth/handler.go Outdated
Comment thread internal/webauthn/domain.go
Comment thread internal/webauthn/module.go Outdated
Comment thread internal/webauthn/repository.go
Comment thread internal/webauthn/service.go
Comment thread internal/webauthn/session.go Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/webauthn/session.go`:
- Around line 67-68: Avoid evicting an existing session when updating the same
key in session management. In the session store logic around evictLocked and the
map write in session.go, first check whether the key already exists in
s.sessions and skip eviction for in-place updates; only call evictLocked when
inserting a brand-new key and capacity would otherwise be exceeded.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7d58083d-6c9d-4356-b8da-020666cbed6b

📥 Commits

Reviewing files that changed from the base of the PR and between 8bc5ff6 and 05f82e2.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (11)
  • .golangci.yml
  • frontend/src/lib/pages/settings/security.svelte
  • go.mod
  • internal/server/auth/dto.go
  • internal/server/auth/handler.go
  • internal/webauthn/domain.go
  • internal/webauthn/errors.go
  • internal/webauthn/models.go
  • internal/webauthn/repository.go
  • internal/webauthn/service.go
  • internal/webauthn/session.go
🚧 Files skipped from review as they are similar to previous changes (8)
  • internal/webauthn/errors.go
  • internal/server/auth/dto.go
  • internal/webauthn/domain.go
  • internal/webauthn/repository.go
  • frontend/src/lib/pages/settings/security.svelte
  • internal/webauthn/service.go
  • internal/webauthn/models.go
  • internal/server/auth/handler.go

Comment thread internal/webauthn/session.go Outdated
@capcom6
capcom6 force-pushed the auth/passkey-support branch from e083872 to 1fe4a95 Compare June 26, 2026 10:08

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/webauthn/service.go`:
- Around line 32-37: The Service constructor still wires in the in-memory
session store via newSessionStore, which keeps session state only on one
process. Replace the sessionStore implementation in internal/webauthn/session.go
and the Service initialization in Service to use a distributed backing store
(for example Redis) so BeginRegistration/FinishRegistration can share sessions
across instances and restarts. Keep the same sessionStore/sessionEntry behavior
and ErrSessionNotFound semantics, but back the store with shared persistence
instead of the local map and cleanup goroutine.
- Around line 105-107: The passwordless login flow in BeginLogin currently
relies on BeginDiscoverableLogin() defaults, which leaves user verification as
preferred instead of required. Update the BeginDiscoverableLogin call in
Service.BeginLogin to pass explicit options that require user verification, so
passwordless authentication cannot succeed with possession-only authenticators.
Use the existing BeginLogin and BeginDiscoverableLogin symbols to locate the
change and keep the rest of the login/session handling unchanged.
- Around line 49-50: Update the registration flow in service.go so
`s.webAuthn.BeginRegistration(wuser)` uses authenticator selection that պահանջs
discoverable/resident credentials, matching the discoverable login path.
Configure this through `webauthn.WithAuthenticatorSelection` on the WebAuthn
setup used by `BeginRegistration`, rather than relying on a non-existent
resident-key option, and ensure the registration options explicitly require a
resident/discoverable credential.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 016a6e2b-9af0-43cd-b932-7294db0c1f4c

📥 Commits

Reviewing files that changed from the base of the PR and between e083872 and 1fe4a95.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (26)
  • .env.example
  • .golangci.yml
  • frontend/src/App.svelte
  • frontend/src/lib/api/passkey.ts
  • frontend/src/lib/pages/profile.svelte
  • frontend/src/lib/pages/settings/security.svelte
  • frontend/src/lib/stores/auth.svelte.ts
  • frontend/src/lib/types/api.ts
  • frontend/src/pages/login.svelte
  • go.mod
  • internal/commands/serve/serve.go
  • internal/config/config.go
  • internal/config/module.go
  • internal/db/migrations/20260623000000_webauthn_credentials.sql
  • internal/server/auth/dto.go
  • internal/server/auth/handler.go
  • internal/server/middlewares/jwtauth/jwtauth.go
  • internal/webauthn/config.go
  • internal/webauthn/domain.go
  • internal/webauthn/encoding.go
  • internal/webauthn/errors.go
  • internal/webauthn/models.go
  • internal/webauthn/module.go
  • internal/webauthn/repository.go
  • internal/webauthn/service.go
  • internal/webauthn/session.go
✅ Files skipped from review due to trivial changes (1)
  • frontend/src/lib/pages/profile.svelte
🚧 Files skipped from review as they are similar to previous changes (23)
  • internal/webauthn/encoding.go
  • internal/db/migrations/20260623000000_webauthn_credentials.sql
  • frontend/src/App.svelte
  • internal/webauthn/errors.go
  • internal/webauthn/config.go
  • internal/server/middlewares/jwtauth/jwtauth.go
  • internal/commands/serve/serve.go
  • .golangci.yml
  • internal/webauthn/module.go
  • internal/webauthn/models.go
  • frontend/src/lib/types/api.ts
  • frontend/src/lib/pages/settings/security.svelte
  • internal/webauthn/repository.go
  • frontend/src/pages/login.svelte
  • internal/server/auth/handler.go
  • frontend/src/lib/stores/auth.svelte.ts
  • internal/config/module.go
  • internal/server/auth/dto.go
  • internal/webauthn/session.go
  • go.mod
  • internal/webauthn/domain.go
  • internal/config/config.go
  • frontend/src/lib/api/passkey.ts

Comment thread internal/webauthn/service.go Outdated
Comment thread internal/webauthn/service.go Outdated
Comment thread internal/webauthn/service.go Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
internal/webauthn/service.go (1)

64-64: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Fail the begin step when session persistence fails.

storeSession only logs Set failures, so both begin flows can return a challenge that was never persisted. The very next finish call then degrades into ErrSessionNotFound instead of surfacing the real server-side failure. Make storeSession return an error and propagate it from both begin methods.

Suggested change
-func (s *Service) storeSession(data *webauthn.SessionData) {
-	if err := s.session.Set(
-		context.Background(),
+func (s *Service) storeSession(ctx context.Context, data *webauthn.SessionData) error {
+	return s.session.Set(
+		ctx,
 		data.Challenge,
 		&sessionEntry{SessionData: data},
 		cache.WithTTL(sessionTTL),
-	); err != nil {
-		s.logger.Error("failed to store session", zap.Error(err))
-	}
+	)
 }
-	s.storeSession(sessionData)
+	if err := s.storeSession(ctx, sessionData); err != nil {
+		return nil, fmt.Errorf("failed to store session: %w", err)
+	}

Also applies to: 123-123, 196-205

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/webauthn/service.go` at line 64, The begin flows in the WebAuthn
service can return a challenge even when session persistence fails because
storeSession only logs errors and does not stop execution. Update storeSession
in the service.go flow to return an error when the session cannot be saved, then
propagate that error from both begin methods that call it so the begin step
fails immediately instead of later surfacing ErrSessionNotFound. Use the
existing begin and storeSession methods in the WebAuthn service to wire the
error through both code paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@internal/webauthn/service.go`:
- Line 64: The begin flows in the WebAuthn service can return a challenge even
when session persistence fails because storeSession only logs errors and does
not stop execution. Update storeSession in the service.go flow to return an
error when the session cannot be saved, then propagate that error from both
begin methods that call it so the begin step fails immediately instead of later
surfacing ErrSessionNotFound. Use the existing begin and storeSession methods in
the WebAuthn service to wire the error through both code paths.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e1c63aee-e2b2-4bd0-bc38-f865b98ad4e8

📥 Commits

Reviewing files that changed from the base of the PR and between 1fe4a95 and 59e4176.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (8)
  • .env.example
  • go.mod
  • internal/commands/serve/serve.go
  • internal/config/config.go
  • internal/config/module.go
  • internal/webauthn/module.go
  • internal/webauthn/service.go
  • internal/webauthn/session.go
✅ Files skipped from review due to trivial changes (1)
  • .env.example
🚧 Files skipped from review as they are similar to previous changes (1)
  • go.mod

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/webauthn/service.go`:
- Around line 64-65: Thread the caller context through session persistence by
updating the registration/login flows and `storeSession` so cache writes no
longer use `context.Background()`. Fix the new `govet` shadow warnings by
renaming any reused `err` variables in the affected call sites, and wrap the
underlying cache error returned from `storeSession` instead of exposing it
directly so `wrapcheck` passes.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 05171e2b-2d06-4d39-9f5a-eeb279f83ea3

📥 Commits

Reviewing files that changed from the base of the PR and between 59e4176 and dc6d22f.

📒 Files selected for processing (1)
  • internal/webauthn/service.go

Comment thread internal/webauthn/service.go Outdated
@capcom6
capcom6 force-pushed the auth/passkey-support branch from ef3e8df to fdaacd1 Compare June 28, 2026 11:54
@coderabbitai
coderabbitai Bot requested a review from dudina-ma June 28, 2026 11:55

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/webauthn/service.go`:
- Around line 213-218: The loadSession helper is swallowing all cache Get
failures by using context.Background() and returning nil, which hides
cancellations and operational cache errors as missing sessions. Update
loadSession in Service to accept the request context and return both session
data and an error, so only a real cache miss maps to ErrSessionNotFound while
other session.Get failures are propagated to the caller. Then adjust the callers
that use loadSession to handle the returned error explicitly and preserve
existing missing-session behavior only for true misses.
- Around line 221-224: The deleteSession helper in Service currently only logs
cache delete failures, which lets FinishRegistration and FinishLogin continue
even if the challenge was not consumed. Change deleteSession to return an error
when session.Delete fails, then update the FinishRegistration and FinishLogin
success paths to check that result and abort the ceremony if challenge
consumption fails. Keep the failure handling close to the existing
deleteSession, FinishRegistration, and FinishLogin flow so a session can’t be
reused when deletion does not succeed.
- Around line 56-59: Registration in WebAuthnService.BeginRegistration only sets
resident-key requirements, so it can create credentials that do not meet the
passwordless login policy enforced by BeginLogin. Update the authenticator
selection passed to s.webAuthn.BeginRegistration to also require user
verification by adding protocol.VerificationRequired alongside the existing
resident-key requirement, so new credentials can satisfy the login flow.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3d86165e-f3a2-4639-a846-19816e9665b2

📥 Commits

Reviewing files that changed from the base of the PR and between ef3e8df and fdaacd1.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (26)
  • .env.example
  • .golangci.yml
  • frontend/src/App.svelte
  • frontend/src/lib/api/passkey.ts
  • frontend/src/lib/pages/profile.svelte
  • frontend/src/lib/pages/settings/security.svelte
  • frontend/src/lib/stores/auth.svelte.ts
  • frontend/src/lib/types/api.ts
  • frontend/src/pages/login.svelte
  • go.mod
  • internal/commands/serve/serve.go
  • internal/config/config.go
  • internal/config/module.go
  • internal/db/migrations/20260623000000_webauthn_credentials.sql
  • internal/server/auth/dto.go
  • internal/server/auth/handler.go
  • internal/server/middlewares/jwtauth/jwtauth.go
  • internal/webauthn/config.go
  • internal/webauthn/domain.go
  • internal/webauthn/encoding.go
  • internal/webauthn/errors.go
  • internal/webauthn/models.go
  • internal/webauthn/module.go
  • internal/webauthn/repository.go
  • internal/webauthn/service.go
  • internal/webauthn/session.go
✅ Files skipped from review due to trivial changes (1)
  • internal/server/middlewares/jwtauth/jwtauth.go
🚧 Files skipped from review as they are similar to previous changes (23)
  • internal/webauthn/config.go
  • internal/webauthn/encoding.go
  • internal/db/migrations/20260623000000_webauthn_credentials.sql
  • internal/webauthn/errors.go
  • internal/webauthn/session.go
  • internal/config/module.go
  • frontend/src/App.svelte
  • frontend/src/lib/pages/profile.svelte
  • internal/server/auth/dto.go
  • frontend/src/lib/types/api.ts
  • .golangci.yml
  • internal/webauthn/models.go
  • internal/commands/serve/serve.go
  • internal/webauthn/module.go
  • frontend/src/lib/pages/settings/security.svelte
  • internal/webauthn/domain.go
  • frontend/src/lib/stores/auth.svelte.ts
  • internal/config/config.go
  • frontend/src/lib/api/passkey.ts
  • frontend/src/pages/login.svelte
  • go.mod
  • internal/webauthn/repository.go
  • internal/server/auth/handler.go

Comment thread internal/webauthn/service.go
Comment thread internal/webauthn/service.go Outdated
Comment thread internal/webauthn/service.go Outdated
@capcom6
capcom6 force-pushed the auth/passkey-support branch from 12644cf to bff6fb7 Compare June 29, 2026 07:12

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/webauthn/service.go`:
- Around line 84-90: The WebAuthn finish flows in `service.go` currently call
`loadSession()` and `deleteSession()` separately, which allows concurrent
requests with the same challenge to reuse the same session. Replace this
two-step pattern with a single atomic consume helper in `Service` (for example,
using backend `GETDEL`, a transaction/Lua script, or a per-challenge lock) that
loads and deletes the session in one operation, and update both finish paths to
use that helper instead of `loadSession()` followed by `deleteSession()`.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1f594e5d-ab1e-47ee-a36a-76e1036dad94

📥 Commits

Reviewing files that changed from the base of the PR and between 12644cf and bff6fb7.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (26)
  • .env.example
  • .golangci.yml
  • frontend/src/App.svelte
  • frontend/src/lib/api/passkey.ts
  • frontend/src/lib/pages/profile.svelte
  • frontend/src/lib/pages/settings/security.svelte
  • frontend/src/lib/stores/auth.svelte.ts
  • frontend/src/lib/types/api.ts
  • frontend/src/pages/login.svelte
  • go.mod
  • internal/commands/serve/serve.go
  • internal/config/config.go
  • internal/config/module.go
  • internal/db/migrations/20260623000000_webauthn_credentials.sql
  • internal/server/auth/dto.go
  • internal/server/auth/handler.go
  • internal/server/middlewares/jwtauth/jwtauth.go
  • internal/webauthn/config.go
  • internal/webauthn/domain.go
  • internal/webauthn/encoding.go
  • internal/webauthn/errors.go
  • internal/webauthn/models.go
  • internal/webauthn/module.go
  • internal/webauthn/repository.go
  • internal/webauthn/service.go
  • internal/webauthn/session.go
🚧 Files skipped from review as they are similar to previous changes (24)
  • internal/db/migrations/20260623000000_webauthn_credentials.sql
  • internal/webauthn/errors.go
  • frontend/src/lib/pages/profile.svelte
  • internal/webauthn/encoding.go
  • internal/webauthn/session.go
  • frontend/src/lib/types/api.ts
  • internal/webauthn/module.go
  • internal/webauthn/config.go
  • frontend/src/App.svelte
  • internal/server/auth/dto.go
  • internal/webauthn/models.go
  • internal/server/middlewares/jwtauth/jwtauth.go
  • internal/commands/serve/serve.go
  • frontend/src/lib/pages/settings/security.svelte
  • internal/config/config.go
  • frontend/src/lib/stores/auth.svelte.ts
  • frontend/src/pages/login.svelte
  • .golangci.yml
  • internal/server/auth/handler.go
  • internal/config/module.go
  • internal/webauthn/repository.go
  • frontend/src/lib/api/passkey.ts
  • internal/webauthn/domain.go
  • go.mod

Comment thread internal/webauthn/service.go Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
internal/webauthn/models.go (1)

36-43: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Use the domain field name in this mapping.

internal/webauthn/domain.go exposes the slice as Transports []string, so Transport: does not match the current domain contract here. This breaks the model-to-domain conversion path.

🛠️ Suggested fix
 	return &Credential{
 		ID:              m.ID,
 		UserID:          m.UserID,
 		CredentialID:    m.CredentialID,
 		PublicKey:       m.PublicKey,
 		AttestationType: m.AttestationType,
-		Transport:       m.Transport,
+		Transports:      append([]string(nil), m.Transport...),
 		AAGUID:          m.AAGUID,
 		Flags:           m.Flags,
 		SignCount:       m.SignCount,
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/webauthn/models.go` around lines 36 - 43, The model-to-domain
mapping in the credential conversion logic is using the wrong field name for the
transport slice. Update the Credential conversion in models.go to map to the
domain field exposed by internal/webauthn/domain.go, using the Transports field
instead of Transport so the mapping matches the current domain contract and
preserves the conversion path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/webauthn/service.go`:
- Around line 177-179: The sign-count update in s.credentials.UpdateSignCount is
currently treated as best-effort, which lets login continue with stale
credential state. Change the webauthn login flow in the service method that
handles the assertion so that a failure from UpdateSignCount is treated as a
hard error and aborts the operation instead of only logging a warning. Keep the
update tied to the existing credential/sign-in path by using the same
credModel.ID and credential.Authenticator.SignCount values, but return or
propagate the update error from the surrounding handler instead of swallowing
it.

---

Outside diff comments:
In `@internal/webauthn/models.go`:
- Around line 36-43: The model-to-domain mapping in the credential conversion
logic is using the wrong field name for the transport slice. Update the
Credential conversion in models.go to map to the domain field exposed by
internal/webauthn/domain.go, using the Transports field instead of Transport so
the mapping matches the current domain contract and preserves the conversion
path.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c0d826ff-48db-4660-8189-404af1a97518

📥 Commits

Reviewing files that changed from the base of the PR and between bff6fb7 and 726b2d8.

📒 Files selected for processing (5)
  • internal/webauthn/domain.go
  • internal/webauthn/errors.go
  • internal/webauthn/models.go
  • internal/webauthn/service.go
  • internal/webauthn/sessions.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • internal/webauthn/errors.go
  • internal/webauthn/domain.go

Comment thread internal/webauthn/service.go

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
internal/webauthn/service.go (1)

56-63: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Require user verification for passwordless passkey auth.

VerificationPreferred can allow possession-only authentication. Since FinishLogin returns a fully authenticated user without a password step, use protocol.VerificationRequired for both registration policy and discoverable login.

🔐 Suggested fix
 			RequireResidentKey:      protocol.ResidentKeyRequired(),
 			ResidentKey:             protocol.ResidentKeyRequirementRequired,
-			UserVerification:        protocol.VerificationPreferred,
+			UserVerification:        protocol.VerificationRequired,
 		}),
 	options, sessionData, err := s.webAuthn.BeginDiscoverableLogin(
-		webauthn.WithUserVerification(protocol.VerificationPreferred),
+		webauthn.WithUserVerification(protocol.VerificationRequired),
 	)

This repeats earlier UV-policy feedback, but the current code explicitly sets preferred. Verify the option names against the pinned github.com/go-webauthn/webauthn version if needed.

github.com/go-webauthn/webauthn v0.17.4 BeginDiscoverableLogin WithUserVerification VerificationRequired

Also applies to: 120-123

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/webauthn/service.go` around lines 56 - 63, Update the WebAuthn UV
policy in the registration and discoverable login flow so passwordless passkey
auth requires user verification instead of allowing possession-only auth. In the
`BeginRegistration` authenticator selection and the discoverable login setup
used with `FinishLogin`, replace the current `protocol.VerificationPreferred`
setting with the correct required-verification option for the pinned
`github.com/go-webauthn/webauthn` version, and verify the exact option names in
the related `BeginDiscoverableLogin`/`WithUserVerification` calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@internal/webauthn/service.go`:
- Around line 56-63: Update the WebAuthn UV policy in the registration and
discoverable login flow so passwordless passkey auth requires user verification
instead of allowing possession-only auth. In the `BeginRegistration`
authenticator selection and the discoverable login setup used with
`FinishLogin`, replace the current `protocol.VerificationPreferred` setting with
the correct required-verification option for the pinned
`github.com/go-webauthn/webauthn` version, and verify the exact option names in
the related `BeginDiscoverableLogin`/`WithUserVerification` calls.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3fe33841-fcc4-4c0f-a8a5-d982a369ceed

📥 Commits

Reviewing files that changed from the base of the PR and between 726b2d8 and ed7be6c.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (6)
  • go.mod
  • internal/commands/serve/serve.go
  • internal/webauthn/domain.go
  • internal/webauthn/models.go
  • internal/webauthn/repository.go
  • internal/webauthn/service.go
💤 Files with no reviewable changes (1)
  • go.mod
🚧 Files skipped from review as they are similar to previous changes (1)
  • internal/webauthn/repository.go

@capcom6
capcom6 force-pushed the auth/passkey-support branch from ed7be6c to 734245d Compare June 30, 2026 07:22

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/webauthn/repository.go`:
- Around line 74-90: Update Repository.UpdateSignCount so the write is monotonic
by adding an atomic condition that only updates when the stored sign_count is
less than or equal to the new signCount, using the existing Update/Where flow in
updateSignCount. Then adjust the post-update handling to return
ErrCredentialNotFound only when no credential row exists, not when the row was
skipped because a newer sign_count was already present; keep the existing error
wrapping for Exec and RowsAffected in Repository.UpdateSignCount.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ed3c84dc-63b3-443c-9fd3-60a54f2daa96

📥 Commits

Reviewing files that changed from the base of the PR and between ed7be6c and 734245d.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (25)
  • .env.example
  • .golangci.yml
  • frontend/src/App.svelte
  • frontend/src/lib/api/passkey.ts
  • frontend/src/lib/pages/profile.svelte
  • frontend/src/lib/pages/settings/security.svelte
  • frontend/src/lib/stores/auth.svelte.ts
  • frontend/src/lib/types/api.ts
  • frontend/src/pages/login.svelte
  • go.mod
  • internal/commands/serve/serve.go
  • internal/config/config.go
  • internal/config/module.go
  • internal/db/migrations/20260623000000_webauthn_credentials.sql
  • internal/server/auth/dto.go
  • internal/server/auth/handler.go
  • internal/server/middlewares/jwtauth/jwtauth.go
  • internal/webauthn/config.go
  • internal/webauthn/domain.go
  • internal/webauthn/errors.go
  • internal/webauthn/models.go
  • internal/webauthn/module.go
  • internal/webauthn/repository.go
  • internal/webauthn/service.go
  • internal/webauthn/sessions.go
✅ Files skipped from review due to trivial changes (2)
  • frontend/src/lib/pages/profile.svelte
  • internal/webauthn/errors.go
🚧 Files skipped from review as they are similar to previous changes (19)
  • internal/server/middlewares/jwtauth/jwtauth.go
  • internal/server/auth/dto.go
  • internal/webauthn/config.go
  • internal/config/module.go
  • internal/db/migrations/20260623000000_webauthn_credentials.sql
  • frontend/src/lib/types/api.ts
  • frontend/src/App.svelte
  • internal/webauthn/module.go
  • .golangci.yml
  • internal/webauthn/sessions.go
  • frontend/src/lib/pages/settings/security.svelte
  • internal/config/config.go
  • frontend/src/lib/stores/auth.svelte.ts
  • internal/commands/serve/serve.go
  • frontend/src/pages/login.svelte
  • go.mod
  • internal/server/auth/handler.go
  • internal/webauthn/models.go
  • frontend/src/lib/api/passkey.ts

Comment thread internal/webauthn/repository.go
@capcom6
capcom6 force-pushed the auth/passkey-support branch from b7cb029 to b4b5c2a Compare July 2, 2026 02:20

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
internal/webauthn/service.go (1)

56-64: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Require VerificationRequired for passkey registration and login

Both passkey ceremonies still use protocol.VerificationPreferred, which allows possession-only authenticators to complete a passwordless login without PIN/biometric confirmation. Switch both call sites to VerificationRequired unless weaker UV support is intentional.

Suggested fix
-			UserVerification:        protocol.VerificationPreferred,
+			UserVerification:        protocol.VerificationRequired,
-		webauthn.WithUserVerification(protocol.VerificationPreferred),
+		webauthn.WithUserVerification(protocol.VerificationRequired),
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/webauthn/service.go` around lines 56 - 64, The passkey WebAuthn
ceremonies are still using protocol.VerificationPreferred, which should be
tightened to protocol.VerificationRequired for user verification. Update the
authenticator selection passed from s.webAuthn.BeginRegistration and the
matching login ceremony call site to require verification, keeping the rest of
the WebAuthn options unchanged unless weaker UV is intentionally needed.
🧹 Nitpick comments (1)
internal/webauthn/service.go (1)

148-175: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Redundant credential lookup in FinishLogin.

The discovery handler (line 149) already loads the credential row by rawID via GetByCredentialID, but line 172 fetches it again by credential.ID after ValidatePasskeyLogin succeeds. Since credential.ID is the same ID used inside the handler, this is an avoidable extra DB round-trip on every login.

♻️ Suggested fix — capture the credential fetched inside the handler
+	var credModel *Credential
 	handler := func(rawID, _ []byte) (webauthn.User, error) {
-		cred, getErr := s.credentials.GetByCredentialID(ctx, rawID)
+		cred, getErr := s.credentials.GetByCredentialID(ctx, rawID)
 		if getErr != nil {
 			return nil, fmt.Errorf("credential not found: %w", getErr)
 		}
+		credModel = cred
 
 		user, getErr := s.usersSvc.GetByID(ctx, cred.UserID)
 		...
 	}
 
 	_, credential, err := s.webAuthn.ValidatePasskeyLogin(handler, *sessionData, parsedResponse)
 	if err != nil {
 		return nil, fmt.Errorf("failed to validate login: %w", err)
 	}
-
-	credModel, err := s.credentials.GetByCredentialID(ctx, credential.ID)
-	if err != nil {
-		return nil, fmt.Errorf("failed to get stored credential: %w", err)
-	}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/webauthn/service.go` around lines 148 - 175, The `FinishLogin` flow
is doing a redundant credential lookup after `ValidatePasskeyLogin` even though
the `handler` already fetches the credential with `GetByCredentialID` using the
same ID. Update `FinishLogin` so it reuses the credential loaded inside the
discovery `handler` (or otherwise returns it from the handler/validation path)
instead of calling `s.credentials.GetByCredentialID` again for `credential.ID`,
while keeping the existing `newWebAuthnUser`, `ValidatePasskeyLogin`, and login
validation flow intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@internal/webauthn/service.go`:
- Around line 56-64: The passkey WebAuthn ceremonies are still using
protocol.VerificationPreferred, which should be tightened to
protocol.VerificationRequired for user verification. Update the authenticator
selection passed from s.webAuthn.BeginRegistration and the matching login
ceremony call site to require verification, keeping the rest of the WebAuthn
options unchanged unless weaker UV is intentionally needed.

---

Nitpick comments:
In `@internal/webauthn/service.go`:
- Around line 148-175: The `FinishLogin` flow is doing a redundant credential
lookup after `ValidatePasskeyLogin` even though the `handler` already fetches
the credential with `GetByCredentialID` using the same ID. Update `FinishLogin`
so it reuses the credential loaded inside the discovery `handler` (or otherwise
returns it from the handler/validation path) instead of calling
`s.credentials.GetByCredentialID` again for `credential.ID`, while keeping the
existing `newWebAuthnUser`, `ValidatePasskeyLogin`, and login validation flow
intact.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d85d1180-00a1-43e2-b621-9515dd232ff6

📥 Commits

Reviewing files that changed from the base of the PR and between b7cb029 and b4b5c2a.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (25)
  • .env.example
  • .golangci.yml
  • frontend/src/App.svelte
  • frontend/src/lib/api/passkey.ts
  • frontend/src/lib/pages/profile.svelte
  • frontend/src/lib/pages/settings/security.svelte
  • frontend/src/lib/stores/auth.svelte.ts
  • frontend/src/lib/types/api.ts
  • frontend/src/pages/login.svelte
  • go.mod
  • internal/commands/serve/serve.go
  • internal/config/config.go
  • internal/config/module.go
  • internal/db/migrations/20260623000000_webauthn_credentials.sql
  • internal/server/auth/dto.go
  • internal/server/auth/handler.go
  • internal/server/middlewares/jwtauth/jwtauth.go
  • internal/webauthn/config.go
  • internal/webauthn/domain.go
  • internal/webauthn/errors.go
  • internal/webauthn/models.go
  • internal/webauthn/module.go
  • internal/webauthn/repository.go
  • internal/webauthn/service.go
  • internal/webauthn/sessions.go
✅ Files skipped from review due to trivial changes (4)
  • frontend/src/lib/pages/profile.svelte
  • internal/webauthn/config.go
  • internal/webauthn/errors.go
  • frontend/src/lib/types/api.ts
🚧 Files skipped from review as they are similar to previous changes (19)
  • internal/db/migrations/20260623000000_webauthn_credentials.sql
  • internal/config/module.go
  • frontend/src/App.svelte
  • frontend/src/lib/pages/settings/security.svelte
  • internal/webauthn/module.go
  • internal/config/config.go
  • internal/webauthn/sessions.go
  • frontend/src/lib/stores/auth.svelte.ts
  • frontend/src/pages/login.svelte
  • internal/commands/serve/serve.go
  • frontend/src/lib/api/passkey.ts
  • internal/server/auth/dto.go
  • internal/webauthn/repository.go
  • .golangci.yml
  • internal/webauthn/domain.go
  • internal/webauthn/models.go
  • internal/server/auth/handler.go
  • go.mod
  • internal/server/middlewares/jwtauth/jwtauth.go

@capcom6
capcom6 force-pushed the auth/passkey-support branch from 63b8613 to c5b50ac Compare July 3, 2026 00:22

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
.env.example (1)

189-217: 🚀 Performance & Scalability | 🔵 Trivial

Document the CACHE__URL dependency for multi-instance WebAuthn deployments.

WebAuthn registration/login sessions are backed by this same cache (per the stack's "cache-backed session storage"). With the documented CACHE__URL=memory:// default, multi-instance/HA deployments will break passkey ceremonies since session state won't be shared across instances. Consider adding a note here (or near the CACHE block) that CACHE__URL must point to Redis whenever WebAuthn is enabled in a multi-instance deployment.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.env.example around lines 189 - 217, Document the WebAuthn dependency on
shared cache storage: in the WebAuthn section around WEBAUTHN__RP_DISPLAY_NAME,
WEBAUTHN__RP_ID, and WEBAUTHN__RP_ORIGINS, add a note that CACHE__URL must use a
shared backend such as Redis for multi-instance or HA deployments when WebAuthn
is enabled, since the passkey ceremony/session state is cache-backed and
memory:// will not work across instances. Keep the guidance near the existing
cache/WebAuthn configuration so the dependency is easy to find.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/webauthn/service.go`:
- Around line 56-67: The WebAuthn registration and login ceremonies are
currently using protocol.VerificationPreferred, which can allow authenticator
use without PIN/biometric verification. Update the authenticator selection in
the registration flow in internal/webauthn/service.go and the corresponding
login flow so both use protocol.VerificationRequired, keeping the change
localized around the BeginRegistration and BeginLogin calls and their WebAuthn
options.

---

Nitpick comments:
In @.env.example:
- Around line 189-217: Document the WebAuthn dependency on shared cache storage:
in the WebAuthn section around WEBAUTHN__RP_DISPLAY_NAME, WEBAUTHN__RP_ID, and
WEBAUTHN__RP_ORIGINS, add a note that CACHE__URL must use a shared backend such
as Redis for multi-instance or HA deployments when WebAuthn is enabled, since
the passkey ceremony/session state is cache-backed and memory:// will not work
across instances. Keep the guidance near the existing cache/WebAuthn
configuration so the dependency is easy to find.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b3797528-c762-4fcd-b571-8b85ccd8b308

📥 Commits

Reviewing files that changed from the base of the PR and between 63b8613 and c5b50ac.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (25)
  • .env.example
  • .golangci.yml
  • frontend/src/App.svelte
  • frontend/src/lib/api/passkey.ts
  • frontend/src/lib/pages/profile.svelte
  • frontend/src/lib/pages/settings/security.svelte
  • frontend/src/lib/stores/auth.svelte.ts
  • frontend/src/lib/types/api.ts
  • frontend/src/pages/login.svelte
  • go.mod
  • internal/commands/serve/serve.go
  • internal/config/config.go
  • internal/config/module.go
  • internal/db/migrations/20260623000000_webauthn_credentials.sql
  • internal/server/auth/dto.go
  • internal/server/auth/handler.go
  • internal/server/middlewares/jwtauth/jwtauth.go
  • internal/webauthn/config.go
  • internal/webauthn/domain.go
  • internal/webauthn/errors.go
  • internal/webauthn/models.go
  • internal/webauthn/module.go
  • internal/webauthn/repository.go
  • internal/webauthn/service.go
  • internal/webauthn/sessions.go
🚧 Files skipped from review as they are similar to previous changes (23)
  • frontend/src/App.svelte
  • frontend/src/lib/pages/profile.svelte
  • internal/server/middlewares/jwtauth/jwtauth.go
  • internal/webauthn/sessions.go
  • internal/config/module.go
  • internal/webauthn/config.go
  • internal/webauthn/module.go
  • internal/commands/serve/serve.go
  • internal/webauthn/errors.go
  • frontend/src/lib/types/api.ts
  • frontend/src/lib/pages/settings/security.svelte
  • internal/server/auth/dto.go
  • internal/config/config.go
  • frontend/src/lib/stores/auth.svelte.ts
  • internal/webauthn/domain.go
  • .golangci.yml
  • frontend/src/pages/login.svelte
  • internal/webauthn/repository.go
  • internal/webauthn/models.go
  • internal/server/auth/handler.go
  • frontend/src/lib/api/passkey.ts
  • go.mod
  • internal/db/migrations/20260623000000_webauthn_credentials.sql

Comment thread internal/webauthn/service.go

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (3)
internal/webauthn/models.go (3)

186-206: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Scan lacks nil-source handling, unlike sibling Transports.Scan.

The notnull constraint on the aaguid column (line 26) makes this unreachable today, but if that constraint ever changes, a NULL value here would error with "unexpected type" rather than being handled like Transports.Scan does.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/webauthn/models.go` around lines 186 - 206, Update AAGUID.Scan to
handle a nil src the same way Transports.Scan does instead of treating it as an
unexpected type. Add a nil case in the type switch in AAGUID.Scan, return the
zero/empty AAGUID value for NULL input, and keep the existing string/[]byte hex
decoding behavior unchanged.

167-183: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Silent data loss when AAGUID length is unexpected.

String() returns "" whenever len(a) != 16, and Value() persists that empty string with no error. If an authenticator ever returns a non-16-byte AAGUID (corruption/bug), the raw bytes are silently discarded on write instead of surfacing the anomaly.

♻️ Proposed fix to fail loudly on unexpected length
 func (a *AAGUID) Value() (driver.Value, error) {
 	if a == nil {
 		return nil, nil //nolint:nilnil //empty value
 	}
-
-	return a.String(), nil
+	const aaguidLen = 16
+	if len(*a) != 0 && len(*a) != aaguidLen {
+		return nil, fmt.Errorf("invalid aaguid length: %d", len(*a))
+	}
+	return a.String(), nil
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/webauthn/models.go` around lines 167 - 183, The AAGUID serialization
path is silently dropping invalid data because AAGUID.String() returns an empty
string for non-16-byte values and (*AAGUID).Value() persists that result without
error. Update AAGUID.String() and/or (*AAGUID).Value() so unexpected lengths are
treated as an error instead of being converted to "". Make the failure explicit
at the Value() boundary, returning an error when the AAGUID length is not
exactly 16 bytes, while keeping the nil receiver behavior unchanged.

165-183: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Replace formatAAGUID with AAGUID(credential.Authenticator.AAGUID).String()

internal/webauthn/service.go still duplicates the same dashed-hex AAGUID formatting logic; using the shared AAGUID.String() method would keep the device-name lookup and string representation aligned.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/webauthn/models.go` around lines 165 - 183, The AAGUID formatting
logic is duplicated in the WebAuthn service, so update the
credential/device-name lookup in service.go to use the shared AAGUID.String()
method instead of the local formatAAGUID helper. Locate the call site using
credential.Authenticator.AAGUID and replace the manual dashed-hex formatting
with AAGUID(credential.Authenticator.AAGUID).String() so the string
representation stays consistent with the AAGUID type in models.go.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@internal/webauthn/models.go`:
- Around line 186-206: Update AAGUID.Scan to handle a nil src the same way
Transports.Scan does instead of treating it as an unexpected type. Add a nil
case in the type switch in AAGUID.Scan, return the zero/empty AAGUID value for
NULL input, and keep the existing string/[]byte hex decoding behavior unchanged.
- Around line 167-183: The AAGUID serialization path is silently dropping
invalid data because AAGUID.String() returns an empty string for non-16-byte
values and (*AAGUID).Value() persists that result without error. Update
AAGUID.String() and/or (*AAGUID).Value() so unexpected lengths are treated as an
error instead of being converted to "". Make the failure explicit at the Value()
boundary, returning an error when the AAGUID length is not exactly 16 bytes,
while keeping the nil receiver behavior unchanged.
- Around line 165-183: The AAGUID formatting logic is duplicated in the WebAuthn
service, so update the credential/device-name lookup in service.go to use the
shared AAGUID.String() method instead of the local formatAAGUID helper. Locate
the call site using credential.Authenticator.AAGUID and replace the manual
dashed-hex formatting with AAGUID(credential.Authenticator.AAGUID).String() so
the string representation stays consistent with the AAGUID type in models.go.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7ab46e7b-329b-4258-a600-7f3854727808

📥 Commits

Reviewing files that changed from the base of the PR and between c5b50ac and cf840d7.

📒 Files selected for processing (2)
  • internal/webauthn/domain.go
  • internal/webauthn/models.go

@capcom6 capcom6 removed the codex label Jul 5, 2026
@coderabbitai coderabbitai Bot added the codex label Jul 5, 2026
@capcom6
capcom6 force-pushed the auth/passkey-support branch from 22b9e1d to 6d8b1a2 Compare July 6, 2026 04:52

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
internal/webauthn/models.go (1)

208-223: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Hoist the AAGUID lookup map to a package-level variable.

names is reconstructed on every DeviceName() call. If this runs per-credential (e.g., rendering a list of passkeys in settings UI), it's needless repeated allocation.

♻️ Proposed refactor
+var aaguidDeviceNames = map[string]string{
+	"00000000-0000-0000-0000-000000000000": "Unknown Device",
+	"adce0002-35bc-c60a-648b-0b25f1f05503": "Chrome on Mac",
+	"089b7b64-0f30-4f8c-8838-666944e5c09e": "Touch ID",
+	"6028b017-b1d4-4c02-b4b3-afcd7c96e1c1": "Windows Hello",
+	"dd3ec08a-88f2-4e0b-b3f2-0ab636182cf5": "iCloud Keychain",
+	"fdb141b2-5d98-4b6c-8d4e-45c0a3e1c7a8": "Google Password Manager",
+	"ea9b8d66-4d01-1d21-3ce4-b6b48cb575d4": "Android Passkey",
+	"50757fe4-208c-4cbb-a72b-05b48def77b7": "1Password",
+}
+
 func (a AAGUID) DeviceName() string {
-	names := map[string]string{
-		"00000000-0000-0000-0000-000000000000": "Unknown Device",
-		"adce0002-35bc-c60a-648b-0b25f1f05503": "Chrome on Mac",
-		"089b7b64-0f30-4f8c-8838-666944e5c09e": "Touch ID",
-		"6028b017-b1d4-4c02-b4b3-afcd7c96e1c1": "Windows Hello",
-		"dd3ec08a-88f2-4e0b-b3f2-0ab636182cf5": "iCloud Keychain",
-		"fdb141b2-5d98-4b6c-8d4e-45c0a3e1c7a8": "Google Password Manager",
-		"ea9b8d66-4d01-1d21-3ce4-b6b48cb575d4": "Android Passkey",
-		"50757fe4-208c-4cbb-a72b-05b48def77b7": "1Password",
-	}
-	if name, ok := names[a.String()]; ok {
+	if name, ok := aaguidDeviceNames[a.String()]; ok {
 		return name
 	}
 	return "Passkey"
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/webauthn/models.go` around lines 208 - 223, Hoist the
AAGUID-to-device-name lookup out of AAGUID.DeviceName() into a package-level
variable so the map is not rebuilt on every call. Move the current names map to
a shared package-level declaration in internal/webauthn/models.go and have
DeviceName() reuse it while keeping the existing lookup and fallback behavior
unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@internal/webauthn/models.go`:
- Around line 208-223: Hoist the AAGUID-to-device-name lookup out of
AAGUID.DeviceName() into a package-level variable so the map is not rebuilt on
every call. Move the current names map to a shared package-level declaration in
internal/webauthn/models.go and have DeviceName() reuse it while keeping the
existing lookup and fallback behavior unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 53e0c53e-64b4-442f-9127-44f8a8b5881f

📥 Commits

Reviewing files that changed from the base of the PR and between 22b9e1d and 6d8b1a2.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (25)
  • .env.example
  • .golangci.yml
  • frontend/src/App.svelte
  • frontend/src/lib/api/passkey.ts
  • frontend/src/lib/pages/profile.svelte
  • frontend/src/lib/pages/settings/security.svelte
  • frontend/src/lib/stores/auth.svelte.ts
  • frontend/src/lib/types/api.ts
  • frontend/src/pages/login.svelte
  • go.mod
  • internal/commands/serve/serve.go
  • internal/config/config.go
  • internal/config/module.go
  • internal/db/migrations/20260623000000_webauthn_credentials.sql
  • internal/server/auth/dto.go
  • internal/server/auth/handler.go
  • internal/server/middlewares/jwtauth/jwtauth.go
  • internal/webauthn/config.go
  • internal/webauthn/domain.go
  • internal/webauthn/errors.go
  • internal/webauthn/models.go
  • internal/webauthn/module.go
  • internal/webauthn/repository.go
  • internal/webauthn/service.go
  • internal/webauthn/sessions.go
✅ Files skipped from review due to trivial changes (2)
  • internal/webauthn/config.go
  • frontend/src/lib/types/api.ts
🚧 Files skipped from review as they are similar to previous changes (21)
  • frontend/src/App.svelte
  • internal/config/module.go
  • internal/webauthn/module.go
  • internal/server/auth/dto.go
  • .golangci.yml
  • internal/webauthn/sessions.go
  • internal/webauthn/errors.go
  • internal/db/migrations/20260623000000_webauthn_credentials.sql
  • internal/server/middlewares/jwtauth/jwtauth.go
  • internal/commands/serve/serve.go
  • frontend/src/lib/pages/profile.svelte
  • frontend/src/pages/login.svelte
  • frontend/src/lib/pages/settings/security.svelte
  • internal/config/config.go
  • internal/webauthn/repository.go
  • internal/webauthn/domain.go
  • frontend/src/lib/api/passkey.ts
  • frontend/src/lib/stores/auth.svelte.ts
  • internal/server/auth/handler.go
  • internal/webauthn/service.go
  • go.mod

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
internal/server/passkey/dto.go (1)

5-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Rename to avoid stutter flagged by lint.

revive reports passkey.PasskeyCredentialResponse stutters. Renaming to CredentialResponse also requires updating the two usages in handler.go.

🔧 Proposed fix
-type PasskeyCredentialResponse struct {
+type CredentialResponse struct {
 	ID        int64     `json:"id"`
 	Name      string    `json:"name"`
 	CreatedAt time.Time `json:"created_at"`
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/server/passkey/dto.go` around lines 5 - 9, Rename
PasskeyCredentialResponse to CredentialResponse to remove the stutter reported
by revive, and update all references to the new type name, including the two
usages in handler.go and any constructors or return signatures that currently
mention PasskeyCredentialResponse.

Source: Linters/SAST tools

internal/server/passkey/handler.go (1)

186-205: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Wrap the error to satisfy wrapcheck.

Line 203 returns the error from c.Next() (external package fiber) unwrapped, which the Lint check flags.

🔧 Proposed fix
 	default:
-		return err
+		return fmt.Errorf("passkey request failed: %w", err)
 	}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/server/passkey/handler.go` around lines 186 - 205, The errorsHandler
method returns the raw error from c.Next() in the default branch, which triggers
wrapcheck because an external error is being passed through unwrapped. Update
errorsHandler so the default case wraps the returned error with local context
before returning it, while keeping the existing error mappings for
webauthn.ErrSessionNotFound, webauthn.ErrCredentialNotFound,
webauthn.ErrInvalidWebAuthnPayload, and jwt.ErrInvalidConfig unchanged.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@internal/server/passkey/dto.go`:
- Around line 5-9: Rename PasskeyCredentialResponse to CredentialResponse to
remove the stutter reported by revive, and update all references to the new type
name, including the two usages in handler.go and any constructors or return
signatures that currently mention PasskeyCredentialResponse.

In `@internal/server/passkey/handler.go`:
- Around line 186-205: The errorsHandler method returns the raw error from
c.Next() in the default branch, which triggers wrapcheck because an external
error is being passed through unwrapped. Update errorsHandler so the default
case wraps the returned error with local context before returning it, while
keeping the existing error mappings for webauthn.ErrSessionNotFound,
webauthn.ErrCredentialNotFound, webauthn.ErrInvalidWebAuthnPayload, and
jwt.ErrInvalidConfig unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 04b1a5d7-cbbd-4a37-8168-9e1d694b920d

📥 Commits

Reviewing files that changed from the base of the PR and between 6d8b1a2 and 8f68361.

📒 Files selected for processing (5)
  • internal/server/auth/dto.go
  • internal/server/auth/handler.go
  • internal/server/module.go
  • internal/server/passkey/dto.go
  • internal/server/passkey/handler.go
✅ Files skipped from review due to trivial changes (1)
  • internal/server/module.go

@capcom6
capcom6 force-pushed the auth/passkey-support branch from ccea28f to d0dfe5a Compare July 7, 2026 07:47

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
internal/webauthn/models.go (1)

208-223: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Hoist the device-name lookup table to a package-level variable.

names is reconstructed on every DeviceName() call; move it to a package-level var so it is only allocated once.

♻️ Proposed refactor
+var aaguidDeviceNames = map[string]string{
+	"00000000-0000-0000-0000-000000000000": "Unknown Device",
+	"adce0002-35bc-c60a-648b-0b25f1f05503": "Chrome on Mac",
+	"089b7b64-0f30-4f8c-8838-666944e5c09e": "Touch ID",
+	"6028b017-b1d4-4c02-b4b3-afcd7c96e1c1": "Windows Hello",
+	"dd3ec08a-88f2-4e0b-b3f2-0ab636182cf5": "iCloud Keychain",
+	"fdb141b2-5d98-4b6c-8d4e-45c0a3e1c7a8": "Google Password Manager",
+	"ea9b8d66-4d01-1d21-3ce4-b6b48cb575d4": "Android Passkey",
+	"50757fe4-208c-4cbb-a72b-05b48def77b7": "1Password",
+}
+
 func (a AAGUID) DeviceName() string {
-	names := map[string]string{
-		"00000000-0000-0000-0000-000000000000": "Unknown Device",
-		"adce0002-35bc-c60a-648b-0b25f1f05503": "Chrome on Mac",
-		"089b7b64-0f30-4f8c-8838-666944e5c09e": "Touch ID",
-		"6028b017-b1d4-4c02-b4b3-afcd7c96e1c1": "Windows Hello",
-		"dd3ec08a-88f2-4e0b-b3f2-0ab636182cf5": "iCloud Keychain",
-		"fdb141b2-5d98-4b6c-8d4e-45c0a3e1c7a8": "Google Password Manager",
-		"ea9b8d66-4d01-1d21-3ce4-b6b48cb575d4": "Android Passkey",
-		"50757fe4-208c-4cbb-a72b-05b48def77b7": "1Password",
-	}
-	if name, ok := names[a.String()]; ok {
+	if name, ok := aaguidDeviceNames[a.String()]; ok {
 		return name
 	}
 	return "Passkey"
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/webauthn/models.go` around lines 208 - 223, Move the device-name
lookup map out of AAGUID.DeviceName() into a package-level variable so it is
allocated once instead of on every call. Keep the lookup logic in DeviceName()
the same, but reference the shared map variable when resolving a.String() to a
friendly name.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@internal/webauthn/models.go`:
- Around line 208-223: Move the device-name lookup map out of
AAGUID.DeviceName() into a package-level variable so it is allocated once
instead of on every call. Keep the lookup logic in DeviceName() the same, but
reference the shared map variable when resolving a.String() to a friendly name.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 440b110c-c31d-4513-939f-17da052ed365

📥 Commits

Reviewing files that changed from the base of the PR and between ccea28f and d0dfe5a.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (28)
  • .env.example
  • .golangci.yml
  • frontend/src/App.svelte
  • frontend/src/lib/api/passkey.ts
  • frontend/src/lib/pages/profile.svelte
  • frontend/src/lib/pages/settings/security.svelte
  • frontend/src/lib/stores/auth.svelte.ts
  • frontend/src/lib/types/api.ts
  • frontend/src/pages/login.svelte
  • go.mod
  • internal/commands/serve/serve.go
  • internal/config/config.go
  • internal/config/module.go
  • internal/db/migrations/20260623000000_webauthn_credentials.sql
  • internal/server/auth/dto.go
  • internal/server/auth/handler.go
  • internal/server/middlewares/jwtauth/jwtauth.go
  • internal/server/module.go
  • internal/server/passkey/dto.go
  • internal/server/passkey/handler.go
  • internal/webauthn/config.go
  • internal/webauthn/domain.go
  • internal/webauthn/errors.go
  • internal/webauthn/models.go
  • internal/webauthn/module.go
  • internal/webauthn/repository.go
  • internal/webauthn/service.go
  • internal/webauthn/sessions.go
✅ Files skipped from review due to trivial changes (2)
  • internal/server/passkey/dto.go
  • frontend/src/lib/types/api.ts
🚧 Files skipped from review as they are similar to previous changes (24)
  • internal/server/module.go
  • internal/webauthn/config.go
  • frontend/src/App.svelte
  • internal/webauthn/errors.go
  • internal/webauthn/module.go
  • internal/commands/serve/serve.go
  • internal/server/auth/dto.go
  • frontend/src/lib/pages/profile.svelte
  • internal/config/config.go
  • frontend/src/lib/pages/settings/security.svelte
  • internal/db/migrations/20260623000000_webauthn_credentials.sql
  • frontend/src/lib/stores/auth.svelte.ts
  • internal/server/middlewares/jwtauth/jwtauth.go
  • internal/webauthn/domain.go
  • internal/config/module.go
  • .golangci.yml
  • frontend/src/lib/api/passkey.ts
  • internal/webauthn/repository.go
  • internal/server/auth/handler.go
  • frontend/src/pages/login.svelte
  • internal/webauthn/sessions.go
  • go.mod
  • internal/server/passkey/handler.go
  • internal/webauthn/service.go

@capcom6
capcom6 marked this pull request as ready for review July 8, 2026 01:19
@capcom6
capcom6 force-pushed the auth/passkey-support branch from d0dfe5a to 531f8b3 Compare July 8, 2026 01:19

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
internal/webauthn/models.go (1)

55-74: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove redundant zero-value assignments in newCredentialModel.

bun.BaseModel{} and ID: 0 are already zero values and don't need explicit assignment. This is a minor readability nit.

♻️ Proposed refactor
 func newCredentialModel(userID int64, cred *webauthn.Credential, transports []string, name string) *credentialModel {
 	now := time.Now()

 	return &credentialModel{
-		BaseModel: bun.BaseModel{},
-
-		ID:              0,
 		UserID:          userID,
 		CredentialID:    CredentialID(cred.ID),
 		PublicKey:       cred.PublicKey,
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/webauthn/models.go` around lines 55 - 74, In newCredentialModel,
remove the redundant zero-value initialization for the credentialModel fields
that are already defaulted by Go. Keep the meaningful assignments like UserID,
CredentialID, PublicKey, and timestamps, and drop the explicit bun.BaseModel{}
and ID: 0 entries so the constructor is cleaner and easier to scan.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/webauthn/models.go`:
- Around line 140-159: Handle nil src consistently in the custom Scan methods:
CredentialID.Scan and AAGUID.Scan should mirror Transports.Scan by returning nil
when the database value is NULL instead of falling through to ErrUnexpectedType.
Update the Scan switch in both methods to check for a nil src before
type-switching, and keep the existing base64 decode / assignment behavior
unchanged for non-nil values.

---

Nitpick comments:
In `@internal/webauthn/models.go`:
- Around line 55-74: In newCredentialModel, remove the redundant zero-value
initialization for the credentialModel fields that are already defaulted by Go.
Keep the meaningful assignments like UserID, CredentialID, PublicKey, and
timestamps, and drop the explicit bun.BaseModel{} and ID: 0 entries so the
constructor is cleaner and easier to scan.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4cdd0fe6-3727-4df0-8ae7-00382fb88ee9

📥 Commits

Reviewing files that changed from the base of the PR and between d0dfe5a and 531f8b3.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (28)
  • .env.example
  • .golangci.yml
  • frontend/src/App.svelte
  • frontend/src/lib/api/passkey.ts
  • frontend/src/lib/pages/profile.svelte
  • frontend/src/lib/pages/settings/security.svelte
  • frontend/src/lib/stores/auth.svelte.ts
  • frontend/src/lib/types/api.ts
  • frontend/src/pages/login.svelte
  • go.mod
  • internal/commands/serve/serve.go
  • internal/config/config.go
  • internal/config/module.go
  • internal/db/migrations/20260623000000_webauthn_credentials.sql
  • internal/server/auth/dto.go
  • internal/server/auth/handler.go
  • internal/server/middlewares/jwtauth/jwtauth.go
  • internal/server/module.go
  • internal/server/passkey/dto.go
  • internal/server/passkey/handler.go
  • internal/webauthn/config.go
  • internal/webauthn/domain.go
  • internal/webauthn/errors.go
  • internal/webauthn/models.go
  • internal/webauthn/module.go
  • internal/webauthn/repository.go
  • internal/webauthn/service.go
  • internal/webauthn/sessions.go
🚧 Files skipped from review as they are similar to previous changes (26)
  • internal/server/module.go
  • frontend/src/App.svelte
  • internal/db/migrations/20260623000000_webauthn_credentials.sql
  • frontend/src/lib/pages/profile.svelte
  • frontend/src/lib/types/api.ts
  • frontend/src/lib/stores/auth.svelte.ts
  • internal/server/passkey/dto.go
  • internal/webauthn/module.go
  • internal/config/module.go
  • internal/webauthn/errors.go
  • internal/server/middlewares/jwtauth/jwtauth.go
  • internal/server/auth/dto.go
  • internal/webauthn/config.go
  • frontend/src/pages/login.svelte
  • .golangci.yml
  • internal/webauthn/domain.go
  • internal/webauthn/sessions.go
  • internal/webauthn/repository.go
  • internal/server/auth/handler.go
  • internal/config/config.go
  • frontend/src/lib/pages/settings/security.svelte
  • frontend/src/lib/api/passkey.ts
  • internal/commands/serve/serve.go
  • internal/server/passkey/handler.go
  • go.mod
  • internal/webauthn/service.go

Comment thread internal/webauthn/models.go
@capcom6
capcom6 force-pushed the auth/passkey-support branch from 905db77 to d0ccd21 Compare July 9, 2026 00:49

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.env.example:
- Line 216: Wrap the WEBAUTHN__RP_ORIGINS environment value in single quotes in
the example config so the JSON array is preserved verbatim by dotenv parsers.
Update the .env.example entry for WEBAUTHN__RP_ORIGINS to keep the inner double
quotes intact and avoid invalid origin parsing.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f704dc78-b718-4de3-b153-cf74625bfc4e

📥 Commits

Reviewing files that changed from the base of the PR and between 905db77 and d0ccd21.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (28)
  • .env.example
  • .golangci.yml
  • frontend/src/App.svelte
  • frontend/src/lib/api/passkey.ts
  • frontend/src/lib/pages/profile.svelte
  • frontend/src/lib/pages/settings/security.svelte
  • frontend/src/lib/stores/auth.svelte.ts
  • frontend/src/lib/types/api.ts
  • frontend/src/pages/login.svelte
  • go.mod
  • internal/commands/serve/serve.go
  • internal/config/config.go
  • internal/config/module.go
  • internal/db/migrations/20260623000000_webauthn_credentials.sql
  • internal/server/auth/dto.go
  • internal/server/auth/handler.go
  • internal/server/middlewares/jwtauth/jwtauth.go
  • internal/server/module.go
  • internal/server/passkey/dto.go
  • internal/server/passkey/handler.go
  • internal/webauthn/config.go
  • internal/webauthn/domain.go
  • internal/webauthn/errors.go
  • internal/webauthn/models.go
  • internal/webauthn/module.go
  • internal/webauthn/repository.go
  • internal/webauthn/service.go
  • internal/webauthn/sessions.go
✅ Files skipped from review due to trivial changes (1)
  • internal/db/migrations/20260623000000_webauthn_credentials.sql
🚧 Files skipped from review as they are similar to previous changes (25)
  • frontend/src/App.svelte
  • internal/server/passkey/dto.go
  • frontend/src/lib/pages/profile.svelte
  • internal/webauthn/module.go
  • internal/commands/serve/serve.go
  • internal/webauthn/errors.go
  • internal/server/module.go
  • internal/config/config.go
  • internal/webauthn/sessions.go
  • internal/server/middlewares/jwtauth/jwtauth.go
  • frontend/src/lib/types/api.ts
  • internal/server/auth/handler.go
  • internal/webauthn/config.go
  • internal/config/module.go
  • internal/server/auth/dto.go
  • frontend/src/lib/stores/auth.svelte.ts
  • internal/webauthn/repository.go
  • go.mod
  • internal/webauthn/service.go
  • .golangci.yml
  • frontend/src/pages/login.svelte
  • internal/webauthn/domain.go
  • frontend/src/lib/api/passkey.ts
  • frontend/src/lib/pages/settings/security.svelte
  • internal/server/passkey/handler.go

Comment thread .env.example Outdated
@capcom6
capcom6 force-pushed the auth/passkey-support branch 2 times, most recently from 7a8f7c3 to 0814d46 Compare July 10, 2026 02:34

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@frontend/src/lib/pages/settings/security.svelte`:
- Around line 154-157: Add an explicit aria-label to the edit-mode Input bound
to editName, clearly identifying it as the passkey name field; retain the
existing placeholder for visual guidance.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 973ffef3-4485-47d3-982d-806115fb0d20

📥 Commits

Reviewing files that changed from the base of the PR and between 7a8f7c3 and 0814d46.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (28)
  • .env.example
  • .golangci.yml
  • frontend/src/App.svelte
  • frontend/src/lib/api/passkey.ts
  • frontend/src/lib/pages/profile.svelte
  • frontend/src/lib/pages/settings/security.svelte
  • frontend/src/lib/stores/auth.svelte.ts
  • frontend/src/lib/types/api.ts
  • frontend/src/pages/login.svelte
  • go.mod
  • internal/commands/serve/serve.go
  • internal/config/config.go
  • internal/config/module.go
  • internal/db/migrations/20260710000000_webauthn_credentials.sql
  • internal/server/auth/dto.go
  • internal/server/auth/handler.go
  • internal/server/middlewares/jwtauth/jwtauth.go
  • internal/server/module.go
  • internal/server/passkey/dto.go
  • internal/server/passkey/handler.go
  • internal/webauthn/config.go
  • internal/webauthn/domain.go
  • internal/webauthn/errors.go
  • internal/webauthn/models.go
  • internal/webauthn/module.go
  • internal/webauthn/repository.go
  • internal/webauthn/service.go
  • internal/webauthn/sessions.go
✅ Files skipped from review due to trivial changes (3)
  • internal/webauthn/errors.go
  • internal/db/migrations/20260710000000_webauthn_credentials.sql
  • frontend/src/lib/types/api.ts
🚧 Files skipped from review as they are similar to previous changes (22)
  • frontend/src/lib/pages/profile.svelte
  • internal/webauthn/config.go
  • internal/server/passkey/dto.go
  • frontend/src/App.svelte
  • internal/config/module.go
  • internal/server/middlewares/jwtauth/jwtauth.go
  • internal/webauthn/module.go
  • internal/server/auth/dto.go
  • internal/server/module.go
  • internal/webauthn/sessions.go
  • internal/server/auth/handler.go
  • .golangci.yml
  • internal/commands/serve/serve.go
  • frontend/src/lib/api/passkey.ts
  • frontend/src/lib/stores/auth.svelte.ts
  • frontend/src/pages/login.svelte
  • internal/config/config.go
  • internal/webauthn/domain.go
  • internal/webauthn/repository.go
  • internal/server/passkey/handler.go
  • internal/webauthn/service.go
  • go.mod

Comment thread frontend/src/lib/pages/settings/security.svelte
@capcom6
capcom6 force-pushed the auth/passkey-support branch from 0814d46 to 4a5be1d Compare July 10, 2026 03:28
@capcom6 capcom6 added deployed PR is deployed to staging and removed codex labels Jul 10, 2026
@capcom6
capcom6 force-pushed the auth/passkey-support branch from 4a5be1d to e20abfd Compare July 10, 2026 23:48
@github-actions github-actions Bot removed the deployed PR is deployed to staging label Jul 10, 2026
@capcom6
capcom6 force-pushed the auth/passkey-support branch from e20abfd to fda8d60 Compare July 10, 2026 23:50
@capcom6 capcom6 added the deployed PR is deployed to staging label Jul 10, 2026
@capcom6
capcom6 merged commit 83faa4c into master Jul 14, 2026
7 checks passed
@capcom6
capcom6 deleted the auth/passkey-support branch July 14, 2026 01:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deployed PR is deployed to staging

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant