diff --git a/dev/generate_openapi_files.ps1 b/dev/generate_openapi_files.ps1 index 02470a0b1d..9eca7dc734 100644 --- a/dev/generate_openapi_files.ps1 +++ b/dev/generate_openapi_files.ps1 @@ -11,9 +11,18 @@ dotnet tool restore Set-Location "./src/Identity" dotnet build dotnet swagger tofile --output "../../identity.json" --host "https://identity.bitwarden.com" "./bin/Debug/net8.0/Identity.dll" "v1" +if ($LASTEXITCODE -ne 0) { + exit $LASTEXITCODE +} # Api internal & public Set-Location "../../src/Api" dotnet build dotnet swagger tofile --output "../../api.json" --host "https://api.bitwarden.com" "./bin/Debug/net8.0/Api.dll" "internal" +if ($LASTEXITCODE -ne 0) { + exit $LASTEXITCODE +} dotnet swagger tofile --output "../../api.public.json" --host "https://api.bitwarden.com" "./bin/Debug/net8.0/Api.dll" "public" +if ($LASTEXITCODE -ne 0) { + exit $LASTEXITCODE +} diff --git a/src/Api/Auth/Controllers/AccountsController.cs b/src/Api/Auth/Controllers/AccountsController.cs index f197f1270b..0bed7c29c4 100644 --- a/src/Api/Auth/Controllers/AccountsController.cs +++ b/src/Api/Auth/Controllers/AccountsController.cs @@ -344,7 +344,6 @@ public class AccountsController : Controller } [HttpPut("profile")] - [HttpPost("profile")] public async Task PutProfile([FromBody] UpdateProfileRequestModel model) { var user = await _userService.GetUserByPrincipalAsync(User); @@ -363,8 +362,14 @@ public class AccountsController : Controller return response; } + [HttpPost("profile")] + [Obsolete("This endpoint is deprecated. Use PUT /profile instead.")] + public async Task PostProfile([FromBody] UpdateProfileRequestModel model) + { + return await PutProfile(model); + } + [HttpPut("avatar")] - [HttpPost("avatar")] public async Task PutAvatar([FromBody] UpdateAvatarRequestModel model) { var user = await _userService.GetUserByPrincipalAsync(User); @@ -382,6 +387,13 @@ public class AccountsController : Controller return response; } + [HttpPost("avatar")] + [Obsolete("This endpoint is deprecated. Use PUT /avatar instead.")] + public async Task PostAvatar([FromBody] UpdateAvatarRequestModel model) + { + return await PutAvatar(model); + } + [HttpGet("revision-date")] public async Task GetAccountRevisionDate() { @@ -430,7 +442,6 @@ public class AccountsController : Controller } [HttpDelete] - [HttpPost("delete")] public async Task Delete([FromBody] SecretVerificationRequestModel model) { var user = await _userService.GetUserByPrincipalAsync(User); @@ -467,6 +478,13 @@ public class AccountsController : Controller throw new BadRequestException(ModelState); } + [HttpPost("delete")] + [Obsolete("This endpoint is deprecated. Use DELETE / instead.")] + public async Task PostDelete([FromBody] SecretVerificationRequestModel model) + { + await Delete(model); + } + [AllowAnonymous] [HttpPost("delete-recover")] public async Task PostDeleteRecover([FromBody] DeleteRecoverRequestModel model) @@ -638,7 +656,6 @@ public class AccountsController : Controller await _twoFactorEmailService.SendNewDeviceVerificationEmailAsync(user); } - [HttpPost("verify-devices")] [HttpPut("verify-devices")] public async Task SetUserVerifyDevicesAsync([FromBody] SetVerifyDevicesRequestModel request) { @@ -654,6 +671,13 @@ public class AccountsController : Controller await _userService.SaveUserAsync(user); } + [HttpPost("verify-devices")] + [Obsolete("This endpoint is deprecated. Use PUT /verify-devices instead.")] + public async Task PostSetUserVerifyDevicesAsync([FromBody] SetVerifyDevicesRequestModel request) + { + await SetUserVerifyDevicesAsync(request); + } + private async Task> GetOrganizationIdsClaimingUserAsync(Guid userId) { var organizationsClaimingUser = await _userService.GetOrganizationsClaimingUserAsync(userId); diff --git a/src/Api/Auth/Controllers/AuthRequestsController.cs b/src/Api/Auth/Controllers/AuthRequestsController.cs index 3f91bd6eea..c62b817905 100644 --- a/src/Api/Auth/Controllers/AuthRequestsController.cs +++ b/src/Api/Auth/Controllers/AuthRequestsController.cs @@ -31,7 +31,7 @@ public class AuthRequestsController( private readonly IAuthRequestService _authRequestService = authRequestService; [HttpGet("")] - public async Task> Get() + public async Task> GetAll() { var userId = _userService.GetProperUserId(User).Value; var authRequests = await _authRequestRepository.GetManyByUserIdAsync(userId); diff --git a/src/Api/Auth/Controllers/EmergencyAccessController.cs b/src/Api/Auth/Controllers/EmergencyAccessController.cs index 53b57fe685..b849dc3e07 100644 --- a/src/Api/Auth/Controllers/EmergencyAccessController.cs +++ b/src/Api/Auth/Controllers/EmergencyAccessController.cs @@ -79,7 +79,6 @@ public class EmergencyAccessController : Controller } [HttpPut("{id}")] - [HttpPost("{id}")] public async Task Put(Guid id, [FromBody] EmergencyAccessUpdateRequestModel model) { var emergencyAccess = await _emergencyAccessRepository.GetByIdAsync(id); @@ -92,14 +91,27 @@ public class EmergencyAccessController : Controller await _emergencyAccessService.SaveAsync(model.ToEmergencyAccess(emergencyAccess), user); } + [HttpPost("{id}")] + [Obsolete("This endpoint is deprecated. Use PUT /{id} instead.")] + public async Task Post(Guid id, [FromBody] EmergencyAccessUpdateRequestModel model) + { + await Put(id, model); + } + [HttpDelete("{id}")] - [HttpPost("{id}/delete")] public async Task Delete(Guid id) { var userId = _userService.GetProperUserId(User); await _emergencyAccessService.DeleteAsync(id, userId.Value); } + [HttpPost("{id}/delete")] + [Obsolete("This endpoint is deprecated. Use DELETE /{id} instead.")] + public async Task PostDelete(Guid id) + { + await Delete(id); + } + [HttpPost("invite")] public async Task Invite([FromBody] EmergencyAccessInviteRequestModel model) { @@ -136,7 +148,7 @@ public class EmergencyAccessController : Controller } [HttpPost("{id}/approve")] - public async Task Accept(Guid id) + public async Task Approve(Guid id) { var user = await _userService.GetUserByPrincipalAsync(User); await _emergencyAccessService.ApproveAsync(id, user); diff --git a/src/Api/Auth/Controllers/TwoFactorController.cs b/src/Api/Auth/Controllers/TwoFactorController.cs index 4155489daa..886ed2cd20 100644 --- a/src/Api/Auth/Controllers/TwoFactorController.cs +++ b/src/Api/Auth/Controllers/TwoFactorController.cs @@ -110,7 +110,6 @@ public class TwoFactorController : Controller } [HttpPut("authenticator")] - [HttpPost("authenticator")] public async Task PutAuthenticator( [FromBody] UpdateTwoFactorAuthenticatorRequestModel model) { @@ -133,6 +132,14 @@ public class TwoFactorController : Controller return response; } + [HttpPost("authenticator")] + [Obsolete("This endpoint is deprecated. Use PUT /authenticator instead.")] + public async Task PostAuthenticator( + [FromBody] UpdateTwoFactorAuthenticatorRequestModel model) + { + return await PutAuthenticator(model); + } + [HttpDelete("authenticator")] public async Task DisableAuthenticator( [FromBody] TwoFactorAuthenticatorDisableRequestModel model) @@ -157,7 +164,6 @@ public class TwoFactorController : Controller } [HttpPut("yubikey")] - [HttpPost("yubikey")] public async Task PutYubiKey([FromBody] UpdateTwoFactorYubicoOtpRequestModel model) { var user = await CheckAsync(model, true); @@ -174,6 +180,13 @@ public class TwoFactorController : Controller return response; } + [HttpPost("yubikey")] + [Obsolete("This endpoint is deprecated. Use PUT /yubikey instead.")] + public async Task PostYubiKey([FromBody] UpdateTwoFactorYubicoOtpRequestModel model) + { + return await PutYubiKey(model); + } + [HttpPost("get-duo")] public async Task GetDuo([FromBody] SecretVerificationRequestModel model) { @@ -183,7 +196,6 @@ public class TwoFactorController : Controller } [HttpPut("duo")] - [HttpPost("duo")] public async Task PutDuo([FromBody] UpdateTwoFactorDuoRequestModel model) { var user = await CheckAsync(model, true); @@ -199,6 +211,13 @@ public class TwoFactorController : Controller return response; } + [HttpPost("duo")] + [Obsolete("This endpoint is deprecated. Use PUT /duo instead.")] + public async Task PostDuo([FromBody] UpdateTwoFactorDuoRequestModel model) + { + return await PutDuo(model); + } + [HttpPost("~/organizations/{id}/two-factor/get-duo")] public async Task GetOrganizationDuo(string id, [FromBody] SecretVerificationRequestModel model) @@ -217,7 +236,6 @@ public class TwoFactorController : Controller } [HttpPut("~/organizations/{id}/two-factor/duo")] - [HttpPost("~/organizations/{id}/two-factor/duo")] public async Task PutOrganizationDuo(string id, [FromBody] UpdateTwoFactorDuoRequestModel model) { @@ -243,6 +261,14 @@ public class TwoFactorController : Controller return response; } + [HttpPost("~/organizations/{id}/two-factor/duo")] + [Obsolete("This endpoint is deprecated. Use PUT /organizations/{id}/two-factor/duo instead.")] + public async Task PostOrganizationDuo(string id, + [FromBody] UpdateTwoFactorDuoRequestModel model) + { + return await PutOrganizationDuo(id, model); + } + [HttpPost("get-webauthn")] public async Task GetWebAuthn([FromBody] SecretVerificationRequestModel model) { @@ -261,7 +287,6 @@ public class TwoFactorController : Controller } [HttpPut("webauthn")] - [HttpPost("webauthn")] public async Task PutWebAuthn([FromBody] TwoFactorWebAuthnRequestModel model) { var user = await CheckAsync(model, false); @@ -277,6 +302,13 @@ public class TwoFactorController : Controller return response; } + [HttpPost("webauthn")] + [Obsolete("This endpoint is deprecated. Use PUT /webauthn instead.")] + public async Task PostWebAuthn([FromBody] TwoFactorWebAuthnRequestModel model) + { + return await PutWebAuthn(model); + } + [HttpDelete("webauthn")] public async Task DeleteWebAuthn( [FromBody] TwoFactorWebAuthnDeleteRequestModel model) @@ -349,7 +381,6 @@ public class TwoFactorController : Controller } [HttpPut("email")] - [HttpPost("email")] public async Task PutEmail([FromBody] UpdateTwoFactorEmailRequestModel model) { var user = await CheckAsync(model, false); @@ -367,8 +398,14 @@ public class TwoFactorController : Controller return response; } + [HttpPost("email")] + [Obsolete("This endpoint is deprecated. Use PUT /email instead.")] + public async Task PostEmail([FromBody] UpdateTwoFactorEmailRequestModel model) + { + return await PutEmail(model); + } + [HttpPut("disable")] - [HttpPost("disable")] public async Task PutDisable([FromBody] TwoFactorProviderRequestModel model) { var user = await CheckAsync(model, false); @@ -377,8 +414,14 @@ public class TwoFactorController : Controller return response; } + [HttpPost("disable")] + [Obsolete("This endpoint is deprecated. Use PUT /disable instead.")] + public async Task PostDisable([FromBody] TwoFactorProviderRequestModel model) + { + return await PutDisable(model); + } + [HttpPut("~/organizations/{id}/two-factor/disable")] - [HttpPost("~/organizations/{id}/two-factor/disable")] public async Task PutOrganizationDisable(string id, [FromBody] TwoFactorProviderRequestModel model) { @@ -401,6 +444,14 @@ public class TwoFactorController : Controller return response; } + [HttpPost("~/organizations/{id}/two-factor/disable")] + [Obsolete("This endpoint is deprecated. Use PUT /organizations/{id}/two-factor/disable instead.")] + public async Task PostOrganizationDisable(string id, + [FromBody] TwoFactorProviderRequestModel model) + { + return await PutOrganizationDisable(id, model); + } + [HttpPost("get-recover")] public async Task GetRecover([FromBody] SecretVerificationRequestModel model) { diff --git a/src/Api/Controllers/CollectionsController.cs b/src/Api/Controllers/CollectionsController.cs index 6d4e9c9fea..f037ab7034 100644 --- a/src/Api/Controllers/CollectionsController.cs +++ b/src/Api/Controllers/CollectionsController.cs @@ -102,7 +102,7 @@ public class CollectionsController : Controller } [HttpGet("")] - public async Task> Get(Guid orgId) + public async Task> GetAll(Guid orgId) { IEnumerable orgCollections; @@ -173,7 +173,6 @@ public class CollectionsController : Controller } [HttpPut("{id}")] - [HttpPost("{id}")] public async Task Put(Guid orgId, Guid id, [FromBody] UpdateCollectionRequestModel model) { var collection = await _collectionRepository.GetByIdAsync(id); @@ -198,6 +197,13 @@ public class CollectionsController : Controller return new CollectionAccessDetailsResponseModel(collectionWithPermissions); } + [HttpPost("{id}")] + [Obsolete("This endpoint is deprecated. Use PUT /{id} instead.")] + public async Task Post(Guid orgId, Guid id, [FromBody] UpdateCollectionRequestModel model) + { + return await Put(orgId, id, model); + } + [HttpPost("bulk-access")] public async Task PostBulkCollectionAccess(Guid orgId, [FromBody] BulkCollectionAccessRequestModel model) { @@ -222,7 +228,6 @@ public class CollectionsController : Controller } [HttpDelete("{id}")] - [HttpPost("{id}/delete")] public async Task Delete(Guid orgId, Guid id) { var collection = await _collectionRepository.GetByIdAsync(id); @@ -235,8 +240,14 @@ public class CollectionsController : Controller await _deleteCollectionCommand.DeleteAsync(collection); } + [HttpPost("{id}/delete")] + [Obsolete("This endpoint is deprecated. Use DELETE /{id} instead.")] + public async Task PostDelete(Guid orgId, Guid id) + { + await Delete(orgId, id); + } + [HttpDelete("")] - [HttpPost("delete")] public async Task DeleteMany(Guid orgId, [FromBody] CollectionBulkDeleteRequestModel model) { var collections = await _collectionRepository.GetManyByManyIdsAsync(model.Ids); @@ -248,4 +259,11 @@ public class CollectionsController : Controller await _deleteCollectionCommand.DeleteManyAsync(collections); } + + [HttpPost("delete")] + [Obsolete("This endpoint is deprecated. Use DELETE / instead.")] + public async Task PostDeleteMany(Guid orgId, [FromBody] CollectionBulkDeleteRequestModel model) + { + await DeleteMany(orgId, model); + } } diff --git a/src/Api/Controllers/DevicesController.cs b/src/Api/Controllers/DevicesController.cs index 07e8552268..1f2cda9cc4 100644 --- a/src/Api/Controllers/DevicesController.cs +++ b/src/Api/Controllers/DevicesController.cs @@ -75,7 +75,7 @@ public class DevicesController : Controller } [HttpGet("")] - public async Task> Get() + public async Task> GetAll() { var devicesWithPendingAuthData = await _deviceRepository.GetManyByUserIdWithDeviceAuth(_userService.GetProperUserId(User).Value); @@ -99,7 +99,6 @@ public class DevicesController : Controller } [HttpPut("{id}")] - [HttpPost("{id}")] public async Task Put(string id, [FromBody] DeviceRequestModel model) { var device = await _deviceRepository.GetByIdAsync(new Guid(id), _userService.GetProperUserId(User).Value); @@ -114,8 +113,14 @@ public class DevicesController : Controller return response; } + [HttpPost("{id}")] + [Obsolete("This endpoint is deprecated. Use PUT /{id} instead.")] + public async Task Post(string id, [FromBody] DeviceRequestModel model) + { + return await Put(id, model); + } + [HttpPut("{identifier}/keys")] - [HttpPost("{identifier}/keys")] public async Task PutKeys(string identifier, [FromBody] DeviceKeysRequestModel model) { var device = await _deviceRepository.GetByIdentifierAsync(identifier, _userService.GetProperUserId(User).Value); @@ -130,6 +135,13 @@ public class DevicesController : Controller return response; } + [HttpPost("{identifier}/keys")] + [Obsolete("This endpoint is deprecated. Use PUT /{identifier}/keys instead.")] + public async Task PostKeys(string identifier, [FromBody] DeviceKeysRequestModel model) + { + return await PutKeys(identifier, model); + } + [HttpPost("{identifier}/retrieve-keys")] [Obsolete("This endpoint is deprecated. The keys are on the regular device GET endpoints now.")] public async Task GetDeviceKeys(string identifier) @@ -187,7 +199,6 @@ public class DevicesController : Controller } [HttpPut("identifier/{identifier}/token")] - [HttpPost("identifier/{identifier}/token")] public async Task PutToken(string identifier, [FromBody] DeviceTokenRequestModel model) { var device = await _deviceRepository.GetByIdentifierAsync(identifier, _userService.GetProperUserId(User).Value); @@ -199,8 +210,14 @@ public class DevicesController : Controller await _deviceService.SaveAsync(model.ToDevice(device)); } + [HttpPost("identifier/{identifier}/token")] + [Obsolete("This endpoint is deprecated. Use PUT /identifier/{identifier}/token instead.")] + public async Task PostToken(string identifier, [FromBody] DeviceTokenRequestModel model) + { + await PutToken(identifier, model); + } + [HttpPut("identifier/{identifier}/web-push-auth")] - [HttpPost("identifier/{identifier}/web-push-auth")] public async Task PutWebPushAuth(string identifier, [FromBody] WebPushAuthRequestModel model) { var device = await _deviceRepository.GetByIdentifierAsync(identifier, _userService.GetProperUserId(User).Value); @@ -216,9 +233,15 @@ public class DevicesController : Controller ); } + [HttpPost("identifier/{identifier}/web-push-auth")] + [Obsolete("This endpoint is deprecated. Use PUT /identifier/{identifier}/web-push-auth instead.")] + public async Task PostWebPushAuth(string identifier, [FromBody] WebPushAuthRequestModel model) + { + await PutWebPushAuth(identifier, model); + } + [AllowAnonymous] [HttpPut("identifier/{identifier}/clear-token")] - [HttpPost("identifier/{identifier}/clear-token")] public async Task PutClearToken(string identifier) { var device = await _deviceRepository.GetByIdentifierAsync(identifier); @@ -230,8 +253,15 @@ public class DevicesController : Controller await _deviceService.ClearTokenAsync(device); } + [AllowAnonymous] + [HttpPost("identifier/{identifier}/clear-token")] + [Obsolete("This endpoint is deprecated. Use PUT /identifier/{identifier}/clear-token instead.")] + public async Task PostClearToken(string identifier) + { + await PutClearToken(identifier); + } + [HttpDelete("{id}")] - [HttpPost("{id}/deactivate")] public async Task Deactivate(string id) { var device = await _deviceRepository.GetByIdAsync(new Guid(id), _userService.GetProperUserId(User).Value); @@ -243,17 +273,24 @@ public class DevicesController : Controller await _deviceService.DeactivateAsync(device); } + [HttpPost("{id}/deactivate")] + [Obsolete("This endpoint is deprecated. Use DELETE /{id} instead.")] + public async Task PostDeactivate(string id) + { + await Deactivate(id); + } + [AllowAnonymous] [HttpGet("knowndevice")] public async Task GetByIdentifierQuery( [Required][FromHeader(Name = "X-Request-Email")] string Email, [Required][FromHeader(Name = "X-Device-Identifier")] string DeviceIdentifier) - => await GetByIdentifier(CoreHelpers.Base64UrlDecodeString(Email), DeviceIdentifier); + => await GetByEmailAndIdentifier(CoreHelpers.Base64UrlDecodeString(Email), DeviceIdentifier); [Obsolete("Path is deprecated due to encoding issues, use /knowndevice instead.")] [AllowAnonymous] [HttpGet("knowndevice/{email}/{identifier}")] - public async Task GetByIdentifier(string email, string identifier) + public async Task GetByEmailAndIdentifier(string email, string identifier) { if (string.IsNullOrWhiteSpace(email) || string.IsNullOrWhiteSpace(identifier)) { diff --git a/src/Api/Controllers/InfoController.cs b/src/Api/Controllers/InfoController.cs index edfd18c79e..590a3006c0 100644 --- a/src/Api/Controllers/InfoController.cs +++ b/src/Api/Controllers/InfoController.cs @@ -6,12 +6,18 @@ namespace Bit.Api.Controllers; public class InfoController : Controller { [HttpGet("~/alive")] - [HttpGet("~/now")] public DateTime GetAlive() { return DateTime.UtcNow; } + [HttpGet("~/now")] + [Obsolete("This endpoint is deprecated. Use GET /alive instead.")] + public DateTime GetNow() + { + return GetAlive(); + } + [HttpGet("~/version")] public JsonResult GetVersion() { diff --git a/src/Api/Controllers/SettingsController.cs b/src/Api/Controllers/SettingsController.cs index 8489b137e8..e872eeeeac 100644 --- a/src/Api/Controllers/SettingsController.cs +++ b/src/Api/Controllers/SettingsController.cs @@ -32,7 +32,6 @@ public class SettingsController : Controller } [HttpPut("domains")] - [HttpPost("domains")] public async Task PutDomains([FromBody] UpdateDomainsRequestModel model) { var user = await _userService.GetUserByPrincipalAsync(User); @@ -46,4 +45,11 @@ public class SettingsController : Controller var response = new DomainsResponseModel(user); return response; } + + [HttpPost("domains")] + [Obsolete("This endpoint is deprecated. Use PUT /domains instead.")] + public async Task PostDomains([FromBody] UpdateDomainsRequestModel model) + { + return await PutDomains(model); + } } diff --git a/src/Api/Utilities/ServiceCollectionExtensions.cs b/src/Api/Utilities/ServiceCollectionExtensions.cs index aa2710c42a..0d8c3dec38 100644 --- a/src/Api/Utilities/ServiceCollectionExtensions.cs +++ b/src/Api/Utilities/ServiceCollectionExtensions.cs @@ -82,15 +82,7 @@ public static class ServiceCollectionExtensions config.DescribeAllParametersInCamelCase(); // config.UseReferencedDefinitionsForEnums(); - config.SchemaFilter(); - config.SchemaFilter(); - - // These two filters require debug symbols/git, so only add them in development mode - if (environment.IsDevelopment()) - { - config.DocumentFilter(); - config.OperationFilter(); - } + config.InitializeSwaggerFilters(environment); var apiFilePath = Path.Combine(AppContext.BaseDirectory, "Api.xml"); config.IncludeXmlComments(apiFilePath, true); diff --git a/src/Identity/Controllers/InfoController.cs b/src/Identity/Controllers/InfoController.cs index 05cf3f2363..79dfd99c44 100644 --- a/src/Identity/Controllers/InfoController.cs +++ b/src/Identity/Controllers/InfoController.cs @@ -6,12 +6,18 @@ namespace Bit.Identity.Controllers; public class InfoController : Controller { [HttpGet("~/alive")] - [HttpGet("~/now")] public DateTime GetAlive() { return DateTime.UtcNow; } + [HttpGet("~/now")] + [Obsolete("This endpoint is deprecated. Use GET /alive instead.")] + public DateTime GetNow() + { + return GetAlive(); + } + [HttpGet("~/version")] public JsonResult GetVersion() { diff --git a/src/Identity/Startup.cs b/src/Identity/Startup.cs index ae628197e8..8da31d87d6 100644 --- a/src/Identity/Startup.cs +++ b/src/Identity/Startup.cs @@ -66,15 +66,7 @@ public class Startup services.AddSwaggerGen(config => { - config.SchemaFilter(); - config.SchemaFilter(); - - // These two filters require debug symbols/git, so only add them in development mode - if (Environment.IsDevelopment()) - { - config.DocumentFilter(); - config.OperationFilter(); - } + config.InitializeSwaggerFilters(Environment); config.SwaggerDoc("v1", new OpenApiInfo { Title = "Bitwarden Identity", Version = "v1" }); }); diff --git a/src/SharedWeb/Swagger/ActionNameOperationFilter.cs b/src/SharedWeb/Swagger/ActionNameOperationFilter.cs new file mode 100644 index 0000000000..b76e8864ba --- /dev/null +++ b/src/SharedWeb/Swagger/ActionNameOperationFilter.cs @@ -0,0 +1,25 @@ +using System.Text.Json; +using Microsoft.OpenApi.Any; +using Microsoft.OpenApi.Models; +using Swashbuckle.AspNetCore.SwaggerGen; + +namespace Bit.SharedWeb.Swagger; + +/// +/// Adds the action name (function name) as an extension to each operation in the Swagger document. +/// This can be useful for the code generation process, to generate more meaningful names for operations. +/// Note that we add both the original action name and a snake_case version, as the codegen templates +/// cannot do case conversions. +/// +public class ActionNameOperationFilter : IOperationFilter +{ + public void Apply(OpenApiOperation operation, OperationFilterContext context) + { + if (!context.ApiDescription.ActionDescriptor.RouteValues.TryGetValue("action", out var action)) return; + if (string.IsNullOrEmpty(action)) return; + + operation.Extensions.Add("x-action-name", new OpenApiString(action)); + // We can't do case changes in the codegen templates, so we also add the snake_case version of the action name + operation.Extensions.Add("x-action-name-snake-case", new OpenApiString(JsonNamingPolicy.SnakeCaseLower.ConvertName(action))); + } +} diff --git a/src/SharedWeb/Swagger/CheckDuplicateOperationIdsDocumentFilter.cs b/src/SharedWeb/Swagger/CheckDuplicateOperationIdsDocumentFilter.cs new file mode 100644 index 0000000000..3079a9171a --- /dev/null +++ b/src/SharedWeb/Swagger/CheckDuplicateOperationIdsDocumentFilter.cs @@ -0,0 +1,80 @@ +using Microsoft.OpenApi.Models; +using Swashbuckle.AspNetCore.SwaggerGen; + +namespace Bit.SharedWeb.Swagger; + +/// +/// Checks for duplicate operation IDs in the Swagger document, and throws an error if any are found. +/// Operation IDs must be unique across the entire Swagger document according to the OpenAPI specification, +/// but we use controller action names to generate them, which can lead to duplicates if a Controller function +/// has multiple HTTP methods or if a Controller has overloaded functions. +/// +public class CheckDuplicateOperationIdsDocumentFilter(bool printDuplicates = true) : IDocumentFilter +{ + public bool PrintDuplicates { get; } = printDuplicates; + + public void Apply(OpenApiDocument swaggerDoc, DocumentFilterContext context) + { + var operationIdMap = new Dictionary>(); + + foreach (var (path, pathItem) in swaggerDoc.Paths) + { + foreach (var operation in pathItem.Operations) + { + if (!operationIdMap.TryGetValue(operation.Value.OperationId, out var list)) + { + list = []; + operationIdMap[operation.Value.OperationId] = list; + } + + list.Add((path, pathItem, operation.Key, operation.Value)); + + } + } + + // Find duplicates + var duplicates = operationIdMap.Where((kvp) => kvp.Value.Count > 1).ToList(); + if (duplicates.Count > 0) + { + if (PrintDuplicates) + { + Console.WriteLine($"\n######## Duplicate operationIds found in the schema ({duplicates.Count} found) ########\n"); + + Console.WriteLine("## Common causes of duplicate operation IDs:"); + Console.WriteLine("- Multiple HTTP methods (GET, POST, etc.) on the same controller function"); + Console.WriteLine(" Solution: Split the methods into separate functions, and if appropiate, mark the deprecated ones with [Obsolete]"); + Console.WriteLine(); + Console.WriteLine("- Overloaded controller functions with the same name"); + Console.WriteLine(" Solution: Rename the overloaded functions to have unique names, or combine them into a single function with optional parameters"); + Console.WriteLine(); + + Console.WriteLine("## The duplicate operation IDs are:"); + + foreach (var (operationId, duplicate) in duplicates) + { + Console.WriteLine($"- operationId: {operationId}"); + foreach (var (path, pathItem, method, operation) in duplicate) + { + Console.Write($" {method.ToString().ToUpper()} {path}"); + + + if (operation.Extensions.TryGetValue("x-source-file", out var sourceFile) && operation.Extensions.TryGetValue("x-source-line", out var sourceLine)) + { + var sourceFileString = ((Microsoft.OpenApi.Any.OpenApiString)sourceFile).Value; + var sourceLineString = ((Microsoft.OpenApi.Any.OpenApiInteger)sourceLine).Value; + + Console.WriteLine($" {sourceFileString}:{sourceLineString}"); + } + else + { + Console.WriteLine(); + } + } + Console.WriteLine("\n"); + } + } + + throw new InvalidOperationException($"Duplicate operation IDs found in Swagger schema"); + } + } +} diff --git a/src/SharedWeb/Swagger/SwaggerGenOptionsExt.cs b/src/SharedWeb/Swagger/SwaggerGenOptionsExt.cs new file mode 100644 index 0000000000..60803705d6 --- /dev/null +++ b/src/SharedWeb/Swagger/SwaggerGenOptionsExt.cs @@ -0,0 +1,33 @@ +using Microsoft.AspNetCore.Hosting; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Swashbuckle.AspNetCore.SwaggerGen; + +namespace Bit.SharedWeb.Swagger; + +public static class SwaggerGenOptionsExt +{ + + public static void InitializeSwaggerFilters( + this SwaggerGenOptions config, IWebHostEnvironment environment) + { + config.SchemaFilter(); + config.SchemaFilter(); + + config.OperationFilter(); + + // Set the operation ID to the name of the controller followed by the name of the function. + // Note that the "Controller" suffix for the controllers, and the "Async" suffix for the actions + // are removed already, so we don't need to do that ourselves. + // TODO(Dani): This is disabled until we remove all the duplicate operation IDs. + // config.CustomOperationIds(e => $"{e.ActionDescriptor.RouteValues["controller"]}_{e.ActionDescriptor.RouteValues["action"]}"); + // config.DocumentFilter(); + + // These two filters require debug symbols/git, so only add them in development mode + if (environment.IsDevelopment()) + { + config.DocumentFilter(); + config.OperationFilter(); + } + } +} diff --git a/test/Api.Test/Auth/Controllers/AuthRequestsControllerTests.cs b/test/Api.Test/Auth/Controllers/AuthRequestsControllerTests.cs index 828911f6bd..1b8e7aba8e 100644 --- a/test/Api.Test/Auth/Controllers/AuthRequestsControllerTests.cs +++ b/test/Api.Test/Auth/Controllers/AuthRequestsControllerTests.cs @@ -43,7 +43,7 @@ public class AuthRequestsControllerTests .Returns([authRequest]); // Act - var result = await sutProvider.Sut.Get(); + var result = await sutProvider.Sut.GetAll(); // Assert Assert.NotNull(result); diff --git a/test/Api.Test/Auth/Controllers/DevicesControllerTests.cs b/test/Api.Test/Auth/Controllers/DevicesControllerTests.cs index 540d23f98b..bed483f83a 100644 --- a/test/Api.Test/Auth/Controllers/DevicesControllerTests.cs +++ b/test/Api.Test/Auth/Controllers/DevicesControllerTests.cs @@ -73,7 +73,7 @@ public class DevicesControllerTest _deviceRepositoryMock.GetManyByUserIdWithDeviceAuth(userId).Returns(devicesWithPendingAuthData); // Act - var result = await _sut.Get(); + var result = await _sut.GetAll(); // Assert Assert.NotNull(result); @@ -94,6 +94,6 @@ public class DevicesControllerTest _userServiceMock.GetProperUserId(Arg.Any()).Returns((Guid?)null); // Act & Assert - await Assert.ThrowsAsync(() => _sut.Get()); + await Assert.ThrowsAsync(() => _sut.GetAll()); } } diff --git a/test/Api.Test/Controllers/CollectionsControllerTests.cs b/test/Api.Test/Controllers/CollectionsControllerTests.cs index a3d34efb63..33b7e20327 100644 --- a/test/Api.Test/Controllers/CollectionsControllerTests.cs +++ b/test/Api.Test/Controllers/CollectionsControllerTests.cs @@ -177,7 +177,7 @@ public class CollectionsControllerTests .GetManySharedCollectionsByOrganizationIdAsync(organization.Id) .Returns(collections); - var response = await sutProvider.Sut.Get(organization.Id); + var response = await sutProvider.Sut.GetAll(organization.Id); await sutProvider.GetDependency().Received(1).GetManySharedCollectionsByOrganizationIdAsync(organization.Id); @@ -219,7 +219,7 @@ public class CollectionsControllerTests .GetManyByUserIdAsync(userId) .Returns(collections); - var result = await sutProvider.Sut.Get(organization.Id); + var result = await sutProvider.Sut.GetAll(organization.Id); await sutProvider.GetDependency().DidNotReceive().GetManyByOrganizationIdAsync(organization.Id); await sutProvider.GetDependency().Received(1).GetManyByUserIdAsync(userId); diff --git a/test/SharedWeb.Test/ActionNameOperationFilterTest.cs b/test/SharedWeb.Test/ActionNameOperationFilterTest.cs new file mode 100644 index 0000000000..c798adea8c --- /dev/null +++ b/test/SharedWeb.Test/ActionNameOperationFilterTest.cs @@ -0,0 +1,67 @@ +using Bit.SharedWeb.Swagger; +using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.ApiExplorer; +using Microsoft.OpenApi.Any; +using Microsoft.OpenApi.Models; +using Swashbuckle.AspNetCore.SwaggerGen; + +namespace SharedWeb.Test; + +public class ActionNameOperationFilterTest +{ + [Fact] + public void WithValidActionNameAddsActionNameExtensions() + { + // Arrange + var operation = new OpenApiOperation(); + var actionDescriptor = new ActionDescriptor(); + actionDescriptor.RouteValues["action"] = "GetUsers"; + + var apiDescription = new ApiDescription + { + ActionDescriptor = actionDescriptor + }; + + var context = new OperationFilterContext(apiDescription, null, null, null); + var filter = new ActionNameOperationFilter(); + + // Act + filter.Apply(operation, context); + + // Assert + Assert.True(operation.Extensions.ContainsKey("x-action-name")); + Assert.True(operation.Extensions.ContainsKey("x-action-name-snake-case")); + + var actionNameExt = operation.Extensions["x-action-name"] as OpenApiString; + var actionNameSnakeCaseExt = operation.Extensions["x-action-name-snake-case"] as OpenApiString; + + Assert.NotNull(actionNameExt); + Assert.NotNull(actionNameSnakeCaseExt); + Assert.Equal("GetUsers", actionNameExt.Value); + Assert.Equal("get_users", actionNameSnakeCaseExt.Value); + } + + [Fact] + public void WithMissingActionRouteValueDoesNotAddExtensions() + { + // Arrange + var operation = new OpenApiOperation(); + var actionDescriptor = new ActionDescriptor(); + // Not setting the "action" route value at all + + var apiDescription = new ApiDescription + { + ActionDescriptor = actionDescriptor + }; + + var context = new OperationFilterContext(apiDescription, null, null, null); + var filter = new ActionNameOperationFilter(); + + // Act + filter.Apply(operation, context); + + // Assert + Assert.False(operation.Extensions.ContainsKey("x-action-name")); + Assert.False(operation.Extensions.ContainsKey("x-action-name-snake-case")); + } +} diff --git a/test/SharedWeb.Test/CheckDuplicateOperationIdsDocumentFilterTest.cs b/test/SharedWeb.Test/CheckDuplicateOperationIdsDocumentFilterTest.cs new file mode 100644 index 0000000000..7b7c5771d3 --- /dev/null +++ b/test/SharedWeb.Test/CheckDuplicateOperationIdsDocumentFilterTest.cs @@ -0,0 +1,84 @@ +using Bit.SharedWeb.Swagger; +using Microsoft.AspNetCore.Mvc; +using Microsoft.OpenApi.Models; +using Swashbuckle.AspNetCore.SwaggerGen; + +namespace SharedWeb.Test; + +public class UniqueOperationIdsController : ControllerBase +{ + [HttpGet("unique-get")] + public void UniqueGetAction() { } + + [HttpPost("unique-post")] + public void UniquePostAction() { } +} + +public class OverloadedOperationIdsController : ControllerBase +{ + [HttpPut("another-duplicate")] + public void AnotherDuplicateAction() { } + + [HttpPatch("another-duplicate/{id}")] + public void AnotherDuplicateAction(int id) { } +} + +public class MultipleHttpMethodsController : ControllerBase +{ + [HttpGet("multi-method")] + [HttpPost("multi-method")] + [HttpPut("multi-method")] + public void MultiMethodAction() { } +} + +public class CheckDuplicateOperationIdsDocumentFilterTest +{ + [Fact] + public void UniqueOperationIdsDoNotThrowException() + { + // Arrange + var (swaggerDoc, context) = SwaggerDocUtil.CreateDocFromControllers(typeof(UniqueOperationIdsController)); + var filter = new CheckDuplicateOperationIdsDocumentFilter(); + filter.Apply(swaggerDoc, context); + // Act & Assert + var exception = Record.Exception(() => filter.Apply(swaggerDoc, context)); + Assert.Null(exception); + } + + [Fact] + public void DuplicateOperationIdsThrowInvalidOperationException() + { + // Arrange + var (swaggerDoc, context) = SwaggerDocUtil.CreateDocFromControllers(typeof(OverloadedOperationIdsController)); + var filter = new CheckDuplicateOperationIdsDocumentFilter(false); + + // Act & Assert + var exception = Assert.Throws(() => filter.Apply(swaggerDoc, context)); + Assert.Contains("Duplicate operation IDs found in Swagger schema", exception.Message); + } + + [Fact] + public void MultipleHttpMethodsThrowInvalidOperationException() + { + // Arrange + var (swaggerDoc, context) = SwaggerDocUtil.CreateDocFromControllers(typeof(MultipleHttpMethodsController)); + var filter = new CheckDuplicateOperationIdsDocumentFilter(false); + + // Act & Assert + var exception = Assert.Throws(() => filter.Apply(swaggerDoc, context)); + Assert.Contains("Duplicate operation IDs found in Swagger schema", exception.Message); + } + + [Fact] + public void EmptySwaggerDocDoesNotThrowException() + { + // Arrange + var swaggerDoc = new OpenApiDocument { Paths = [] }; + var context = new DocumentFilterContext([], null, null); + var filter = new CheckDuplicateOperationIdsDocumentFilter(false); + + // Act & Assert + var exception = Record.Exception(() => filter.Apply(swaggerDoc, context)); + Assert.Null(exception); + } +} diff --git a/test/SharedWeb.Test/SharedWeb.Test.csproj b/test/SharedWeb.Test/SharedWeb.Test.csproj index 8ae7a56a99..c631ac9227 100644 --- a/test/SharedWeb.Test/SharedWeb.Test.csproj +++ b/test/SharedWeb.Test/SharedWeb.Test.csproj @@ -9,6 +9,7 @@ all + diff --git a/test/SharedWeb.Test/SwaggerDocUtil.cs b/test/SharedWeb.Test/SwaggerDocUtil.cs new file mode 100644 index 0000000000..45a3033dec --- /dev/null +++ b/test/SharedWeb.Test/SwaggerDocUtil.cs @@ -0,0 +1,85 @@ +using System.Reflection; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.ApiExplorer; +using Microsoft.AspNetCore.Mvc.ApplicationParts; +using Microsoft.AspNetCore.Mvc.Controllers; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.OpenApi.Models; +using NSubstitute; +using Swashbuckle.AspNetCore.Swagger; +using Swashbuckle.AspNetCore.SwaggerGen; + +namespace SharedWeb.Test; + +public class SwaggerDocUtil +{ + /// + /// Creates an OpenApiDocument and DocumentFilterContext from the specified controller type by setting up + /// a minimal service collection and using the SwaggerProvider to generate the document. + /// + public static (OpenApiDocument, DocumentFilterContext) CreateDocFromControllers(params Type[] controllerTypes) + { + if (controllerTypes.Length == 0) + { + throw new ArgumentException("At least one controller type must be provided", nameof(controllerTypes)); + } + + var services = new ServiceCollection(); + services.AddLogging(); + services.AddSingleton(Substitute.For()); + services.AddControllers() + .ConfigureApplicationPartManager(manager => + { + // Clear existing parts and feature providers + manager.ApplicationParts.Clear(); + manager.FeatureProviders.Clear(); + + // Add a custom feature provider that only includes the specific controller types + manager.FeatureProviders.Add(new MultipleControllerFeatureProvider(controllerTypes)); + + // Add assembly parts for all unique assemblies containing the controllers + foreach (var assembly in controllerTypes.Select(t => t.Assembly).Distinct()) + { + manager.ApplicationParts.Add(new AssemblyPart(assembly)); + } + }); + services.AddSwaggerGen(config => + { + config.SwaggerDoc("v1", new OpenApiInfo { Title = "Test API", Version = "v1" }); + config.CustomOperationIds(e => $"{e.ActionDescriptor.RouteValues["controller"]}_{e.ActionDescriptor.RouteValues["action"]}"); + }); + var serviceProvider = services.BuildServiceProvider(); + + // Get API descriptions + var allApiDescriptions = serviceProvider.GetRequiredService() + .ApiDescriptionGroups.Items + .SelectMany(group => group.Items) + .ToList(); + + if (allApiDescriptions.Count == 0) + { + throw new InvalidOperationException("No API descriptions found for controller, ensure your controllers are defined correctly (public, not nested, inherit from ControllerBase, etc.)"); + } + + // Generate the swagger document and context + var document = serviceProvider.GetRequiredService().GetSwagger("v1"); + var schemaGenerator = serviceProvider.GetRequiredService(); + var context = new DocumentFilterContext(allApiDescriptions, schemaGenerator, new SchemaRepository()); + + return (document, context); + } +} + +public class MultipleControllerFeatureProvider(params Type[] controllerTypes) : ControllerFeatureProvider +{ + private readonly HashSet _allowedControllerTypes = [.. controllerTypes]; + + protected override bool IsController(TypeInfo typeInfo) + { + return _allowedControllerTypes.Contains(typeInfo.AsType()) + && typeInfo.IsClass + && !typeInfo.IsAbstract + && typeof(ControllerBase).IsAssignableFrom(typeInfo); + } +}