feat: Use roles in API #2198
feat: Use roles in API #2198
Conversation
|
Removed vultr server and associated DNS entries |
| @@ -107,7 +101,7 @@ const publishFlow = async ( | |||
| { | |||
| data: flattenedFlow, | |||
| flow_id: req.params.flowId, | |||
| publisher_id: parseInt(req.user.sub, 10), | |||
| publisher_id: parseInt(req.user!.sub!, 10), | |||
There was a problem hiding this comment.
The next PR will handle this a bit more elegantly than forcing !
| */ | ||
| export const useRoleAuth: UseRoleAuth = | ||
| (authRoles) => async (req, res, next) => { | ||
| useJWT(req, res, () => { |
There was a problem hiding this comment.
I spent a while trying to get these two middlewares a little less tightly coupled. The Express docs have a nice example of using an array of middleware - https://expressjs.com/en/guide/using-middleware.html
The issue I was hitting was keeping the return types for req, res, next outside this file when passing in the role argument to the middleware / function which returns a middleware array.
In reality they are tightly coupled - we can't check roles without decoding and validating the JWT, but I don't think this reads particularly clearly.
| "teamViewer", | ||
| "teamEditor", | ||
| "platformAdmin", |
There was a problem hiding this comment.
These arrays indicate that any of these roles will be valid here. See comment on inherited roles here - #2182 (comment)
In future, PlanX roles could be less hierarchical so this approach would still work there also.
| `Authentication error: User ${userId} does have have any of the roles [${authRoles.join( | ||
| ", ", | ||
| )}] which are required to access ${req.path}`, |
There was a problem hiding this comment.
We can probably drop this in future but it could prove handy for debugging any initial issues we have implementing roles.
DafyddLlyr
force-pushed
the
dp/use-roles-in-api
branch
from
58ba9e9 to
6304ef2
Compare
DafyddLlyr
force-pushed
the
dp/use-roles-in-api
branch
from
6304ef2 to
2219fa1
Compare
| .post("/flows/1/move/new-team") | ||
| .set(authHeader({ role: "teamViewer" })) | ||
| .expect(403); | ||
| }); |
There was a problem hiding this comment.
these feel very explicit & readable 👍
| if (!req.user?.sub) | ||
| next({ status: 401, message: "User ID missing from JWT" }); | ||
|
|
||
| app.get("/me", usePlatformAdminAuth, async function (req, res, next) { |
There was a problem hiding this comment.
will any signed in user (including non-platform admins) still be able to call /me if we're using this auth method here? if not, do we want that to be possible? thinking forward to the /me endpoint perhaps underpinning a frontend user store & ability to display your use rdetails in the editor?
There was a problem hiding this comment.
Yeah this is a good catch - I added usePlatformAdminAuth as I was viewing this as a nice way for devs to check their status.
I'll make it accessible to any logged on user again on a follow up PR 👍
For a user store, we could populate it a number of ways -
- Calling
/me - Calling
$admin.user.getByEmail()directly - Enriching the JWT with all required user details
I think you're right here - /me may be the simplest method
What does this PR do?
userRoleAuthteamEditorrole #2182 so that I could work on this in isolation. I'll rebase and tidy this up once the above PR is merged.AsyncLocalStoreagecontext for Express.User throughout the callstack #2199 which illustrates the changes made in this PR.