fix: Add creator_id into CreateFlow mutation #826
Conversation
|
Removed vultr server and associated DNS entries |
editor.planx.uk/src/utils.ts
Outdated
| export function getLoggedInUserId(): number | undefined { | ||
| const jwt = getCookie("jwt"); | ||
| if (!jwt) return; | ||
| const userId = Number( | ||
| (jwtDecode(jwt) as any)["https://hasura.io/jwt/claims"]["x-hasura-user-id"] | ||
| ); | ||
| return userId; | ||
| } |
There was a problem hiding this comment.
do you think it's worth refactoring this block to use the same bit of code?
planx-new/editor.planx.uk/src/index.tsx
Lines 34 to 43 in 67bb8b5
There was a problem hiding this comment.
Good catch - looking at this lead me to have a bit of a rethink, see comment below.
|
@johnrees Had a bit of a rethink on this one - if feels like a bit of an anti-pattern to have a utils function which could be called time and time again to parse the JWT. Decided it makes more sense to deal with the JWT a single time after login, and then set the user details on the React-Navi context to be referred to later. There might be a better place to store this - open to thoughts 👍 |
| slug: $slug | ||
| team_id: $teamId | ||
| version: 1 | ||
| creator_id: $creatorId |
There was a problem hiding this comment.
I think there might be a bit of a security issue here that could get flagged by a security audit in future. It might be that we can ignore it for the time being though*
The issue is - a user could potentially set any creator_id they wanted with some sneaky frontend code tweaking, or by looking at the network request and then replaying it with a different creator_id of their choosing.
The way around it would be to extract the x-hasura-user-id from the JWT in hasura itself, so in the hasura console, you'd set creator_id to have the default value of x-hasura-user-id and have permissions so that creator_id can't be overridden.
e.g.
Doing that should mean that it's impossible for someone to spoof the creator_id.
* However, IIRC we've all got full admin permissions right now anyway, which is why I think we might have bigger problems than this if we had an audit soon. Hmm 🤔
There was a problem hiding this comment.
That's very helpful feedback thanks - I'll take a crack at this approach.
|
Following up here after a chat on Slack about this. The approach of using a column preset suggested by @johnrees will only work custom roles, not the default "admin" role created with Hasura - hasura/graphql-engine#6519 I agree that we should move away from this role, it's likely something that will be flagged in an audit. Closing this PR and creating a ticket on Trello to handle the problem properly - https://trello.com/c/oCV7Vhof/1768-create-more-granular-user-roles |
Addresses issue raised here - #825
Unpacks JWT to get userId, which is then passed into the
CreateFlowmutation.In future, we may want a
UserStoreor similar to keep hold of some user info (e.g. username for Avatar display), but this seemed a pragmatic solution for now. Feedback welcome if there's a better way to do this!