feat(server): add structured logger, global error handler, shared asy… - #51
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR: authenticate & authorize Middleware
Summary
Introduces a canonical
middleware/auth.tswith anauthenticatefunctionthat distinguishes expired tokens from invalid ones, and an
authorize(...roles)factory for role-based access control. All protected routes (trades, wallet,
admin) are wired through the new middleware. Existing route imports are kept
working via a backwards-compatible shim. Unit tests cover every acceptance
criterion with 20 cases.
Closes #34
Type of Change
feat— new featurerefactor— code change with no behaviour change (authenticate.ts shim)What Changed
server/src/middleware/auth.tsauthenticate,authorize,AuthPayload,AuthenticatedRequestserver/src/middleware/authenticate.tsauth.tsserver/src/middleware/auth.test.tsserver/src/routes/admin.tsauthenticate + authorize("admin")server/src/routes/auth.tsrole; JWT sign payload includesroleserver/src/routes/trades.ts../middleware/authserver/src/routes/wallet.ts../middleware/authserver/src/index.tsadminRouterat/api/adminArchitecture
Middleware signatures
Error matrix
Authorizationheader{ "error": "Unauthorized" }Bearer{ "error": "Unauthorized" }Bearer{ "error": "Unauthorized" }{ "error": "Unauthorized" }expclaim{ "error": "Token expired" }{ "error": "Forbidden" }JWT_SECRETenv var missing{ "error": "Internal server error" }Route protection map
Backwards compatibility
authenticate.tsis kept as a one-file re-export shim:All existing route files that imported from
../middleware/authenticatecontinueto work. New code should import directly from
../middleware/auth.Role in JWT
jwt.signnow includesrolein the payload:{ "sub": "<userId>", "stellarPublicKey": "<G…>", "role": "user" }authenticatenormalises legacy tokens (noroleclaim) torole = "user"automatically so existing sessions don't break.
DB migration required
The
userstable needs arolecolumn:How to Test
Checklist
General
API changes
role— documented aboveDatabase changes
Docs changes
docs/api-reference.md— admin endpoints and the new 403 response should be added (follow-up)Notes for Reviewer
authorizeis fail-closed: if called without a priorauthenticate(soreq.userisundefined), it returns 401 rather than throwing a runtimeerror. This makes accidental mis-ordering safe.
JWT_SECRETmissing at runtime returns 500 and logs toconsole.error.This matches the fail-closed pattern used in
authenticate.tsand ispreferable to allowing unauthenticated requests through.
PATCH /api/admin/trades/:id/releaseroute currently updates the DBdirectly. The full
release_paymentSoroban call (viastellar.ts) shouldbe wired in as a follow-up once the contract address is stable in the target
environment.