fireproof icon indicating copy to clipboard operation
fireproof copied to clipboard
verified publisher icon fireproof-storage

feat: add shareWithUser endpoint

Open necrodome opened this issue 3 months ago • 5 comments

Summary by CodeRabbit

  • New Features

    • Users can share ledgers with others via email.
    • Configurable roles and permissions when granting access.
    • Prevents sharing a ledger with oneself.

  • Security

    • Improved token verification and session handling for share actions.
    • Clearer authorization and not-found error messages during sharing flows.

necrodome avatar Oct 10 '25 18:10 necrodome

Walkthrough

Adds ledger-sharing-by-email: new request/response message types and a type guard; FP API method to share a ledger with a user (token verification, auth checks, DB updates); request handler routing for the new message; minor import reorganizations.

Changes

Cohort / File(s) Summary
Message types & guards
core/protocols/dashboard/msg-types.ts, core/protocols/dashboard/msg-is.ts		 Add ReqShareWithUser and ResShareWithUser interfaces; add/import ReqShareWithUser and implement isReqShareWithUser type guard in FAPIMsgImpl.
Backend API logic
dashboard/backend/api.ts		 Add shareWithUser(req: ReqShareWithUser): Promise<Result<ResShareWithUser>> to FPApiInterface/FPApiSQL; add private verifyFPToken method; update token verification/extension flow and related imports/types.
Request routing / handler
dashboard/backend/create-handler.ts		 Add case for FPAPIMsg.isReqShareWithUser(jso) to route to fpApi.shareWithUser; reorder imports.
HTTP server imports
dashboard/backend/cf-serve.ts		 Reorganize imports from Cloudflare types/modules (import ordering/explicit CFRequest/CFResponse; no functional change).
Repository metadata
manifest_file, package.json		 Metadata / dependency-related files touched per diff (updates not detailed in diff summary).

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Client
    participant Handler as Request Handler
    participant API as FPApiSQL
    participant DB as Database
    participant JWT as Token Verifier

    Client->>Handler: POST ReqShareWithUser
    activate Handler
    Handler->>Handler: isReqShareWithUser(jso)
    Handler->>API: shareWithUser(req)
    deactivate Handler

    activate API
    API->>JWT: verifyFPToken(auth token)
    alt token invalid
        JWT-->>API: Error
        API-->>Client: Error (Invalid token)
    else token valid
        API->>DB: Lookup ledger & verify admin
        alt ledger not found
            API-->>Client: Error (Not found)
        else not admin
            API-->>Client: Error (Not authorized)
        else admin
            API->>DB: Lookup user by email
            API->>DB: Add user to ledger (role/right)
            API-->>Client: ResShareWithUser (success)
        end
    end
    deactivate API

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Review verifyFPToken for correct JWT validation, error handling, and integration with extendToken/createFPToken.
  • Inspect shareWithUser for authorization checks (ledger admin verification), self-share prevention, and correct DB updates (user lookup, tenant/ledger updates).
  • Confirm ReqShareWithUser / ResShareWithUser shapes align with handler and clients and that isReqShareWithUser correctly narrows input.
  • Check import reorganizations in create-handler.ts and cf-serve.ts for unused or missing imports.

Pre-merge checks and finishing touches

❌ 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%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the primary change: adding a new endpoint for sharing users. It accurately reflects the main objective of the PR.
✨ Finishing touches
  • [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment
  • [ ] Commit unit tests in branch selem/sharing
📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 269c3a3df8a5c4eb4569daffe4b6a332b0dcc767 and 7916aaa9f58d97c5f97c9baa1e5436b7f4d31383.

📒 Files selected for processing (5)
  • core/protocols/dashboard/msg-is.ts (3 hunks)
  • core/protocols/dashboard/msg-types.ts (1 hunks)
  • dashboard/backend/api.ts (8 hunks)
  • dashboard/backend/cf-serve.ts (1 hunks)
  • dashboard/backend/create-handler.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • core/protocols/dashboard/msg-types.ts
  • core/protocols/dashboard/msg-is.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-09T19:56:31.545Z 
Learnt from: mabels
Repo: fireproof-storage/fireproof PR: 1130
File: core/device-id/device-id-protocol.ts:47-60
Timestamp: 2025-09-09T19:56:31.545Z
Learning: In the Result type from adviser/cement, Result.Err can accept Result objects directly without needing to unwrap them with .Err(). The codebase consistently uses Result.Err(resultObject) pattern, as seen in core/keybag/key-bag.ts lines 94 and 102.

Applied to files:

  • dashboard/backend/api.ts
🧬 Code graph analysis (2)
dashboard/backend/api.ts (5)
core/protocols/dashboard/msg-types.ts (4)
  • ReqShareWithUser (407-413)
  • ResShareWithUser (415-424)
  • ReqExtendToken (518-521)
  • ResExtendToken (523-526)
dashboard/backend/create-fp-token.ts (3)
  • FPTokenContext (7-14)
  • getFPTokenContext (31-53)
  • createFPToken (16-29)
dashboard/backend/ledgers.ts (1)
  • sqlLedgers (7-25)
dashboard/backend/users.ts (1)
  • queryUser (56-62)
core/types/protocols/cloud/msg-types.zod.ts (1)
  • FPCloudClaim (43-43)
dashboard/backend/create-handler.ts (1)
dashboard/backend/api.ts (1)
  • FPAPIMsg (127-127)
🔇 Additional comments (3)
dashboard/backend/cf-serve.ts (1)

1-2: Typed Cloudflare Request/Response wiring looks good

Aliasing Request/Response to CFRequest/CFResponse from @cloudflare/workers-types and using them only for env.ASSETS.fetch keeps the worker-global Request/Response free and avoids type confusion. No behavior change here.

dashboard/backend/create-handler.ts (1)

7-11: FP API wiring and Env typing are consistent

The new imports (FPAPIMsg, FPApiSQL, FPApiToken, DashSqlite, Env) and the isReqShareWithUser switch case follow the existing handler pattern: the API surface is extended cleanly and the new request type is routed to fpApi.shareWithUser without altering error handling or CORS behavior.

Also applies to: 304-306

dashboard/backend/api.ts (1)

2070-2119: FP token verification and extension flow look consistent

The new verifyFPToken helper and extendToken implementation correctly:

  • Derive a JWK from ctx.publicToken and call jwtVerify with issuer/audience constraints.
  • Reject expired tokens (redundant with jwtVerify, but safe).
  • Re-issue a token using createFPToken with validFor overridden to ctx.extendValidFor, preserving the original claim payload.

This is a reasonable and self-contained JWT flow; no behavioral issues spotted in these additions.

[!TIP]

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions: | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context. Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.

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 and usage tips.

coderabbitai[bot] avatar Oct 10 '25 18:10 coderabbitai[bot]

From where the double CORS is coming from? --- this is not correct, this will break the local version.

https://github.com/fireproof-storage/fireproof/blob/b957cc6b95feb0e547db1f83456b78cf07d3889b/dashboard/backend/cf-serve.ts#L38

necrodome avatar Oct 11 '25 07:10 necrodome

that had been there for ever, the question where is the second coming from

mabels avatar Oct 14 '25 07:10 mabels

pls use u github handle in the branch name @necrodome

mabels avatar Oct 15 '25 09:10 mabels

move the CORS fix in mabels/fix-double-cors or #1257

mabels avatar Oct 15 '25 09:10 mabels