From 0def1830afae29ff6ace9da871758ca836f66d3c Mon Sep 17 00:00:00 2001 From: Oscar Hinton Date: Mon, 17 Jan 2022 13:21:51 +0100 Subject: [PATCH] Move identity endpoints to Identity service (#1807) --- bitwarden-server.sln | 8 ++ src/Api/Controllers/AccountsController.cs | 10 +- .../SetKeyConnectorKeyRequestModel.cs | 1 + .../Accounts/SetPasswordRequestModel.cs | 1 + .../Api}/Request/Accounts/KeysRequestModel.cs | 2 +- .../Request/Accounts/PreloginRequestModel.cs | 2 +- .../Request/Accounts/RegisterRequestModel.cs | 3 +- .../Accounts}/PreloginResponseModel.cs | 2 +- .../Controllers/AccountsController.cs | 70 +++++++++++ ...{AccountController.cs => SsoController.cs} | 20 ++- .../Controllers/AccountsControllerTests.cs | 3 +- .../Controllers/AccountsControllerTests.cs | 115 ++++++++++++++++++ test/Identity.Test/Identity.Test.csproj | 29 +++++ 13 files changed, 246 insertions(+), 20 deletions(-) rename src/{Api/Models => Core/Models/Api}/Request/Accounts/KeysRequestModel.cs (93%) rename src/{Api/Models => Core/Models/Api}/Request/Accounts/PreloginRequestModel.cs (82%) rename src/{Api/Models => Core/Models/Api}/Request/Accounts/RegisterRequestModel.cs (97%) rename src/{Api/Models/Response => Core/Models/Api/Response/Accounts}/PreloginResponseModel.cs (88%) create mode 100644 src/Identity/Controllers/AccountsController.cs rename src/Identity/Controllers/{AccountController.cs => SsoController.cs} (94%) create mode 100644 test/Identity.Test/Controllers/AccountsControllerTests.cs create mode 100644 test/Identity.Test/Identity.Test.csproj diff --git a/bitwarden-server.sln b/bitwarden-server.sln index 6dae86dc3..ecd098317 100644 --- a/bitwarden-server.sln +++ b/bitwarden-server.sln @@ -81,8 +81,11 @@ EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "SharedWeb", "src\SharedWeb\SharedWeb.csproj", "{713D44C0-1BC1-4024-96A3-A98A49F33908}" EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Infrastructure.EntityFramework", "src\Infrastructure.EntityFramework\Infrastructure.EntityFramework.csproj", "{ED880735-0250-43C7-9662-FDC7C7416E7F}" +EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Billing.Test", "test\Billing.Test\Billing.Test.csproj", "{B8639B10-2157-44BC-8CE1-D9EB4B50971F}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Identity.Test", "test\Identity.Test\Identity.Test.csproj", "{310A1D8E-2D3F-4FA0-84D4-FFE31FCE193E}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -195,6 +198,10 @@ Global {B8639B10-2157-44BC-8CE1-D9EB4B50971F}.Debug|Any CPU.Build.0 = Debug|Any CPU {B8639B10-2157-44BC-8CE1-D9EB4B50971F}.Release|Any CPU.ActiveCfg = Release|Any CPU {B8639B10-2157-44BC-8CE1-D9EB4B50971F}.Release|Any CPU.Build.0 = Release|Any CPU + {310A1D8E-2D3F-4FA0-84D4-FFE31FCE193E}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {310A1D8E-2D3F-4FA0-84D4-FFE31FCE193E}.Debug|Any CPU.Build.0 = Debug|Any CPU + {310A1D8E-2D3F-4FA0-84D4-FFE31FCE193E}.Release|Any CPU.ActiveCfg = Release|Any CPU + {310A1D8E-2D3F-4FA0-84D4-FFE31FCE193E}.Release|Any CPU.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -226,6 +233,7 @@ Global {713D44C0-1BC1-4024-96A3-A98A49F33908} = {DD5BD056-4AAE-43EF-BBD2-0B569B8DA84D} {ED880735-0250-43C7-9662-FDC7C7416E7F} = {DD5BD056-4AAE-43EF-BBD2-0B569B8DA84D} {B8639B10-2157-44BC-8CE1-D9EB4B50971F} = {DD5BD056-4AAE-43EF-BBD2-0B569B8DA84F} + {310A1D8E-2D3F-4FA0-84D4-FFE31FCE193E} = {DD5BD056-4AAE-43EF-BBD2-0B569B8DA84F} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {E01CBF68-2E20-425F-9EDB-E0A6510CA92F} diff --git a/src/Api/Controllers/AccountsController.cs b/src/Api/Controllers/AccountsController.cs index 539431601..471de18ea 100644 --- a/src/Api/Controllers/AccountsController.cs +++ b/src/Api/Controllers/AccountsController.cs @@ -11,6 +11,8 @@ using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Enums.Provider; using Bit.Core.Exceptions; +using Bit.Core.Models.Api.Request.Accounts; +using Bit.Core.Models.Api.Response.Accounts; using Bit.Core.Models.Business; using Bit.Core.Models.Data; using Bit.Core.Repositories; @@ -64,6 +66,9 @@ namespace Bit.Api.Controllers _sendService = sendService; } + #region DEPRECATED (Moved to Identity Service) + + [Obsolete("2022-01-12 Moved to Identity, left for backwards compatability with older clients")] [HttpPost("prelogin")] [AllowAnonymous] public async Task PostPrelogin([FromBody] PreloginRequestModel model) @@ -74,12 +79,13 @@ namespace Bit.Api.Controllers kdfInformation = new UserKdfInformation { Kdf = KdfType.PBKDF2_SHA256, - KdfIterations = 100000 + KdfIterations = 100000, }; } return new PreloginResponseModel(kdfInformation); } + [Obsolete("2022-01-12 Moved to Identity, left for backwards compatability with older clients")] [HttpPost("register")] [AllowAnonymous] [CaptchaProtected] @@ -101,6 +107,8 @@ namespace Bit.Api.Controllers throw new BadRequestException(ModelState); } + #endregion + [HttpPost("password-hint")] [AllowAnonymous] public async Task PostPasswordHint([FromBody] PasswordHintRequestModel model) diff --git a/src/Api/Models/Request/Accounts/SetKeyConnectorKeyRequestModel.cs b/src/Api/Models/Request/Accounts/SetKeyConnectorKeyRequestModel.cs index 030d1adbc..39c17bc4b 100644 --- a/src/Api/Models/Request/Accounts/SetKeyConnectorKeyRequestModel.cs +++ b/src/Api/Models/Request/Accounts/SetKeyConnectorKeyRequestModel.cs @@ -1,6 +1,7 @@ using System.ComponentModel.DataAnnotations; using Bit.Core.Entities; using Bit.Core.Enums; +using Bit.Core.Models.Api.Request.Accounts; namespace Bit.Api.Models.Request.Accounts { diff --git a/src/Api/Models/Request/Accounts/SetPasswordRequestModel.cs b/src/Api/Models/Request/Accounts/SetPasswordRequestModel.cs index ae5368e9b..287ba8769 100644 --- a/src/Api/Models/Request/Accounts/SetPasswordRequestModel.cs +++ b/src/Api/Models/Request/Accounts/SetPasswordRequestModel.cs @@ -1,6 +1,7 @@ using System.ComponentModel.DataAnnotations; using Bit.Core.Entities; using Bit.Core.Enums; +using Bit.Core.Models.Api.Request.Accounts; namespace Bit.Api.Models.Request.Accounts { diff --git a/src/Api/Models/Request/Accounts/KeysRequestModel.cs b/src/Core/Models/Api/Request/Accounts/KeysRequestModel.cs similarity index 93% rename from src/Api/Models/Request/Accounts/KeysRequestModel.cs rename to src/Core/Models/Api/Request/Accounts/KeysRequestModel.cs index b6351f3ad..77015a96e 100644 --- a/src/Api/Models/Request/Accounts/KeysRequestModel.cs +++ b/src/Core/Models/Api/Request/Accounts/KeysRequestModel.cs @@ -1,7 +1,7 @@ using System.ComponentModel.DataAnnotations; using Bit.Core.Entities; -namespace Bit.Api.Models.Request.Accounts +namespace Bit.Core.Models.Api.Request.Accounts { public class KeysRequestModel { diff --git a/src/Api/Models/Request/Accounts/PreloginRequestModel.cs b/src/Core/Models/Api/Request/Accounts/PreloginRequestModel.cs similarity index 82% rename from src/Api/Models/Request/Accounts/PreloginRequestModel.cs rename to src/Core/Models/Api/Request/Accounts/PreloginRequestModel.cs index 4b0c26bf9..dca9e08bf 100644 --- a/src/Api/Models/Request/Accounts/PreloginRequestModel.cs +++ b/src/Core/Models/Api/Request/Accounts/PreloginRequestModel.cs @@ -1,6 +1,6 @@ using System.ComponentModel.DataAnnotations; -namespace Bit.Api.Models.Request.Accounts +namespace Bit.Core.Models.Api.Request.Accounts { public class PreloginRequestModel { diff --git a/src/Api/Models/Request/Accounts/RegisterRequestModel.cs b/src/Core/Models/Api/Request/Accounts/RegisterRequestModel.cs similarity index 97% rename from src/Api/Models/Request/Accounts/RegisterRequestModel.cs rename to src/Core/Models/Api/Request/Accounts/RegisterRequestModel.cs index c028de500..ad5e9e747 100644 --- a/src/Api/Models/Request/Accounts/RegisterRequestModel.cs +++ b/src/Core/Models/Api/Request/Accounts/RegisterRequestModel.cs @@ -3,11 +3,10 @@ using System.Collections.Generic; using System.ComponentModel.DataAnnotations; using Bit.Core.Entities; using Bit.Core.Enums; -using Bit.Core.Models.Api; using Bit.Core.Utilities; using Newtonsoft.Json; -namespace Bit.Api.Models.Request.Accounts +namespace Bit.Core.Models.Api.Request.Accounts { public class RegisterRequestModel : IValidatableObject, ICaptchaProtectedModel { diff --git a/src/Api/Models/Response/PreloginResponseModel.cs b/src/Core/Models/Api/Response/Accounts/PreloginResponseModel.cs similarity index 88% rename from src/Api/Models/Response/PreloginResponseModel.cs rename to src/Core/Models/Api/Response/Accounts/PreloginResponseModel.cs index c4da92371..755182f76 100644 --- a/src/Api/Models/Response/PreloginResponseModel.cs +++ b/src/Core/Models/Api/Response/Accounts/PreloginResponseModel.cs @@ -1,7 +1,7 @@ using Bit.Core.Enums; using Bit.Core.Models.Data; -namespace Bit.Api.Models.Response +namespace Bit.Core.Models.Api.Response.Accounts { public class PreloginResponseModel { diff --git a/src/Identity/Controllers/AccountsController.cs b/src/Identity/Controllers/AccountsController.cs new file mode 100644 index 000000000..b56f5277b --- /dev/null +++ b/src/Identity/Controllers/AccountsController.cs @@ -0,0 +1,70 @@ +using System.Linq; +using System.Threading.Tasks; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.Models.Api.Request.Accounts; +using Bit.Core.Models.Api.Response.Accounts; +using Bit.Core.Models.Data; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Core.Utilities; +using Microsoft.AspNetCore.Mvc; +using Microsoft.Extensions.Logging; + +namespace Bit.Identity.Controllers +{ + [Route("accounts")] + public class AccountsController : Controller + { + private readonly ILogger _logger; + private readonly IUserRepository _userRepository; + private readonly IUserService _userService; + + public AccountsController( + ILogger logger, + IUserRepository userRepository, + IUserService userService) + { + _logger = logger; + _userRepository = userRepository; + _userService = userService; + } + + // Moved from API, If you modify this endpoint, please update Identity as well. + [HttpPost("register")] + [CaptchaProtected] + public async Task PostRegister([FromBody] RegisterRequestModel model) + { + var result = await _userService.RegisterUserAsync(model.ToUser(), model.MasterPasswordHash, + model.Token, model.OrganizationUserId); + if (result.Succeeded) + { + return; + } + + foreach (var error in result.Errors.Where(e => e.Code != "DuplicateUserName")) + { + ModelState.AddModelError(string.Empty, error.Description); + } + + await Task.Delay(2000); + throw new BadRequestException(ModelState); + } + + // Moved from API, If you modify this endpoint, please update Identity as well. + [HttpPost("prelogin")] + public async Task PostPrelogin([FromBody] PreloginRequestModel model) + { + var kdfInformation = await _userRepository.GetKdfInformationByEmailAsync(model.Email); + if (kdfInformation == null) + { + kdfInformation = new UserKdfInformation + { + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = 100000, + }; + } + return new PreloginResponseModel(kdfInformation); + } + } +} diff --git a/src/Identity/Controllers/AccountController.cs b/src/Identity/Controllers/SsoController.cs similarity index 94% rename from src/Identity/Controllers/AccountController.cs rename to src/Identity/Controllers/SsoController.cs index f0b7bcec3..bb8251337 100644 --- a/src/Identity/Controllers/AccountController.cs +++ b/src/Identity/Controllers/SsoController.cs @@ -11,7 +11,6 @@ using Bit.Identity.Models; using IdentityModel; using IdentityServer4; using IdentityServer4.Services; -using IdentityServer4.Stores; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Localization; @@ -20,31 +19,28 @@ using Microsoft.Extensions.Logging; namespace Bit.Identity.Controllers { - public class AccountController : Controller + // TODO: 2022-01-12, Remove account alias + [Route("account/[action]")] + [Route("sso/[action]")] + public class SsoController : Controller { - private readonly IClientStore _clientStore; private readonly IIdentityServerInteractionService _interaction; - private readonly ILogger _logger; + private readonly ILogger _logger; private readonly ISsoConfigRepository _ssoConfigRepository; private readonly IUserRepository _userRepository; - private readonly IOrganizationRepository _organizationRepository; private readonly IHttpClientFactory _clientFactory; - public AccountController( - IClientStore clientStore, + public SsoController( IIdentityServerInteractionService interaction, - ILogger logger, + ILogger logger, ISsoConfigRepository ssoConfigRepository, IUserRepository userRepository, - IOrganizationRepository organizationRepository, IHttpClientFactory clientFactory) { - _clientStore = clientStore; _interaction = interaction; _logger = logger; _ssoConfigRepository = ssoConfigRepository; _userRepository = userRepository; - _organizationRepository = organizationRepository; _clientFactory = clientFactory; } @@ -272,7 +268,7 @@ namespace Bit.Identity.Controllers } } - public bool IsNativeClient(IdentityServer4.Models.AuthorizationRequest context) + private bool IsNativeClient(IdentityServer4.Models.AuthorizationRequest context) { return !context.RedirectUri.StartsWith("https", StringComparison.Ordinal) && !context.RedirectUri.StartsWith("http", StringComparison.Ordinal); diff --git a/test/Api.Test/Controllers/AccountsControllerTests.cs b/test/Api.Test/Controllers/AccountsControllerTests.cs index 39cdb4455..0124eaf89 100644 --- a/test/Api.Test/Controllers/AccountsControllerTests.cs +++ b/test/Api.Test/Controllers/AccountsControllerTests.cs @@ -3,10 +3,10 @@ using System.Security.Claims; using System.Threading.Tasks; using Bit.Api.Controllers; using Bit.Api.Models.Request.Accounts; -using Bit.Core; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; +using Bit.Core.Models.Api.Request.Accounts; using Bit.Core.Models.Data; using Bit.Core.Repositories; using Bit.Core.Services; @@ -27,7 +27,6 @@ namespace Bit.Api.Test.Controllers private readonly IOrganizationService _organizationService; private readonly IOrganizationUserRepository _organizationUserRepository; private readonly IPaymentService _paymentService; - private readonly ISsoUserRepository _ssoUserRepository; private readonly IUserRepository _userRepository; private readonly IUserService _userService; private readonly ISendRepository _sendRepository; diff --git a/test/Identity.Test/Controllers/AccountsControllerTests.cs b/test/Identity.Test/Controllers/AccountsControllerTests.cs new file mode 100644 index 000000000..73faa8b46 --- /dev/null +++ b/test/Identity.Test/Controllers/AccountsControllerTests.cs @@ -0,0 +1,115 @@ +using System; +using System.Threading.Tasks; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.Models.Api.Request.Accounts; +using Bit.Core.Models.Data; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Identity.Controllers; +using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Logging; +using NSubstitute; +using Xunit; + +namespace Bit.Identity.Test.Controllers +{ + public class AccountsControllerTests : IDisposable + { + + private readonly AccountsController _sut; + private readonly ILogger _logger; + private readonly IUserRepository _userRepository; + private readonly IUserService _userService; + + public AccountsControllerTests() + { + _logger = Substitute.For>(); + _userRepository = Substitute.For(); + _userService = Substitute.For(); + _sut = new AccountsController( + _logger, + _userRepository, + _userService + ); + } + + public void Dispose() + { + _sut?.Dispose(); + } + + [Fact] + public async Task PostPrelogin_WhenUserExists_ShouldReturnUserKdfInfo() + { + var userKdfInfo = new UserKdfInformation + { + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = 5000 + }; + _userRepository.GetKdfInformationByEmailAsync(Arg.Any()).Returns(Task.FromResult(userKdfInfo)); + + var response = await _sut.PostPrelogin(new PreloginRequestModel { Email = "user@example.com" }); + + Assert.Equal(userKdfInfo.Kdf, response.Kdf); + Assert.Equal(userKdfInfo.KdfIterations, response.KdfIterations); + } + + [Fact] + public async Task PostPrelogin_WhenUserDoesNotExist_ShouldDefaultToSha256And100000Iterations() + { + _userRepository.GetKdfInformationByEmailAsync(Arg.Any()).Returns(Task.FromResult((UserKdfInformation)null)); + + var response = await _sut.PostPrelogin(new PreloginRequestModel { Email = "user@example.com" }); + + Assert.Equal(KdfType.PBKDF2_SHA256, response.Kdf); + Assert.Equal(100000, response.KdfIterations); + } + + [Fact] + public async Task PostRegister_ShouldRegisterUser() + { + var passwordHash = "abcdef"; + var token = "123456"; + var userGuid = new Guid(); + _userService.RegisterUserAsync(Arg.Any(), passwordHash, token, userGuid) + .Returns(Task.FromResult(IdentityResult.Success)); + var request = new RegisterRequestModel + { + Name = "Example User", + Email = "user@example.com", + MasterPasswordHash = passwordHash, + MasterPasswordHint = "example", + Token = token, + OrganizationUserId = userGuid + }; + + await _sut.PostRegister(request); + + await _userService.Received(1).RegisterUserAsync(Arg.Any(), passwordHash, token, userGuid); + } + + [Fact] + public async Task PostRegister_WhenUserServiceFails_ShouldThrowBadRequestException() + { + var passwordHash = "abcdef"; + var token = "123456"; + var userGuid = new Guid(); + _userService.RegisterUserAsync(Arg.Any(), passwordHash, token, userGuid) + .Returns(Task.FromResult(IdentityResult.Failed())); + var request = new RegisterRequestModel + { + Name = "Example User", + Email = "user@example.com", + MasterPasswordHash = passwordHash, + MasterPasswordHint = "example", + Token = token, + OrganizationUserId = userGuid + }; + + await Assert.ThrowsAsync(() => _sut.PostRegister(request)); + } + } +} + diff --git a/test/Identity.Test/Identity.Test.csproj b/test/Identity.Test/Identity.Test.csproj new file mode 100644 index 000000000..627fdd19e --- /dev/null +++ b/test/Identity.Test/Identity.Test.csproj @@ -0,0 +1,29 @@ + + + + enable + false + + + + + runtime; build; native; contentfiles; analyzers; buildtransitive + all + + + + + + all + runtime; build; native; contentfiles; analyzers; buildtransitive + + + + + + + + + + +