Page MenuHomePhabricator

[keyserver] refactor JSON endpoints to require policy acknowledgment
ClosedPublic

Authored by kamil on Dec 6 2022, 6:08 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Dec 15, 8:30 PM
Unknown Object (File)
Sun, Dec 15, 8:30 PM
Unknown Object (File)
Sun, Dec 15, 8:30 PM
Unknown Object (File)
Sun, Dec 15, 8:29 PM
Unknown Object (File)
Sun, Dec 15, 8:20 PM
Unknown Object (File)
Fri, Dec 6, 5:54 PM
Unknown Object (File)
Fri, Nov 29, 8:57 PM
Unknown Object (File)
Nov 8 2024, 9:59 AM
Subscribers

Details

Summary

This code modifies POST request type, each of them will need to specify an array of needed policies, that will be checked before accessing endpoint. An empty array means that this endpoint doesn't need acknowledgment or information about a missing acknowledgments will be attached to the response later on. Looks like other handlers, e.g. for GET requests don't need to check for policies, there is no need to complicate this because this change will still make users unable to use the app without accepting what's needed.

Context in ENG-2363.

Test Plan

Endpoints should maintain old functionality, this is just a refactor.

Diff Detail

Repository
rCOMM Comm
Branch
policy-stuff-5
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

kamil held this revision as a draft.
kamil published this revision for review.Dec 6 2022, 6:55 AM
tomek added a reviewer: ashoat.

Adding @ashoat to make sure if this makes sense, especially if not checking policies on GET is ok.

keyserver/src/endpoints.js
67 ↗(On Diff #19176)

I think it might make sense to create a constant with policies used by most of the endpoints. I guess that if we decide to have another policy required, this will affect almost all the endpoints at once.

Another option is to create two object: one with endpoints that require a policy and one with the ones that don't need it and then construct jsonEndpoints from these two by adding array with policy to one set and empty array to another.

Seems right

especially if not checking policies on GET is ok.

What APIs do we have GET for? The only GETs I can think of are the ones that return HTML or files (error report JSON, uploads). I think the HTML ones need to be special-cased and the others are fine for now

This revision is now accepted and ready to land.Dec 6 2022, 7:38 PM

instroduce constant with common policies

keyserver/src/endpoints.js
67 ↗(On Diff #19176)

I think it might make sense to create a constant with policies used by most of the endpoints. I guess that if we decide to have another policy required, this will affect almost all the endpoints at once.

I'll stick to this one, the second is also okay but I think only for one policy, it might get tricky when we will have more