Skip to content

Aakash - Add Accept account request + support JWT for existing request methods#245

Merged
llam36 merged 8 commits intomainfrom
aakash/accept-req
Apr 13, 2026
Merged

Aakash - Add Accept account request + support JWT for existing request methods#245
llam36 merged 8 commits intomainfrom
aakash/accept-req

Conversation

@aakashg00
Copy link
Copy Markdown
Collaborator

Adds endpoint for accepting account requests for SDK/dashboard to use. This creates a new user using the information from their request, and if they're an admin with an associated project, it creates the project and links them to it. It also deletes the request object.

…isting account request methods to support jwt
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 9, 2026

Greptile Summary

This PR adds a POST /auth/account-request/:id/accept endpoint that creates a user from a pending account request, optionally creates/links a project for ADMIN requests, deletes the request, and returns the created user and project. It also enables JWT-based authentication (alongside existing email/password credentials) for the existing GET /auth/account-request and DELETE /auth/account-request/:id endpoints by marking credential headers as non-required in Swagger and wiring the new accept route through CredentialsMiddleware.

Areas needing improvement:

  • Missing transaction error handlingacceptAccountRequest in auth.service.ts wraps the core work in a $transaction but has no try/catch. A duplicate email (P2002) or a concurrent double-acceptance deleting the request before the second transaction reaches it (P2025) will both surface as opaque gRPC INTERNAL errors rather than meaningful ALREADY_EXISTS or NOT_FOUND responses.
  • TOCTOU race condition — The findUnique existence check for the account request runs outside the transaction. Two concurrent calls with the same ID can both pass the guard and race into the transaction body, triggering the uncaught Prisma errors described above.
  • projectIds is locally derived — The returned projectIds is inferred as [project.id] or [] from local state rather than re-fetched from the DB, which is correct today but fragile if the creation logic changes.
  • No test for duplicate-email scenario — The e2e test suite does not cover accepting a request whose email already belongs to an existing user, which is the main unhandled error path.

Checklist score: 72/100
Items needing attention:

  • Uses try/catch blocks for error handling ❌ (transaction body unprotected)
  • Handles edge cases where necessary ❌ (duplicate email, concurrent acceptance)
  • Operations are optimized to avoid unnecessary computations ⚠️ (existence check duplicated outside/inside transaction)

Confidence Score: 3/5

  • This PR is mostly safe to merge but has real error-handling gaps in the core service method that can leak internal Prisma errors to callers.
  • The overall structure is sound — transactions are used for atomicity, auth guards are correct, proto definitions and DTO mappings are accurate, and test coverage is good. However, the acceptAccountRequest service method lacks a try/catch around the transaction, meaning duplicate-email and concurrent-acceptance scenarios result in unhandled Prisma errors propagating as gRPC INTERNAL rather than meaningful status codes. This is a real production issue, not a theoretical one, since account request emails are user-supplied and could match existing accounts.
  • packages/db-service/src/modules/auth/auth.service.ts requires the most attention — the transaction block needs error handling and the existence check should be moved inside the transaction.

Important Files Changed

Filename Overview
packages/db-service/src/modules/auth/auth.service.ts Adds acceptAccountRequest: uses a Prisma transaction correctly for atomicity, but the NOT_FOUND guard runs outside the transaction (TOCTOU) and the transaction itself has no error handling, so concurrent calls or duplicate emails surface as raw gRPC INTERNAL errors.
packages/db-service/src/modules/auth/auth.controller.ts Adds acceptAccountRequest gRPC handler; maps service result to proto response correctly with mapPrismaRoleToRPC. No issues found.
packages/api-gateway/src/modules/auth/auth.controller.ts Adds the POST /auth/account-request/:id/accept endpoint with correct auth guard, ID parsing, and Swagger documentation. Also updates required: false for credential headers on existing endpoints to support JWT. Clean implementation.
packages/api-gateway/src/models/registration.dto.ts Adds AcceptAccountRequestResponseModel, AcceptAccountRequestResponseUser, and AcceptAccountRequestResponseProject DTOs with proper Swagger annotations and constructor mapping from proto response.
packages/api-gateway/src/modules/auth/auth.module.ts Adds auth/account-request/:id/accept route to the CredentialsMiddleware forRoutes list. Minor reformatting of existing code.
packages/proto/definitions/user.proto Adds acceptAccountRequest RPC to AccountRequestService, along with AcceptAccountRequestMessage and AcceptAccountRequestResponse messages. Uses existing common.User and common.Project types correctly.
packages/db-service/test/auth.e2e-spec.ts Good e2e test coverage for happy paths (USER request, ADMIN+project request, deletion of request post-acceptance, non-existent ID). Missing a test for accepting a request whose email already belongs to an active user.
packages/api-gateway/test/auth.e2e-spec.ts Adds integration tests for the accept endpoint at the gateway layer — covers USER, ADMIN+project, unauthenticated, and invalid ID cases. Well structured.

Sequence Diagram

sequenceDiagram
    participant Client
    participant ApiGateway as API Gateway<br/>(AuthController)
    participant Middleware as CredentialsMiddleware
    participant DbService as DB Service<br/>(ApiKeyDbController)
    participant AuthService as AuthService
    participant DB as Prisma / DB

    Client->>Middleware: POST /auth/account-request/:id/accept<br/>(X-User-Email + X-User-Password OR JWT)
    Middleware->>ApiGateway: resolved User
    ApiGateway->>ApiGateway: guard: user must be ADMIN or SUPERADMIN
    ApiGateway->>ApiGateway: parse & validate id param
    ApiGateway->>DbService: gRPC acceptAccountRequest({ id })
    DbService->>AuthService: acceptAccountRequest(id)
    AuthService->>DB: findUnique NewAccountRequest (outside tx)
    alt not found
        AuthService-->>DbService: RpcException NOT_FOUND
        DbService-->>ApiGateway: gRPC NOT_FOUND
        ApiGateway-->>Client: 404 Not Found
    else found
        AuthService->>DB: $transaction start
        DB->>DB: user.create(email, name, password, type)
        alt userType == ADMIN && projectName set
            DB->>DB: project.findUnique OR project.create
            DB->>DB: user.update (connect project)
        end
        DB->>DB: newAccountRequest.delete
        AuthService-->>ApiGateway: { user, project? }
        ApiGateway-->>Client: 201 AcceptAccountRequestResponseModel
    end
Loading

Last reviewed commit: f47801f

Comment thread packages/db-service/src/modules/auth/auth.service.ts Outdated
Comment thread packages/db-service/src/modules/auth/auth.service.ts Outdated
Comment thread packages/db-service/src/modules/auth/auth.service.ts Outdated
@llam36 llam36 merged commit c69ad0e into main Apr 13, 2026
8 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