Skip to content

Commit

Permalink
Auth/PM-3659 - Disable Passkey registration if Require SSO Policy Ena…
Browse files Browse the repository at this point in the history 
…bled (#3399)

* PM-3659 - WebAuthnController.cs - Passkey Creation - Add RequireSSO login policy validation to prevent users from creating passkeys if require SSO applies to them.

* PM-3659 - per PR feedback, apply new require SSO validation to options call

* PM-3659 - Remove unneeded comment

* PM-3659 - Per PR feedback, add unit tests for new require SSO scenarios on both Post and Options endpoints on the WebAuthnController

* Remove duplicated line

* Remove extra whitespace
  • Loading branch information
@JaredSnider-Bitwarden
JaredSnider-Bitwarden authored Nov 1, 2023
1 parent 1fb5e49 commit f5f6405
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 5 deletions.
19 changes: 18 additions & 1 deletion src/Api/Auth/Controllers/WebAuthnController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using Bit.Core;
using Bit.Core.Auth.Models.Business.Tokenables;
using Bit.Core.Auth.Repositories;
using Bit.Core.Enums;
using Bit.Core.Exceptions;
using Bit.Core.Services;
using Bit.Core.Tokens;
Expand All @@ -22,15 +23,18 @@ public class WebAuthnController : Controller
private readonly IUserService _userService;
private readonly IWebAuthnCredentialRepository _credentialRepository;
private readonly IDataProtectorTokenFactory<WebAuthnCredentialCreateOptionsTokenable> _createOptionsDataProtector;
private readonly IPolicyService _policyService;

public WebAuthnController(
IUserService userService,
IWebAuthnCredentialRepository credentialRepository,
IDataProtectorTokenFactory<WebAuthnCredentialCreateOptionsTokenable> createOptionsDataProtector)
IDataProtectorTokenFactory<WebAuthnCredentialCreateOptionsTokenable> createOptionsDataProtector,
IPolicyService policyService)
{
_userService = userService;
_credentialRepository = credentialRepository;
_createOptionsDataProtector = createOptionsDataProtector;
_policyService = policyService;
}

[HttpGet("")]
Expand All @@ -46,6 +50,7 @@ public async Task<ListResponseModel<WebAuthnCredentialResponseModel>> Get()
public async Task<WebAuthnCredentialCreateOptionsResponseModel> PostOptions([FromBody] SecretVerificationRequestModel model)
{
var user = await VerifyUserAsync(model);
await ValidateRequireSsoPolicyDisabledOrNotApplicable(user.Id);
var options = await _userService.StartWebAuthnLoginRegistrationAsync(user);

var tokenable = new WebAuthnCredentialCreateOptionsTokenable(user, options);
Expand All @@ -62,7 +67,9 @@ public async Task<WebAuthnCredentialCreateOptionsResponseModel> PostOptions([Fro
public async Task Post([FromBody] WebAuthnCredentialRequestModel model)
{
var user = await GetUserAsync();
await ValidateRequireSsoPolicyDisabledOrNotApplicable(user.Id);
var tokenable = _createOptionsDataProtector.Unprotect(model.Token);

if (!tokenable.TokenIsValid(user))
{
throw new BadRequestException("The token associated with your request is expired. A valid token is required to continue.");
Expand All @@ -75,6 +82,16 @@ public async Task Post([FromBody] WebAuthnCredentialRequestModel model)
}
}

private async Task ValidateRequireSsoPolicyDisabledOrNotApplicable(Guid userId)
{
var requireSsoLogin = await _policyService.AnyPoliciesApplicableToUserAsync(userId, PolicyType.RequireSso);

if (requireSsoLogin)
{
throw new BadRequestException("Passkeys cannot be created for your account. SSO login is required.");
}
}

[HttpPost("{id}/delete")]
public async Task Delete(Guid id, [FromBody] SecretVerificationRequestModel model)
{
Expand Down
50 changes: 46 additions & 4 deletions test/Api.Test/Auth/Controllers/WebAuthnControllerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using Bit.Api.Auth.Models.Request.Webauthn;
using Bit.Core.Auth.Models.Business.Tokenables;
using Bit.Core.Entities;
using Bit.Core.Enums;
using Bit.Core.Exceptions;
using Bit.Core.Services;
using Bit.Core.Tokens;
Expand All @@ -22,7 +23,7 @@ public class WebAuthnControllerTests
[Theory, BitAutoData]
public async Task Get_UserNotFound_ThrowsUnauthorizedAccessException(SutProvider<WebAuthnController> sutProvider)
{
// Arrange
// Arrange
sutProvider.GetDependency<IUserService>().GetUserByPrincipalAsync(default).ReturnsNullForAnyArgs();

// Act
Expand All @@ -35,7 +36,7 @@ public async Task Get_UserNotFound_ThrowsUnauthorizedAccessException(SutProvider
[Theory, BitAutoData]
public async Task PostOptions_UserNotFound_ThrowsUnauthorizedAccessException(SecretVerificationRequestModel requestModel, SutProvider<WebAuthnController> sutProvider)
{
// Arrange
// Arrange
sutProvider.GetDependency<IUserService>().GetUserByPrincipalAsync(default).ReturnsNullForAnyArgs();

// Act
Expand All @@ -59,10 +60,25 @@ public async Task PostOptions_UserVerificationFailed_ThrowsBadRequestException(S
await Assert.ThrowsAsync<BadRequestException>(result);
}

[Theory, BitAutoData]
public async Task PostOptions_RequireSsoPolicyApplicable_ThrowsBadRequestException(
SecretVerificationRequestModel requestModel, User user, SutProvider<WebAuthnController> sutProvider)
{
// Arrange
sutProvider.GetDependency<IUserService>().GetUserByPrincipalAsync(default).ReturnsForAnyArgs(user);
sutProvider.GetDependency<IUserService>().VerifySecretAsync(user, default).ReturnsForAnyArgs(true);
sutProvider.GetDependency<IPolicyService>().AnyPoliciesApplicableToUserAsync(user.Id, PolicyType.RequireSso).ReturnsForAnyArgs(true);

// Act & Assert
var exception = await Assert.ThrowsAsync<BadRequestException>(
() => sutProvider.Sut.PostOptions(requestModel));
Assert.Contains("Passkeys cannot be created for your account. SSO login is required", exception.Message);
}

[Theory, BitAutoData]
public async Task Post_UserNotFound_ThrowsUnauthorizedAccessException(WebAuthnCredentialRequestModel requestModel, SutProvider<WebAuthnController> sutProvider)
{
// Arrange
// Arrange
sutProvider.GetDependency<IUserService>().GetUserByPrincipalAsync(default).ReturnsNullForAnyArgs();

// Act
Expand Down Expand Up @@ -113,10 +129,36 @@ public async Task Post_ValidInput_Returns(WebAuthnCredentialRequestModel request
// Nothing to assert since return is void
}

[Theory, BitAutoData]
public async Task Post_RequireSsoPolicyApplicable_ThrowsBadRequestException(
WebAuthnCredentialRequestModel requestModel,
CredentialCreateOptions createOptions,
User user,
SutProvider<WebAuthnController> sutProvider)
{
// Arrange
var token = new WebAuthnCredentialCreateOptionsTokenable(user, createOptions);
sutProvider.GetDependency<IUserService>()
.GetUserByPrincipalAsync(default)
.ReturnsForAnyArgs(user);
sutProvider.GetDependency<IUserService>()
.CompleteWebAuthLoginRegistrationAsync(user, requestModel.Name, createOptions, Arg.Any<AuthenticatorAttestationRawResponse>())
.Returns(true);
sutProvider.GetDependency<IDataProtectorTokenFactory<WebAuthnCredentialCreateOptionsTokenable>>()
.Unprotect(requestModel.Token)
.Returns(token);
sutProvider.GetDependency<IPolicyService>().AnyPoliciesApplicableToUserAsync(user.Id, PolicyType.RequireSso).ReturnsForAnyArgs(true);

// Act & Assert
var exception = await Assert.ThrowsAsync<BadRequestException>(
() => sutProvider.Sut.Post(requestModel));
Assert.Contains("Passkeys cannot be created for your account. SSO login is required", exception.Message);
}

[Theory, BitAutoData]
public async Task Delete_UserNotFound_ThrowsUnauthorizedAccessException(Guid credentialId, SecretVerificationRequestModel requestModel, SutProvider<WebAuthnController> sutProvider)
{
// Arrange
// Arrange
sutProvider.GetDependency<IUserService>().GetUserByPrincipalAsync(default).ReturnsNullForAnyArgs();

// Act
Expand Down

0 comments on commit f5f6405

Please sign in to comment.