SEAB-6310: Log privileged endpoints at webservice startup

svonworl commented 1 month ago

Description This PR modifies the webservice to log a message that contains a list of "privileged" endpoints, which are endpoints with a @RolesAllowed annotation which allows one or more of the admin, curator, or platformPartner roles. The list is logged once, at startup time.

Example log excerpt:

INFO  [2024-05-09 00:28:37,868] io.dockstore.webservice.DockstoreWebserviceApplication: Endpoints that allow a role in [admin, curator, platformPartner]:
GET /organizations/all @jakarta.annotation.security.RolesAllowed({"curator", "admin"})
POST /organizations/{organizationId}/reject @jakarta.annotation.security.RolesAllowed({"curator", "admin"})
POST /organizations/{organizationId}/approve @jakarta.annotation.security.RolesAllowed({"curator", "admin"})
POST /cloudInstances @jakarta.annotation.security.RolesAllowed({"admin"})
DELETE /cloudInstances/{cloudInstanceId} @jakarta.annotation.security.RolesAllowed({"admin"})
GET /lambdaEvents/user/{userid} @jakarta.annotation.security.RolesAllowed({"admin", "curator"})
POST /curation/notifications @jakarta.annotation.security.RolesAllowed({"curator", "admin"})
[...]

The ticket (and ticket writer) preferred that the role information was added to our swagger UI page. This would have required us to first propagate the "admin" info from our code into openapi.yaml, and then to modify our Swagger UI page (which reads a public copy of openapi.yaml) to render the new OpenAPI information. There doesn't appear to be direct support for roles in OpenAPI, so we would have had to map the @RolesAllowed annotations to the closest security-related OpenAPI analog. Certainly, all of this would have been possible, but there didn't appear to be a quick and simple way...

The goal was to get the auditor a list of "admin" endpoints, and although this implementation might not be 100% optimal, they should be able to work with this format.

Originally, I'd anticipated logging at DEBUG level, but to ensure the information makes it to CloudWatch, should we need it, I promoted the message to INFO level. It's logged one time per run, so no big deal.

The semantics which we use to combine endpoint path fragments are a bit different than typical file path semantics, thus the custom joinPaths method (rather than using a canned method).

Review Instructions Check the webservice logs, and confirm that the new message appears, and has the correct number of endpoints listed.

Issue https://ucsc-cgl.atlassian.net/browse/SEAB-6310

Security and Privacy No concerns.

[x] Security and Privacy assessed

Please make sure that you've checked the following before submitting your pull request. Thanks!

[x] Check that you pass the basic style checks and unit tests by running mvn clean install
[x] Ensure that the PR targets the correct branch. Check the milestone or fix version of the ticket.
[x] Follow the existing JPA patterns for queries, using named parameters, to avoid SQL injection
[x] If you are changing dependencies, check the Snyk status check or the dashboard to ensure you are not introducing new high/critical vulnerabilities
[x] Assume that inputs to the API can be malicious, and sanitize and/or check for Denial of Service type values, e.g., massive sizes
[x] Do not serve user-uploaded binary images through the Dockstore API
[x] Ensure that endpoints that only allow privileged access enforce that with the @RolesAllowed annotation
[x] Do not create cookies, although this may change in the future
[x] If this PR is for a user-facing feature, create and link a documentation ticket for this feature (usually in the same milestone as the linked issue). Style points if you create a documentation PR directly and link that instead.

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 12.50000% with 21 lines in your changes are missing coverage. Please review.

Project coverage is 72.42%. Comparing base (e6d0782) to head (012f0f3).

Files	Patch %	Lines
...ore/webservice/DockstoreWebserviceApplication.java	4.76%	20 Missing :warning:
...java/io/dockstore/webservice/SimpleAuthorizer.java	66.66%	0 Missing and 1 partial :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## develop #5881 +/- ## ============================================= - Coverage 73.89% 72.42% -1.47% + Complexity 5264 5160 -104 ============================================= Files 371 371 Lines 19190 19212 +22 Branches 2006 2012 +6 ============================================= - Hits 14180 13915 -265 - Misses 4053 4322 +269 - Partials 957 975 +18 ``` | [Flag](https://app.codecov.io/gh/dockstore/dockstore/pull/5881/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dockstore) | Coverage Δ | | |---|---|---| | [bitbuckettests](https://app.codecov.io/gh/dockstore/dockstore/pull/5881/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dockstore) | `27.12% <8.33%> (-0.03%)` | :arrow_down: | | [integrationtests](https://app.codecov.io/gh/dockstore/dockstore/pull/5881/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dockstore) | `58.43% <8.33%> (-0.06%)` | :arrow_down: | | [languageparsingtests](https://app.codecov.io/gh/dockstore/dockstore/pull/5881/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dockstore) | `11.07% <8.33%> (-0.01%)` | :arrow_down: | | [localstacktests](https://app.codecov.io/gh/dockstore/dockstore/pull/5881/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dockstore) | `21.67% <12.50%> (-0.02%)` | :arrow_down: | | [toolintegrationtests](https://app.codecov.io/gh/dockstore/dockstore/pull/5881/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dockstore) | `30.47% <8.33%> (-0.03%)` | :arrow_down: | | [unit-tests_and_non-confidential-tests](https://app.codecov.io/gh/dockstore/dockstore/pull/5881/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dockstore) | `28.45% <8.33%> (-0.03%)` | :arrow_down: | | [workflowintegrationtests](https://app.codecov.io/gh/dockstore/dockstore/pull/5881/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dockstore) | `34.75% <8.33%> (-3.93%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dockstore#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

denis-yuen commented 1 month ago

There doesn't appear to be direct support for roles in OpenAPI, so we would have had to map the @RolesAllowed annotations to the closest security-related OpenAPI analog. Certainly, all of this would have been possible, but there didn't appear to be a quick and simple way...

I guess the other idea would be to process the openapi.yaml ourselves or find some script. But this works

svonworl commented 1 month ago

I added some constants to SimpleAuthorizer and changed the code to use them, and indented each line of the message to make it less confusable with the log levels in other log messages.

dockstore / dockstore

SEAB-6310: Log privileged endpoints at webservice startup #5881

Codecov Report