Fix: AUTHORIZED_ACCESS is now returned as a privilege - #755
Conversation
Gcolon021
changed the title
Gcolon021
added
bug
|
Great job! No new security vulnerabilities introduced in this pull requestUse @Checkmarx to interact with Checkmarx PR Assistant. |
587fa74 to
820bd22
Compare
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Can you add some tests for the actual auth guards, not just nav? I get its not exactly your fault there are no tests but I think we probably should have some. One case with |
getUser pushed a synthetic AUTHORIZED_ACCESS onto the privilege list whenever /user/me returned any name containing PRIV_MANAGED_. PSAMA stopped minting per-study privileges when study authorization moved to user_consents, so the substring never matched for users created since, and the explorer and analysis routes disappeared for them while older accounts kept working off stale rows. The name now arrives for real: the auth migration hangs AUTHORIZED_ACCESS off MANUAL_ROLE_AUTH_ACCESS, which every authenticated user already holds. Drop the inference and the PRIV_MANAGED prefix it relied on. Cover the four route guards, each OR of two privilege names, with one case per arm. Testing a single arm cannot detect the other holding a name PSAMA does not send, which is how this shipped: the authorized guard was only ever reached through PIC_SURE_ANY_QUERY, its negative case passed [] and so failed both arms at once, and the analyze, dataset and admin guards had no tests at all. The authorized layout test also mocked the privilege enum with invented values (PRIV_QUERY, PRIV_AUTHORIZED_ACCESS), supplying both the name the code checks and the name the fixture provides; it now uses the real enum. test:vitest filtered to tests/unit and tests/component, so nothing under src/routes ran in CI, including the only existing guard test. Add src/routes. Deploy after the migration has run. Until then no user receives the name and the routes stay hidden for everyone rather than just for new accounts.
820bd22 to
d763bcc
Compare
3 participants
No description provided.