Police-Data-Accessibility-Project / data-sources-app

An API and UI for using and maintaining the Data Sources database
MIT License
3 stars 5 forks source link

Create `get_permitted_columns()` and `check_edit_values_permitted()` functions #391

Closed maxachis closed 2 months ago

maxachis commented 2 months ago

A number of our tables have columns and views where view and edit access varies depending on the user and their permissions.

Broadly speaking, we have STANDARD and ADMIN users, who have either EDIT or VIEW permissions for each column.

Currently, in code we determine who can do what through a number of arrays of varying permutations, which list columns that can or cannot be used. Our current means of organizing them is confusing:

DATA_SOURCES_OUTPUT_COLUMNS = DATA_SOURCES_APPROVED_COLUMNS + ARCHIVE_INFO_APPROVED_COLUMNS + ["agency_name"]
RESTRICTED_DATA_SOURCE_COLUMNS = [
    "rejection_note",
    "data_source_request",
    "approval_status",
    "airtable_uid",
    "airtable_source_last_modified",
]

Instead of creating more permutations of arrays, I propose that for each table and view, we have a tabular structure where:

While it takes more effort in the short-term than the array pattern, in the long term it will pose less cumbersome and scale more effectively.

This is partly a refactoring job, and partly about making the development of issues such as #390, which involve varying levels of visibility and permissions, more manageable. For the moment, I'll put aside refactoring existing code.

The other component, check_edit_values_permitted, will take a dictionary of key-value "edit pairs" (where the key represents the column and the value represents the new value) and checks to see that all values are allowable for editing. This is designed to catch and reject PUT requests that include edit values we do not allow that type of user to edit.

Implementation Steps

Reference

https://docs.pdap.io/activities/data-dictionaries/hidden-properties

maxachis commented 2 months ago

@josh-chamberlain One interesting wrinkle in all of this is how to design our endpoints when the view permissions between standard and non-standard (i.e. admin or otherwise elevated) users differ for a column.

For example, consider the following column as described in #390:

COLUMN STANDARD OWNER ADMIN
archive_reason VIEW EDIT

The owner can view it, the admin can view and edit it, and the standard user can do neither.

Here's the issue as it relates to our current design of endpoints:

So what should these other endpoints, where we're returning data from a table and where the viewable columns differ based on user, look like? There's a few options as I see it:

  1. Have a Basic endpoint that can look up permissions.
  2. Make the endpoint Bearer and look up permissions that way
  3. Have separate endpoints for different view structures
  4. Have the endpoint be both Bearer and Basic, where the authorization provided impacts whether it has standard permissions (Basic) or whether it should be evaluated for elevated permissions (Bearer).

I think 4 makes the most sense, but I wanted to present all options as I see them.

josh-chamberlain commented 2 months ago

@maxachis ooh, this is a good point + I probably should have seen it coming! Yes, with # 4 you articulated it perfectly. I think the authorization provided goes through the permission checking machine, and impacts what happens (which fields you get back, what actions you can take). You could provide both bearer and basic auth, but if your account has no permissions, you don't get the good stuff.