HHS / OPRE-OPS

ACF's OPRE OPS product. Code name Unicorn.
Other
12 stars 3 forks source link

Extra Checks for API Endpoints not being checked #2870

Open rajohnson90 opened 1 day ago

rajohnson90 commented 1 day ago

Our API Auth decorator has two optional parameters: groups and extra-check. With the way the decorator is currently coded, the group check and extra check are not actually checked. As a result, a number of API endpoints (about 4-5) that we think are currently locked-down to specific users are not locked down.

I'm not sure if they can be tested directly yet as some of these extra checks may rely on logging in as users that are configured differently from our current set of fake_auth users.

Expected Behavior

Any endpoint with an 'extra-check' in the is_authorized decorator should be checked and cause an endpoint to return a 401 if the check is failed.

Current Behavior

The only thing a user needs to do to use an endpoint with an 'extra-check' condition is have the correct permissions. It does not take into account the extra check.

Possible Cause

The cause is that the is_authorized decorator is using or statements instead of and statements for the checks. After discussion with the engineers during collaboration time, we've decided that all this extra check logic should be inside the API endpoints (or even better, the service layer) instead.

Steps to Reproduce

  1. Move any of the extra check functions into their associated API endpoints
  2. Run unit tests associated with this endpoint
  3. See unit tests fail

Context

Discovered this while testing how we can lock-down change requests to the appropriate division director. It turns out we already had some of this logic but due to an error the logic is not actually being run.

Possible Implementation

Move all the extra-checks into the API endpoints and fix unit tests where necessary.