stjude / proteinpaint

Data visualization and analysis framework focused on phenotype-molecular data integration at cohort level.
https://proteinpaint.stjude.org/
Other
13 stars 3 forks source link

Move jwt serverconfig handling bellow the bodyParser.json handling #1749

Closed aacic closed 2 weeks ago

aacic commented 2 weeks ago

Description

I moved jwt serverconfig handling bellow the bodyParser.json handling.

To test:

Add to serverconfig.json:

  "jwt": {
    "secret": "XXX",
    "permissioncheck": "clinical_permission"
  }

curl -X POST http://localhost:3000/ -H "Content-Type: application/json" -d '{"jwt":"value"}'

Should get: {"error":"Invalid token"}

Instead of: {"error":"json web token missing"}

Checklist

Check each task that has been performed or verified to be not applicable.

siosonel commented 2 weeks ago

@aacic there is already app.use(bodyParser.json({ limit: '5mb' })) in setAppMiddlewares(). Maybe move that code before the jwt processing? We shouldn't need two middlewares to process json payloads.

siosonel commented 2 weeks ago

and jwt should ideally be in the header as Authorization: Bearer xxxx or X-Auth-Token or domain-based cookie (best), although some legacy usage submits it in the body which is less secure but okay for a few use cases, like a private portal

aacic commented 2 weeks ago

@aacic there is already app.use(bodyParser.json({ limit: '5mb' })) in setAppMiddlewares(). Maybe move that code before the jwt processing? We shouldn't need two middlewares to process json payloads.

@siosonel this is fixed.