Added addCohortPermission.

[chrisknoll]

Removed validation check from cohort saving because you should be allowed to save a cohort definition even with errors.

[chrisknoll]

Note, I am not able to perform security related tests due to that we don't have a security-enabled dev site to test with.

[codecov[bot]]

Codecov Report

Merging #241 (98138e9) into develop (4f6c0ff) will increase coverage by 8.04%. The diff coverage is 36.00%.

:exclamation: Current head 98138e9 differs from pull request most recent head d55c4a6. Consider uploading reports for the commit d55c4a6 to get more accurate results

@@             Coverage Diff             @@
##           develop     #241      +/-   ##
===========================================
+ Coverage    58.70%   66.75%   +8.04%     
===========================================
  Files           18       19       +1     
  Lines         1487     1504      +17     
===========================================
+ Hits           873     1004     +131     
+ Misses         614      500     -114     

Impacted Files	Coverage Δ
R/GetPostDeleteDefinition.R	67.76% <ø> (+46.51%)	:arrow_up:
R/permissions.R	36.00% <36.00%> (ø)

... and 3 files with indirect coverage changes

[ablack3]

Note, I am not able to perform security related tests due to that we don't have a security-enabled dev site to test with.

I think there is (or was) a security enabled webapi instance to test against. These are the environment variables I was using. I think Lee set this up. I'm not sure if these variables are available on the github actions runners.

WEBAPI_TEST_WEBAPI_URL WEBAPI_TEST_SECURE_WEBAPI_URL WEBAPI_TEST_ADMIN_USER_NAME WEBAPI_TEST_ADMIN_USER_PASSWORD

[ablack3]

@chrisknoll What is the preferred way to check if security/auth is enabled in WebAPI?

I'm also thinking we need a way to get user IDs using ROhdsiWebAPI.

[chrisknoll]

Yes, if you do a HTTP GET to WebAPI/info you get this back (this is from my local env:)

{
  "version": "2.12.0",
  "buildInfo": {
    "artifactVersion": "WebAPI 2.12.0-SNAPSHOT",
    "build": "NA",
    "timestamp": "Fri Jul 29 18:11:47 UTC 2022",
    "branch": "issue-2061",
    "commitId": "a4dbb85",
    "atlasRepositoryInfo": {
      "milestoneId": 42,
      "releaseTag": "*"
    },
    "webapiRepositoryInfo": {
      "milestoneId": 43,
      "releaseTag": "*"
    }
  },
  "configuration": {
    "security": {
      "samlActivated": false,
      "enabled": false,
      "samlEnabled": false
    },
    "vocabulary": {
      "solrEnabled": false
    },
    "plugins": {
      "atlasgisEnabled": false
    },
    "person": {
      "viewDatesPermitted": false
    },
    "heracles": {
      "smallCellCount": "5"
    }
  }
}

Under configuration -> security -> enabled, you can determine if security is enabled. Note: the /WebAPI/info endpoint should not require an authentication header.

[ablack3]

Here is a test for the new function. @leeevans I think we need an account that can set permissions on the security enabled webapi instance.

# devtools::install_github("OHDSI/ROhdsiWebApi", ref = "permissions-cohortDef")

library(ROhdsiWebApi)

baseUrl <- Sys.getenv("WEBAPI_TEST_SECURE_WEBAPI_URL") 

authorizeWebApi(baseUrl = baseUrl,
                authMethod = "db",
                webApiUsername = Sys.getenv("WEBAPI_TEST_ADMIN_USER_NAME"),
                webApiPassword = Sys.getenv("WEBAPI_TEST_ADMIN_USER_PASSWORD"))

df <- getUserPermission(baseUrl) 

names(df)
#> [1] "id"          "login"       "name"        "permissions"

# We need an account that can be used to test this function
setCohortPermission(baseUrl, cohortId = 3, userId = 1000, permission = "WRITE")
#> Error in `.request()` at ROhdsiWebApi/R/httrWrappers.R:10:2:
#> ! http error 403: Forbidden request.
#>    You do not have permission to perform this action.

^{Created on 2022-07-29 by the reprex package (v2.0.1)}

[ablack3]

I've made some edits to this PR. Let me know what you all think. I like the get/set pattern for modifying permissions. I don't have a great understanding of how webapi handles permissions though. Currently we have

getUserPermissions which hits the users endpoint http://webapidoc.ohdsi.org/index.html#-741996078 setCohortPermissions which hits the permissions endpoint http://webapidoc.ohdsi.org/index.html#439635542

I'm not quite sure what the pattern should look like but I do think we need a way to get User IDs and get the permissions set by the function being added in this PR.

[leeevans]

@ablack3 The test admin user has the Atlas admin role, it has 'cohort creator' role and I've added it to a new role called 'set permissions' with all the permissions that contain the word 'permission'. When I try setCohortPermission(baseUrl, cohortId = 3, userId = 1000, permission = "WRITE") It still fails with a permissions error.

I can update the permissions for the test admin user, if you can let me know what specific set of permissions it needs.

Additional info: I logged into the secure atlas as the test admin user and I was able to successfully manually update and save cohort id 3. I undid the change I made in atlas and saved it again successfully.

I also manually made a copy of cohort id 3 in Atlas, which is cohort id 559 (owned by the test admin user) and the following command executed without an error: setCohortPermission(baseUrl, cohortId = 559, userId = 1000, permission = "WRITE")

(I temporarily installed a copy of this branch in my local RStudio for testing)

[ablack3]

@gowthamrao has asked for help with this PR and I can put a little time toward it to try and help out. I'd like to get a clear idea of the scope of this PR. Usually we discuss the scope of a change in an issue and I'm not able to find an issue associated with this PR (please post the link if there is one). From what I see in the code the goal here is to expose some WebAPI permission management through ROhdsiWebApi.

In particular it seems that @chrisknoll would like to be able to set cohort read/write permissions for individual users. I think we also need a way to get user-level cohort permissions so we can see the result of the permission change from ROhdsiWebApi.

What would be helpful for me is some clear documentation about how WebAPI handles permissions. As far as I can tell there are roles which can be granted READ or WRITE permissions on various entities (cohort, pathway, etc). A user ID can be used as a role ID.

My general question with respect to this PR is about scope. Is our goal here for ROhdsiWebAPI to have functions to get/set Cohort level permissions for any user ID, a task that would require admin privileges in WebAPI? Would it make more sense to expose more general permission management through ROhdsiWebApi that mirrors what is available through WebAPI or are we only interested in user level cohort permissions? If there will be a need to set permissions for other entities or for roles other than a single user then I think it would be better to have a general function that wraps the permission endpoints. http://webapidoc.ohdsi.org/index.html#471491671

I'm also not very clear on how to find out if a user has read or write permissions for a particular cohort. Here is my example that seems to suggest I can't get my own permission status for a cohort.

library(ROhdsiWebApi)
library(magrittr)
baseUrl <- Sys.getenv("WEBAPI_TEST_SECURE_WEBAPI_URL") 

authorizeWebApi(baseUrl = baseUrl,
                authMethod = "db",
                webApiUsername = Sys.getenv("WEBAPI_TEST_ADMIN_USER_NAME"),
                webApiPassword = Sys.getenv("WEBAPI_TEST_ADMIN_USER_PASSWORD"))

# Get my User Id
me <- ROhdsiWebApi:::.GET(paste0(baseUrl, "/user/me")) %>% 
  httr::content()

(userId <- me$id)
#> [1] 2500

# Get a cohort Id
cohorts <- getCohortDefinitionsMetaData(baseUrl)
(cohortId <- cohorts$id[[1]])
#> [1] 305

# Figure out if I have READ or WRITE permissions on the cohort
url <- paste0(baseUrl, "/permission/access/COHORT_DEFINITION/", cohortId)

CohortPermission <- ROhdsiWebApi:::.GET(url) %>% 
  httr::content(r)
#> Error in `.request()` at ROhdsiWebApi/R/httrWrappers.R:6:2:
#> ! http error 403: Forbidden request.
#>    You do not have permission to perform this action.

^{Created on 2022-08-19 with reprex v2.0.2}

[gowthamrao]

Closing in favor of https://github.com/OHDSI/ROhdsiWebApi/pull/254