Skip to content

  • Projects
  • Groups
  • Snippets
  • Help
    • Loading...
    • Help
    • Submit feedback
    • Contribute to GitLab
  • Sign in
haskell-gargantext
haskell-gargantext
  • Project
    • Project
    • Details
    • Activity
    • Releases
    • Cycle Analytics
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
    • Charts
  • Issues 166
    • Issues 166
    • List
    • Board
    • Labels
    • Milestones
  • Merge Requests 11
    • Merge Requests 11
  • CI / CD
    • CI / CD
    • Pipelines
    • Jobs
    • Schedules
    • Charts
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Members
    • Members
  • Collapse sidebar
  • Activity
  • Graph
  • Charts
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
  • gargantext
  • haskell-gargantexthaskell-gargantext
  • Issues
  • #276

Closed
Open
Opened Oct 09, 2023 by Alfredo Di Napoli@AlfredoDiNapoli
  • Report abuse
  • New issue
Report abuse New issue

`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:

  1. It calls user id what is morally speaking a NodeId;
  2. 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:

  1. Give a UserId a proper netwtype, so that we get compile time errors if we try to mix and match the two, or convert between the two which is wrong;

  2. 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.

cc @anoe @cgenie

Assignee
Assign to
None
Milestone
None
Assign milestone
Time tracking
None
Due date
None
0
Labels
None
Assign labels
  • View project labels
Reference: gargantext/haskell-gargantext#276