Page MenuHomePhabricator

[keyserver] validate policies only for logged in users
ClosedPublic

Authored by kamil on Sep 26 2023, 3:49 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Dec 9, 6:29 PM
Unknown Object (File)
Tue, Nov 26, 2:52 AM
Unknown Object (File)
Sun, Nov 24, 8:09 PM
Unknown Object (File)
Nov 20 2024, 5:10 PM
Unknown Object (File)
Nov 20 2024, 5:10 PM
Unknown Object (File)
Nov 20 2024, 5:10 PM
Unknown Object (File)
Nov 20 2024, 5:08 PM
Unknown Object (File)
Nov 13 2024, 3:12 AM
Subscribers

Details

Summary

Fix for ENG-4978.

An alternative solution is running policiesReducer only when user is logged in to avoid setting flag in store, but changing this on keyserver seems more appropriate, as keyserver should've never throw policies_not_accepted for anonymous user.

Test Plan

Make sue policies work:

  • tested for live session
  • tested for logged in

Diff Detail

Repository
rCOMM Comm
Branch
fix-prompt
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

kamil held this revision as a draft.
kamil published this revision for review.Sep 26 2023, 4:20 AM

Is it valid to require policy acceptance only when a user is logged in? When a user isn't logged in, we don't allow them to do much, but I'm wondering, if from the legal perspective, we should require them to agree to something.

This revision is now accepted and ready to land.Sep 26 2023, 7:53 AM
In D9295#273224, @tomek wrote:

Is it valid to require policy acceptance only when a user is logged in? When a user isn't logged in, we don't allow them to do much, but I'm wondering, if from the legal perspective, we should require them to agree to something.

Can't find a comment right now, but I remember this was discussed and for logged-out users, we use only default data while returning something, don't allow to use of any features of Comm, and don't collect any sensitive data, so it was okay.
I think this could be reconsidered/consulted with legal specialists while working on policies service.

Related to: https://phab.comm.dev/D9295#274628
There is no reverting change, I've added the phrase Revert D[number] as part of the D9320 test plan and phabricator treated this as a reverting change for this diff 😳😳