Skip to content

Aakash - Add Accept account request to SDK + support JWT for existing request methods#73

Merged
llam36 merged 10 commits intomainfrom
aakash/req-acc-jwt
Apr 13, 2026
Merged

Aakash - Add Accept account request to SDK + support JWT for existing request methods#73
llam36 merged 10 commits intomainfrom
aakash/req-acc-jwt

Conversation

@aakashg00
Copy link
Copy Markdown
Contributor

Adds an accept account request method so that requests can be accepted from the frontend. Also adds JWT support for authenticated methods, as before it required email/password.
Depends on GTBitsOfGood/juno#245 to be merged first

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 9, 2026

Greptile Summary

This PR adds an acceptAccountRequest method to the SDK so that pending account requests can be accepted from the frontend, and retrofits the existing getAllAccountRequests and deleteAccountRequest methods to accept a UserCredentials union type (string JWT or { email, password } object) instead of requiring email/password exclusively.

Key changes and findings:

  • New acceptAccountRequest public method in src/lib/auth.ts and the corresponding low-level authControllerAcceptAccountRequest in src/internal/api/authApi.ts, following the same credential-switching pattern as the refactored methods.
  • Shared mutable state bug: When JWT credentials are used, this.internalApi.accessToken is set on the shared AuthApi instance but never cleared in the email/password branch. A subsequent email/password call on the same AuthAPI instance will inadvertently forward the stale JWT in the request. This affects getAllAccountRequests, acceptAccountRequest, and deleteAccountRequest.
  • Inconsistent error handling: The three new/refactored methods omit the try/catch wrapper present in all other methods (createKey, revokeKey, getUserJWT, etc.), creating a stylistic inconsistency that could complicate future error-handling additions.
  • Checklist score: 78/100 — The implementation is well-structured and the new model files are correctly registered. The stale-state bug is the primary concern before merging.

Confidence Score: 3/5

  • Not safe to merge as-is — the shared mutable accessToken state causes stale JWT tokens to bleed into email/password authenticated calls on the same instance.
  • The new model files and low-level API method are correct and well-formed. However, the shared accessToken state mutation in src/lib/auth.ts is a real functional bug: once any JWT-authenticated call is made on an AuthAPI instance, all subsequent email/password calls on that same instance will also carry the stale JWT. This could cause unexpected authentication behavior and potential security concerns depending on how the backend prioritises the two auth mechanisms.
  • src/lib/auth.ts — the else (email/password) branches in getAllAccountRequests, acceptAccountRequest, and deleteAccountRequest all need to reset this.internalApi.accessToken before making the API call.

Important Files Changed

Filename Overview
src/lib/auth.ts Refactors getAllAccountRequests and deleteAccountRequest to accept a UserCredentials union type (JWT string or email/password object), adds new acceptAccountRequest method, and introduces a UserType string literal type — but the email/password branches do not reset this.internalApi.accessToken, causing stale JWTs to leak into subsequent calls.
src/internal/api/authApi.ts Adds authControllerAcceptAccountRequest low-level API method and makes xUserPassword/xUserEmail optional in authControllerDeleteAccountRequest and authControllerGetAllAccountRequests to support JWT-only auth — minor missing blank line between methods.
src/internal/model/acceptAccountRequestResponseModel.ts Auto-generated OpenAPI model for the accept account request response containing a required user and optional project field — no issues found.
src/internal/model/acceptAccountRequestResponseProject.ts Auto-generated OpenAPI model for the project sub-object in the accept account request response — no issues found.
src/internal/model/acceptAccountRequestResponseUser.ts Auto-generated OpenAPI model for the user sub-object in the accept account request response, including a TypeEnum for SUPERADMIN/ADMIN/USER — no issues found.
src/internal/model/models.ts Registers the three new model classes in the typeMap and enumsMap used by ObjectSerializer — straightforward and correctly follows the established pattern.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant AuthAPI as AuthAPI (src/lib/auth.ts)
    participant InternalApi as AuthApi (internal)
    participant Backend as Juno Backend

    Note over AuthAPI,InternalApi: JWT credential path
    Caller->>AuthAPI: getAllAccountRequests(credentials: string)
    AuthAPI->>InternalApi: set accessToken = jwt
    AuthAPI->>InternalApi: authControllerGetAllAccountRequests(undefined, undefined)
    InternalApi->>Backend: GET /auth/account-requests [jwt header]
    Backend-->>Caller: NewAccountRequestsResponse

    Note over AuthAPI,InternalApi: Email/password credential path
    Caller->>AuthAPI: acceptAccountRequest(id, credentials: object)
    Note over AuthAPI: accessToken from prior call still set
    AuthAPI->>InternalApi: authControllerAcceptAccountRequest(id, password, email)
    InternalApi->>Backend: POST /auth/account-request/id/accept [email+password headers + stale jwt]
    Backend-->>Caller: AcceptAccountRequestResponseModel

    Note over AuthAPI,InternalApi: deleteAccountRequest follows same pattern
Loading

Last reviewed commit: f1bb0ac

Comment thread src/lib/auth.ts
Comment thread src/internal/api/authApi.ts Outdated
Comment thread src/lib/auth.ts
@aakashg00 aakashg00 force-pushed the aakash/req-acc-jwt branch from 34b11cc to bcc7122 Compare March 9, 2026 08:02
@aakashg00 aakashg00 requested a review from llam36 April 2, 2026 19:58
@llam36 llam36 merged commit bfab880 into main Apr 13, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants