Skip to content

Feature: List and Delete API Keys Endpoints#246

Merged
llam36 merged 50 commits intomainfrom
feat/69-spring-2026-api-key-creation-page
Apr 12, 2026
Merged

Feature: List and Delete API Keys Endpoints#246
llam36 merged 50 commits intomainfrom
feat/69-spring-2026-api-key-creation-page

Conversation

@kworathur
Copy link
Copy Markdown
Collaborator

@kworathur kworathur commented Mar 9, 2026

This pull requests adds the backend functionality for issue #69 in juno-dashboard

  • an API endpoint DELETE /auth/key/:id for deleting API keys by ID
  • an API endpoint GET /key/auth/all for listing all API keys in Juno (superadmin) or all API keys associated with linked projects (admin). Supports pagination url parameters query and offset.
  • end-to-end tests in the api-gateway and db-service packages.
  • Note: the revokeApiKey route of the auth controller for api-gateway package has been deprecated. The reason for this decision is that the method uses the authorization header for both auth and identification of the resource to be deleted, it is usually preferred to separate auth from idenfication.

@kworathur kworathur marked this pull request as ready for review March 10, 2026 21:55
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 10, 2026

Greptile Summary

This PR adds two new API endpoints — DELETE /auth/key/:id for deleting API keys by ID and GET /auth/key/all for listing all API keys with pagination — along with updated protobuf definitions, a Prisma migration adding createdAt to the ApiKey model, and end-to-end tests across the api-gateway and db-service packages. The ownership check for deletion and admin-type guard for listing are correctly implemented, and the db-service layer was properly refactored to push project filtering into the Prisma WHERE clause.

Issues needing attention before merge:

  • Pagination arithmetic is broken: next/prev links increment offset by 1 (offset ± 1), but offset is a Prisma skip count (rows to skip). Consecutive pages overlap heavily — e.g., offset=0&limit=10 produces a next link of offset=1&limit=10 (skipping only 1 row). The step should be limit.
  • Division by zero when limit=0: A caller can explicitly pass ?limit=0; ParseIntPipe passes it through, and Math.floor(count / 0) = Infinity gets embedded into the pagination URL strings.
  • console.debug statements left in production code: Two console.debug calls remain at lines 410 and 430 of auth.controller.ts.
  • Swallowed catch block returns undefined: The try/catch around pagination-link construction swallows exceptions silently; the function then returns undefined, which NestJS sends as a 200 OK with an empty body.
  • Hardcoded 500 status code on line 315 — use HttpStatus.INTERNAL_SERVER_ERROR for consistency.
  • Stray ?projectId=0 in the delete test — not a recognised query parameter, silently ignored, making the test misleading.

PR Checklist areas needing improvement:

  • ❌ No console.logs or commented-out code — console.debug calls at lines 410 and 430 must be removed.
  • ❌ Uses try/catch blocks for error handling — the catch block at line 429 swallows errors instead of propagating them.
  • ❌ Uses status codes correctly — 500 is hardcoded on line 315.
  • ❌ Operations are optimised — the SUPERADMIN path fetches all projects via an extra gRPC round-trip before listing keys.

Score: 62/100

Confidence Score: 2/5

Not safe to merge — pagination links are semantically incorrect and a zero limit silently corrupts the response.

Core security checks are correctly in place, but GET /auth/key/all has a functional pagination bug (offset incremented by 1 instead of limit), a division-by-zero path when limit=0, a swallowed catch block that returns undefined, and stray console.debug calls — all affecting correctness and reliability in production.

packages/api-gateway/src/modules/auth/auth.controller.ts requires the most attention — pagination arithmetic, division-by-zero guard, console.debug removal, and the silent catch block all need fixing before this is production-ready.

Important Files Changed

Filename Overview
packages/api-gateway/src/modules/auth/auth.controller.ts Adds DELETE /auth/key/:id and GET /auth/key/all endpoints; contains pagination arithmetic bug (offset±1 instead of offset±limit), division-by-zero risk when limit=0, stray console.debug statements, swallowed catch block, and hardcoded 500 status code.
packages/auth-service/src/modules/api_key/api_key.controller.ts Adds getAllApiKeys and deleteApiKey relay methods; the limit: request.limit ?? undefined pattern still passes 0 when limit is absent.
packages/db-service/src/modules/auth/auth.controller.ts Previously-flagged in-memory filtering replaced with a proper Prisma OR whereClause; project-ownership validation restored via validateApiKeyIdentifier; logic is now correct.
packages/db-service/src/modules/auth/auth.service.ts apiKeys() now throws an RpcException on failure (previously swallowed); correctly uses a Prisma transaction for count+fetch; looks good.
packages/proto/definitions/api_key.proto GetAllApiKeysParams still uses plain int32 (not optional) for offset/limit, meaning an absent limit serializes as 0 on the wire — previously flagged issue not yet resolved.
packages/api-gateway/test/auth.e2e-spec.ts Tests now correctly target /auth/key/all and /auth/key/:id; one test passes a stray ?projectId=0 param (ignored by the server but misleading); coverage is good.

Sequence Diagram

sequenceDiagram
    participant Client
    participant ApiGateway as API Gateway
    participant AuthService as Auth Service
    participant DbService as DB Service
    participant Prisma

    Note over Client,Prisma: DELETE /auth/key/:id
    Client->>ApiGateway: DELETE /auth/key/:id (user JWT)
    ApiGateway->>ApiGateway: Parse & validate id
    ApiGateway->>AuthService: getApiKey({ id })
    AuthService->>DbService: getApiKey(identifier)
    DbService->>Prisma: findUnique({ where: {id} })
    Prisma-->>DbService: ApiKey row
    DbService-->>AuthService: ApiKey proto
    AuthService-->>ApiGateway: GetApiKeyResponse
    ApiGateway->>ApiGateway: userLinkedToProject check
    ApiGateway->>AuthService: deleteApiKey({ id })
    AuthService->>DbService: deleteApiKey(identifier)
    DbService->>Prisma: delete({ where: {id} })
    Prisma-->>DbService: deleted ApiKey
    DbService-->>AuthService: ApiKey proto
    AuthService-->>ApiGateway: DeleteApiKeyResponse
    ApiGateway-->>Client: 204 No Content

    Note over Client,Prisma: GET /auth/key/all?offset=N&limit=M
    Client->>ApiGateway: GET /auth/key/all (user JWT)
    ApiGateway->>ApiGateway: Determine user type
    alt SUPERADMIN
        ApiGateway->>AuthService: getAllProjects (extra round-trip)
        AuthService-->>ApiGateway: projects[]
    end
    ApiGateway->>AuthService: getAllApiKeys({ projects, offset, limit })
    AuthService->>DbService: getAllApiKeys({ projects, offset, limit })
    DbService->>Prisma: findMany({ where: OR[projectIds], skip, take })
    Prisma-->>DbService: ApiKey[]
    DbService-->>AuthService: GetAllApiKeysResult
    AuthService-->>ApiGateway: GetAllApiKeysResponse
    ApiGateway->>ApiGateway: Build pagination links (offset+/-1 bug)
    ApiGateway-->>Client: 200 GetAllApiKeysResponse
Loading

Reviews (5): Last reviewed commit: "chore: clean up unused imports" | Re-trigger Greptile

Comment thread packages/db-service/src/modules/auth/auth.controller.ts Outdated
Comment thread packages/db-service/src/modules/auth/auth.controller.ts
Comment thread packages/api-gateway/test/auth.e2e-spec.ts
Comment thread packages/api-gateway/src/modules/auth/auth.controller.ts
Comment thread packages/api-gateway/src/modules/project/project.controller.ts Outdated
Comment thread packages/api-gateway/src/modules/auth/auth.controller.ts Outdated
Comment thread packages/api-gateway/src/modules/project/project.controller.ts Outdated
@kworathur kworathur marked this pull request as draft March 11, 2026 13:41
@kworathur kworathur changed the title Feature: List and Bulk Delete API Keys Endpoint Feature: List and Delete API Keys Endpoints Mar 15, 2026
@kworathur kworathur closed this Mar 15, 2026
@kworathur kworathur reopened this Mar 15, 2026
@kworathur kworathur marked this pull request as ready for review March 15, 2026 17:55
Comment thread packages/api-gateway/src/modules/auth/auth.controller.ts Outdated
Comment thread packages/api-gateway/src/modules/auth/auth.controller.ts Outdated
Comment thread packages/db-service/src/modules/auth/auth.service.ts
Comment thread packages/proto/definitions/api_key.proto
kworathur and others added 3 commits March 15, 2026 18:55
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@kworathur kworathur marked this pull request as draft March 15, 2026 22:58
@kworathur kworathur marked this pull request as ready for review April 3, 2026 19:03
Comment thread packages/api-gateway/src/modules/auth/auth.controller.ts Outdated
Comment thread packages/api-gateway/src/modules/auth/auth.controller.ts Outdated
Comment thread packages/api-gateway/src/modules/auth/auth.controller.ts
Comment thread packages/api-gateway/src/modules/auth/auth.controller.ts
Comment thread packages/api-gateway/src/modules/auth/auth.controller.ts
Comment thread packages/api-gateway/test/auth.e2e-spec.ts
kworathur and others added 7 commits April 3, 2026 18:04
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Copy link
Copy Markdown
Collaborator

@aakashg00 aakashg00 left a comment

Choose a reason for hiding this comment

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

looks good

@llam36 llam36 merged commit df97e7d into main Apr 12, 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.

3 participants