Improve authorization management

[erskingardner]

Summary

• route Amber sign-in through the existing Nostr Connect flow and update the button copy
• add optional authorization nicknames with a SQLite migration and UI display
• add API and UI support to revoke authorizations, including scoped backend deletion of redeemed user rows

Validation

• cargo fmt --check
• cargo check --workspace
• cargo test --workspace (with temporary master.key, removed afterward)
• cd web && bun run check
• cd web && bun test
• cd web && bun run build
• git diff --check
• browser smoke check for app shell/sign-in modal

Summary by CodeRabbit

• New Features

  • Assign nicknames to team key authorizations for clearer identification.
  • Revoke individual authorizations from the UI.

• Bug Fixes

  • Authorization nicknames are trimmed and empty values are stored as null.

• Sign‑in

  • “Connect with Amber” sign-in flow added with improved session handling.

• Tests

  • Expanded coverage for authorization migrations, creation, revocation, and related lifecycle operations.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 597b9d0c-d618-4374-b71c-0add27b46de4

📥 Commits

Reviewing files that changed from the base of the PR and between f62a35f and 24c8b75.

📒 Files selected for processing (2)

• web/src/lib/components/SignInMenu.svelte
• web/src/routes/(app)/teams/[id]/keys/[pubkey]/+page.svelte

🚧 Files skipped from review as they are similar to previous changes (1)

• web/src/routes/(app)/teams/[id]/keys/[pubkey]/+page.svelte

---

Walkthrough

Adds optional authorization names (DB + types), persists trimmed nicknames on create, exposes DELETE endpoint to revoke authorizations (transactionally removes redemptions), updates UI to create and revoke nicknamed authorizations, and refactors Nostr Connect signin to accept signerKind options and adjust Amber support detection.

Changes

Authorization Naming and Revocation

Layer / File(s)	Summary
Database schema and backend data model database/migrations/0003_add_authorization_name.sql, core/src/types/authorization.rs, core/src/database.rs	Migration adds nullable name TEXT column to authorizations; Authorization struct gains optional name field populated via FromRow; test fixtures and assertions updated to include the new column.
API request contracts api/src/api/types.rs, web/src/lib/types.ts	AddAuthorizationRequest.name becomes Option<String> with #[serde(default)]; web Authorization type gains nullable name field.
Authorization endpoints and routes api/src/api/http/routes.rs, api/src/api/http/teams.rs	DELETE route wired to remove_authorization; add_authorization normalizes incoming name (trim + empty-to-None); remove_authorization validates admin access, resolves stored key, ensures authorization exists, deletes related user_authorizations and authorization record within transaction.
Authorization card component with revoke web/src/lib/components/AuthorizationCard.svelte	Accepts onRevoke callback prop; displays authorization name with secret on separate line when name exists, otherwise secret only; replaces copy button with revoke button that disables during async revocation.
Authorization creation and revocation pages web/src/routes/(app)/teams/[id]/keys/[pubkey]/authorizations/new/+page.svelte, web/src/routes/(app)/teams/[id]/keys/[pubkey]/+page.svelte	New page collects nickname input, trims and includes name as null when empty in POST payload; key page adds revokeAuthorization handler confirming action, calling DELETE endpoint, filtering local state, and displaying success/error toasts.
Backend tests for authorization features api/src/api/http/teams.rs	Test module adds TestKeyManager pass-through implementation; test setup applies migration and sets global KEYCAST_STATE; add_authorization test verifies name persistence and trimming; remove_authorization tests verify targeted deletion and access control rejection via mismatched key paths.

Nostr Connect Refactoring

Layer / File(s)	Summary
Nostr signer options and support detection web/src/lib/nostr.ts	New NostrConnectSigninOptions type parameterizes relays and signerKind; createNostrConnectSigninSession accepts options object instead of array parameter; isAmberSigninSupported() checks Android user agent instead of AmberClipboardSigner.SUPPORTED; getAmberUser() function removed.
Sign-in menu Nostr Connect integration web/src/lib/components/SignInMenu.svelte	New ConnectSessionStartOptions type with autoOpen, busyState, and optional signerKind; BusyState type expanded to include "amber"; startConnectLink and new startAmberConnect delegate to generalized startConnectSession; Amber option retitled to "Connect with Amber" with updated readiness and busy text.
Auth utilities and sign-in method simplification web/src/lib/utils/auth.ts	SigninMethod union reduced from `"extension"
Nostr Connect test coverage web/src/lib/utils/nostr.test.ts	Imports createNostrConnectSigninSession; adds test verifying Amber signer session creation produces nostrconnect: URI with correct perms query parameter.

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Improve authorization management' accurately summarizes the main changes: adding authorization nicknames, revoke functionality, and Nostr Connect integration.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/amber-auth-authorization-management

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

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)

web/src/lib/components/SignInMenu.svelte (1)

92-126: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Clear busy when a connect session is canceled.

If the user clicks Cancel, Lines 133-135 abort the session and null out connectController before the finally block runs. That makes the guard on Line 121 fail, so busy never resets and the modal stays disabled until it is closed and reopened.

Suggested fix

 function cancelConnectLink() {
     connectController?.abort();
     connectSession?.cancel();
+    busy = null;
     connectController = null;
     connectSession = null;
     connectUri = "";
     connectError = null;
 }

Also applies to: 132-139

🤖 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 `@web/src/lib/components/SignInMenu.svelte` around lines 92 - 126, The finally
guard in startConnectSession prevents clearing busy when a session is canceled
because cancel logic nulls connectController before finally; update the finally
block in startConnectSession so it clears busy and resets connectController when
the current controller was used OR when the controller has been aborted (e.g.,
check controller.signal.aborted or similar), referencing connectController,
busy, controller and controller.signal.aborted to ensure busy is always cleared
on cancellation.

🤖 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 `@web/src/routes/`(app)/teams/[id]/keys/[pubkey]/+page.svelte:
- Around line 109-125: The call to buildAuthHeader can throw but is currently
outside the try/catch so users get no feedback; move the await
api.buildAuthHeader(endpoint, "DELETE", user.pubkey) into the same try block (or
wrap it in its own try/catch) so any signing/header generation errors are caught
and reported via toast.error, then only call api.delete with the resulting
authHeader and update authorizations (referencing buildAuthHeader, api.delete,
authorizationId, authorizations, and toast.error).

---

Outside diff comments:
In `@web/src/lib/components/SignInMenu.svelte`:
- Around line 92-126: The finally guard in startConnectSession prevents clearing
busy when a session is canceled because cancel logic nulls connectController
before finally; update the finally block in startConnectSession so it clears
busy and resets connectController when the current controller was used OR when
the controller has been aborted (e.g., check controller.signal.aborted or
similar), referencing connectController, busy, controller and
controller.signal.aborted to ensure busy is always cleared on cancellation.

🪄 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: 09730916-132c-49ee-812f-4633f92ba6e8

📥 Commits

Reviewing files that changed from the base of the PR and between 1f0610c and 1de10e6.

📒 Files selected for processing (14)

• api/src/api/http/routes.rs
• api/src/api/http/teams.rs
• api/src/api/types.rs
• core/src/database.rs
• core/src/types/authorization.rs
• database/migrations/0003_add_authorization_name.sql
• web/src/lib/components/AuthorizationCard.svelte
• web/src/lib/components/SignInMenu.svelte
• web/src/lib/nostr.ts
• web/src/lib/types.ts
• web/src/lib/utils/auth.ts
• web/src/lib/utils/nostr.test.ts
• web/src/routes/(app)/teams/[id]/keys/[pubkey]/+page.svelte
• web/src/routes/(app)/teams/[id]/keys/[pubkey]/authorizations/new/+page.svelte