Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CORE-307] Fix Spend Report Visibility for Billing Project Users (Workspace Owners) #3193

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

kevinmarete
Copy link
Contributor

Ticket: https://broadworkbench.atlassian.net/browse/CORE-307

This PR ensures that Billing Project Users can view spend reports for workspaces they own on the billing project workspaces list page. Previously, these costs appeared as N/A, and users would receive a 403 Forbidden error when attempting to access the billing project spend report. This was due to the requireProjectAction check for readSpendReport, which was restricted to project owners. I have updated this to createWorkspace, allowing workspace owners to access spend reports across all billing projects.

Before
Before

After
After


PR checklist

  • Include the JIRA issue number in the PR description and title
  • Make sure Swagger is updated if API changes
    • ...and Orchestration's Swagger too!
  • If you changed anything in model/, then you should publish a new official rawls-model and perform the corresponding dependency updates as specified in the README:
    • in the automation subdirectory
    • in workbench-libs
    • in firecloud-orchestration
  • Get two thumbsworth of PR review
  • Verify all tests go green, including CI tests
  • Squash commits and merge to develop (branches are automatically deleted after merging)
  • Inform other teams of any substantial changes via Slack and/or email

@kevinmarete kevinmarete self-assigned this Feb 21, 2025
@kevinmarete kevinmarete marked this pull request as ready for review February 21, 2025 02:26
@kevinmarete kevinmarete requested a review from a team as a code owner February 21, 2025 02:26
@kevinmarete kevinmarete requested review from dvoet and davidangb and removed request for a team February 21, 2025 02:26
@@ -477,7 +477,9 @@ class SpendReportingService(
aggregations: Set[SpendReportingAggregationKeyWithSub]
): Future[SpendReportingResults] = {
validateReportParameters(start, end)
requireProjectAction(project, SamBillingProjectActions.readSpendReport) {
requireProjectAction(project,
SamBillingProjectActions.createWorkspace
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

readSpendReport sounds like the right action to be checking. This probably should be a sam config change to grant readSpendReport to the right workspace role.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reading the jira more closely, it sounds like we have some inconsistent permission checking. Here, we are checking a project action but in the consolidated spend report it is perhaps checking a workspace action? We should make the permission checks consistent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants