Skip to content

Conversation

dani-garcia
Copy link
Member

@dani-garcia dani-garcia commented Aug 22, 2025

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-25182

📔 Objective

This PR is a continuation of the work started in #6229, with some of the team specific changes split out for easier review/merge. You can read that PR for a more complete explanation of the purpose, but in short:

  • The SDK generates bindings from the Swagger specification, which creates function names in the format HTTP path + HTTP method. This leads to very unwieldy names.
  • Our goal is to instead generate function names that match the functions in the controller, which means we can't have duplicate overloaded function names, and we can't have more than one route per function.

To fullfill those two goals, this PR does two things:

  • If there are two functions with the same name in a controller, I've renamed one of them to be more descriptive.
  • If a function has two route attributes, I've moved one of the route attributes to a new function and marked it as Obsolete. In general I tried to mark the less precise HTTP method as obsolete. I will leave it up to each team to evaluate if the obsolete routes can be deleted or if they should be maintained.

The final result in the SDK will look like this:

// The current function for GET {}/organizations/{id}/access-policies/people/potential-grantees
pub async fn organizations_id_access_policies_people_potential_grantees_get(...) {}

// Once all these PRs are merged, the name will be
pub async fn get_people_potential_grantees() {}

📸 Screenshots

⏰ Reminders before review

  • Contributor guidelines followed
  • All formatters and local linters executed and passed
  • Written new unit and / or integration tests where applicable
  • Protected functional changes with optionality (feature flags)
  • Used internationalization (i18n) for all UI strings
  • CI builds passed
  • Communicated to DevOps any deployment requirements
  • Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

  • 👍 (:+1:) or similar for great changes
  • 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
  • ❓ (:question:) for questions
  • 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
  • 🎨 (:art:) for suggestions / improvements
  • ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
  • 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
  • ⛏ (:pick:) for minor or nitpick changes

Copy link

codecov bot commented Aug 22, 2025

Codecov Report

❌ Patch coverage is 0% with 78 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.12%. Comparing base (e456b4c) to head (a65a5d5).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...Console/Controllers/OrganizationUsersController.cs 0.00% 30 Missing ⚠️
...c/Api/AdminConsole/Controllers/GroupsController.cs 0.00% 12 Missing ⚠️
...dminConsole/Controllers/ProviderUsersController.cs 0.00% 9 Missing ⚠️
...dminConsole/Controllers/OrganizationsController.cs 0.00% 6 Missing ⚠️
...pi/AdminConsole/Controllers/ProvidersController.cs 0.00% 6 Missing ⚠️
...e/Controllers/OrganizationConnectionsController.cs 0.00% 3 Missing ⚠️
...onsole/Controllers/OrganizationDomainController.cs 0.00% 3 Missing ⚠️
.../OrganizationIntegrationConfigurationController.cs 0.00% 3 Missing ⚠️
...e/Controllers/OrganizationIntegrationController.cs 0.00% 3 Missing ⚠️
...ole/Controllers/ProviderOrganizationsController.cs 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6236      +/-   ##
==========================================
- Coverage   49.17%   49.12%   -0.05%     
==========================================
  Files        1782     1782              
  Lines       79298    79376      +78     
  Branches     7045     7045              
==========================================
  Hits        38997    38997              
- Misses      38782    38860      +78     
  Partials     1519     1519              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

github-actions bot commented Aug 22, 2025

Logo
Checkmarx One – Scan Summary & Details0174413e-343e-40ba-8c0b-4c5b650ec760

New Issues (3)

Checkmarx found the following issues in this Pull Request

Severity Issue Source File / Package Checkmarx Insight
MEDIUM CSRF /src/Api/AdminConsole/Controllers/GroupsController.cs: 289
detailsMethod at line 289 of /src/Api/AdminConsole/Controllers/GroupsController.cs gets a parameter from a user request from orgUserId. This parameter ...
ID: dVqeoLRiIT%2BaNv577pp645pD5AA%3D
Attack Vector
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1328
detailsMethod at line 1328 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ...
ID: VNU0cckjAEKP%2BnjQ1IQgudmWPN0%3D
Attack Vector
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1208
detailsMethod at line 1208 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ...
ID: khyrn9nhNbqoM9vWfy%2FKql5SICY%3D
Attack Vector
Fixed Issues (3)

Great job! The following issues were fixed in this Pull Request

Severity Issue Source File / Package
MEDIUM CSRF /src/Api/Controllers/CollectionsController.cs: 176
MEDIUM CSRF /src/Api/AdminConsole/Controllers/GroupsController.cs: 272
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 278

@dani-garcia dani-garcia changed the title Improve Swagger OperationIDs for AC [PM-25182] Improve Swagger OperationIDs for AC Aug 25, 2025
Base automatically changed from ps/improve-swagger-operationids-part1 to main September 2, 2025 16:30
@dani-garcia dani-garcia force-pushed the ps/improve-swagger-operationids-ac branch from e9a0222 to d987926 Compare September 2, 2025 17:22
Copy link

sonarqubecloud bot commented Sep 4, 2025

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.

1 participant