search

OHDSI / WebAPI

OHDSI WebAPI contains all OHDSI services that can be called from OHDSI applications
Apache License 2.0
126 stars 156 forks source link

Permission Checks now use Wildcard semantics. #2355

Closed chrisknoll closed 2 months ago

chrisknoll commented 4 months ago

Removed role cache from Permission Service. Changed read/write permission checks to use permissions instead of roles. Permission checks are using Subject.isPermitted() which honors wildcard semantics. Altered JwtAuthRealm to filter user permissions to either * or first element of permission to check for speed. Changed permission index from JsonNode to Map<>. Serializes same way, but map semantics are simpler to navigate. Altered AuthrizationInfo to contain index of Permissions and store Wildcard perms.

General cleanup of unused imports and removed unused dependencies (ie: Autowired fields were removed if no longer needed).

Fixes #2353.

chrisknoll commented 4 months ago

Leaving this PR as draft as there may be additional code to be removed from the codebase as a result of this change: things like sql templates and entity graphs that are used to find roles for the user may no longer be necessary (I'm not sure where we would ever use roles to determine a permission, except for 'Admin').

As we code review, feel free to identify blocks of code that may be redundant or no longer necessary. We are always making efforts to simplify the codebase.

rkboyce commented 3 months ago

@chrisknoll - I tested this change in my environment where have the read-restricted feature enabled and everything worked as expected. Great job!

chrisknoll commented 2 months ago

The last commit adds tests.

For people on the call, DB Unit has several modes of inserting described here: https://www.dbunit.org/components.html

The code I demonstrated did a CLEAN_ALL at first which blows away all the data in a table, which would include 'system roles' so it was rightfully pointed out about that issue.

I attempted a quick demo to show how the test could be changed, and I switched it to DatabaseOperation.INSERT thinking it would insert the data, but from the docs, DatabaseOperation.INSERT will fail if the table exists (ie: INSERT requires a table creation). The correct operation I wanted to use is REFRESH, which will leave existing rows alone and add new records based on the test data set.

@anthonysena this is ready for review.