`AuthenticatedUser` should store the `NodeId` _and_ `UserId`
Currently AuthenticatedUser
is defined this way:
newtype AuthenticatedUser = AuthenticatedUser
{ _authUser_id :: NodeId
} deriving (Generic)
This definition has a few problems:
- It calls user id what is morally speaking a
NodeId
; - It doesn't store the
UserId
of the user.
This has an impact on the access policy manager I have added as part of #259 (closed): a bug was introduced because a GraphQL route wanted to access a node by its user_id
and I erroneously used isDescendantOf
on the input user_id
and _authUser_id
, but those are really apple and oranges, and it's easy to make mistakes.
Furthermore, our UserId
is currently, IIRC, just a plain Int
, rather than a newtype
wrapper.
I propose the following refactoring:
-
Give a
UserId
a propernetwtype
, so that we get compile time errors if we try to mix and match the two, or convert between the two which is wrong; -
Modify
AuthenticatedUser
this way:
newtype AuthenticatedUser = AuthenticatedUser
{ _authUser_id :: UserId
, _authNode_id :: NodeId
} deriving (Generic)
Now it's clear that _authUser_id
stores a UserId
and _authNode_id
stores the node, and the ambiguity is resolved. It also means we can pass both information to the access policy manager.
I'm not sure if this can be done without modifying the JWT format, and if this will be a breaking change. The alternative would be to lookup the UserId
in the handler that builds an AuthenticatedUser
, but that is not very performant.