Skip to content

[Feature] : API ENDPOINTS PR 2: Auth#1131

Open
pulk17 wants to merge 2 commits into
CCExtractor:masterfrom
pulk17:api-pr2-auth
Open

[Feature] : API ENDPOINTS PR 2: Auth#1131
pulk17 wants to merge 2 commits into
CCExtractor:masterfrom
pulk17:api-pr2-auth

Conversation

@pulk17

@pulk17 pulk17 commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

[FEATURE]

In raising this pull request, I confirm the following (please check boxes):

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • I have considered, and confirmed that this submission will be valuable to others.
  • I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  • I give this submission freely, and claim no ownership to its content.

My familiarity with the project is as follows (check one):

  • I have never used the project.
  • I have used the project briefly.
  • I have used the project extensively, but have not contributed previously.
  • I am an active contributor to the project.

Overview

Second PR of the CI API stack (#1130#1141, merge bottom-up). Builds on the auth middleware from #1130 and adds the token lifecycle — the only part of the API usable with just an email and password; everything else requires a token minted here.

Endpoints

Endpoint Auth Purpose
POST /auth/tokens email + password Mint a scoped bearer token
GET /auth/tokens tokens:manage + admin role List tokens (own or ?all=true)
DELETE /auth/tokens/current any valid token Self-revocation
DELETE /auth/tokens/{id} tokens:manage + admin role Revoke by id

Design decisions

  • The plaintext token is returned exactly once, in the creation response, and never stored or shown again. List responses contain metadata only — no token material, not even the prefix.
  • Scopes are validated against the requester's role at mint time: a contributor cannot mint baselines:write even by asking for it (403 with the missing scopes in details). Default expiry 7 days, max 30.
  • Self-revocation is deliberately scope-free: possession of a valid token is sufficient authority to destroy that token. Requiring tokens:manage here would force every automation token to carry an admin-level scope just to be able to clean up after itself. Revoke-by-id, which acts on other tokens, does require it.
  • Token names stay reserved after revocation — revoked rows are kept for audit, and the unique constraint spans them. The 409-style conflict message states this explicitly instead of leaving users confused about a name they "deleted". Uniqueness is also enforced at the DB level, and the IntegrityError path is handled (and tested) for races between the pre-check and the insert.

Also in this PR

  • Integration tests for the error-handler and rate-limit middleware (they need a live endpoint to exercise; POST /auth/tokens is the first).
  • One-line tests/test_ci fix: test_webhook_prerelease now mocks requests.get like its sibling webhook tests, removing a network dependency.

Testing

23 new tests: happy paths, invalid credentials, role/scope rejection, duplicate names, the IntegrityError race, revocation flows (revoked token immediately 401s), and rate-limit/error-handler behavior. 65 total at this layer.

@pulk17 pulk17 changed the title Api pr2 auth [Feature] : API ENDPOINTS PR 2: Auth Jun 24, 2026
@cfsmp3
cfsmp3 self-requested a review June 24, 2026 16:35

@cfsmp3 cfsmp3 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing tests:

  • Rate limiter still untested (tests bypass it via TESTING + _rate_limit_store.clear()). The 5/15min create limit is unverified.
  • H4 fix (500→JSON) still unverified — no test forces a real route 500.

Bug:

  • Non-admins can still request tokens:manage scope (harmless given role gates, but loose).

@sonarqubecloud

Copy link
Copy Markdown

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