Skip to content
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
65 commits
Select commit Hold shift + click to select a range
14f3e88
Adding auto confirm endpoint and initial command work.
jrmccannon Oct 23, 2025
4645222
Adding validator
jrmccannon Oct 23, 2025
6a29377
Finished command implementation.
jrmccannon Oct 23, 2025
89b8d59
Enabled the feature renomved used method. Enabled the policy in the tโ€ฆ
jrmccannon Oct 24, 2025
f60f0f5
Added extension functions to allow for railroad programming.
jrmccannon Oct 27, 2025
0ba770d
Removed guid from route template. Added xml docs
jrmccannon Oct 27, 2025
e5f07de
Merge branch 'refs/heads/main' into jmccannon/ac/pm-26636-auto-confirโ€ฆ
jrmccannon Oct 28, 2025
5527ca6
Added validation for command.
jrmccannon Oct 28, 2025
27ce4b8
Added default collection creation to command.
jrmccannon Oct 28, 2025
2069c9f
formatting.
jrmccannon Oct 28, 2025
cb81d29
Added additional error types and mapped to appropriate results.
jrmccannon Oct 29, 2025
24a39d8
Merge branch 'main' into jmccannon/ac/pm-26636-auto-confirm-user-command
jrmccannon Oct 29, 2025
a1b2bac
Added tests for auto confirm validator
jrmccannon Oct 30, 2025
7307454
Adding tests
jrmccannon Oct 30, 2025
1303b22
fixing file name
jrmccannon Oct 31, 2025
4bb8734
Cleaned up OrgUserController. Added integration tests.
jrmccannon Oct 31, 2025
68afb17
Consolidated CommandResult and validation result stuff into a v2 direโ€ฆ
jrmccannon Oct 31, 2025
aee2734
changing result to match handle method.
jrmccannon Oct 31, 2025
031c080
Moves validation thenasync method.
jrmccannon Oct 31, 2025
805eba9
Added brackets.
jrmccannon Nov 3, 2025
ce596b2
Updated XML comment
jrmccannon Nov 3, 2025
6e5188f
Adding idempotency comment.
jrmccannon Nov 3, 2025
d6a5813
Merge branch 'refs/heads/main' into jmccannon/ac/pm-26636-auto-confirโ€ฆ
jrmccannon Nov 3, 2025
9d47bf4
Fixed up merge problems. Fixed return types for handle.
jrmccannon Nov 3, 2025
929ac41
Renamed to ValidationRequest
jrmccannon Nov 3, 2025
f648953
I added some methods for CommandResult to cover some future use casesโ€ฆ
jrmccannon Nov 4, 2025
f800119
Fixed up logic around should create default colleciton. Added more meโ€ฆ
jrmccannon Nov 5, 2025
2e80a4d
Clearing nullable enable.
jrmccannon Nov 5, 2025
9b009b2
Fixed up validator tests.
jrmccannon Nov 5, 2025
e1eeaf9
Tests for auto confirm command
jrmccannon Nov 5, 2025
3af7ba4
Fixed up command result and AutoConfirmCommand.
jrmccannon Nov 5, 2025
cd1eca4
Merge branch 'main' into jmccannon/ac/pm-26636-auto-confirm-user-command
jrmccannon Nov 5, 2025
34a05f4
Removed some unused methods.
jrmccannon Nov 5, 2025
72fc706
Moved autoconfirm tests to their own class.
jrmccannon Nov 5, 2025
781d6e6
Moved some stuff around. Need to clean up creation of accepted org usโ€ฆ
jrmccannon Nov 6, 2025
e5ec8d7
Moved some more code around. Folded Key into accepted constructor. reโ€ฆ
jrmccannon Nov 6, 2025
eb5f98e
Merge branch 'main' into jmccannon/ac/pm-26636-auto-confirm-user-command
jrmccannon Nov 10, 2025
7fb8910
Merge branch 'refs/heads/main' into jmccannon/ac/pm-26636-auto-confirโ€ฆ
jrmccannon Nov 12, 2025
3ee2bf4
Clean up clean up everybody everywhere. Clean up clean up everybody dโ€ฆ
jrmccannon Nov 12, 2025
44a5813
Another quick one
jrmccannon Nov 12, 2025
a35c789
Removed aggregate Errors.cs
jrmccannon Nov 12, 2025
a9d4dc3
Cleaned up validator and fixed up tests.
jrmccannon Nov 12, 2025
290f391
Fixed auto confirm repo
jrmccannon Nov 12, 2025
f02f761
Cleaned up command tests.
jrmccannon Nov 12, 2025
fa18f3b
Unused method.
jrmccannon Nov 12, 2025
0487fb4
Restoring Bulk command back to what it was. deleted handle method forโ€ฆ
jrmccannon Nov 12, 2025
084849a
Remove unused method.
jrmccannon Nov 12, 2025
46b7bfd
removed unnecssary lines and comments
jrmccannon Nov 12, 2025
b4d5d50
fixed layout.
jrmccannon Nov 12, 2025
8dd84e2
Fixed test.
jrmccannon Nov 12, 2025
b91def1
fixed spelling mistake. removed unused import.
jrmccannon Nov 12, 2025
032b1f8
Merge branch 'main' into jmccannon/ac/pm-26636-auto-confirm-user-command
jrmccannon Nov 14, 2025
cdcff93
Update test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUโ€ฆ
jrmccannon Nov 14, 2025
7a5fd87
Merge remote-tracking branch 'origin/jmccannon/ac/pm-26636-auto-confiโ€ฆ
jrmccannon Nov 14, 2025
c301185
Ensuring collection is created before full sync. Cleaning up tests anโ€ฆ
jrmccannon Nov 14, 2025
4551891
Added org cleanup
jrmccannon Nov 14, 2025
6187cf0
Lowering to 5 to see if that helps the runner.
jrmccannon Nov 14, 2025
2b0b1ee
:shrug:
jrmccannon Nov 14, 2025
80872b8
Trying this
jrmccannon Nov 14, 2025
6b0af95
Maybe this time will be different.
jrmccannon Nov 14, 2025
64d3b1e
seeing if awaiting and checking independently will work in ci
jrmccannon Nov 14, 2025
3fb9810
I figured it out. Locally, it would be fast enough to all return NoCoโ€ฆ
jrmccannon Nov 14, 2025
420b63e
Updated tests and validator
jrmccannon Nov 18, 2025
d0fdf44
Merge branch 'main' into jmccannon/ac/pm-26636-auto-confirm-user-command
jrmccannon Nov 18, 2025
448dd1a
Fixed name
jrmccannon Nov 18, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
using Bit.Api.Vault.AuthorizationHandlers.Collections;
using Bit.Core;
using Bit.Core.AdminConsole.Enums;
using Bit.Core.AdminConsole.Models.Data;
using Bit.Core.AdminConsole.Models.Data.Organizations.Policies;
using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.AutoConfirmUser;
using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.DeleteClaimedAccount;
using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces;
using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.InviteUsers;
Expand Down Expand Up @@ -66,6 +68,8 @@ public class OrganizationUsersController : Controller
private readonly IFeatureService _featureService;
private readonly IPricingClient _pricingClient;
private readonly IResendOrganizationInviteCommand _resendOrganizationInviteCommand;
private readonly IAutomaticallyConfirmOrganizationUserCommand _automaticallyConfirmOrganizationUserCommand;
private readonly TimeProvider _timeProvider;
private readonly IConfirmOrganizationUserCommand _confirmOrganizationUserCommand;
private readonly IRestoreOrganizationUserCommand _restoreOrganizationUserCommand;
private readonly IInitPendingOrganizationCommand _initPendingOrganizationCommand;
Expand Down Expand Up @@ -97,7 +101,9 @@ public OrganizationUsersController(IOrganizationRepository organizationRepositor
IRestoreOrganizationUserCommand restoreOrganizationUserCommand,
IInitPendingOrganizationCommand initPendingOrganizationCommand,
IRevokeOrganizationUserCommand revokeOrganizationUserCommand,
IResendOrganizationInviteCommand resendOrganizationInviteCommand)
IResendOrganizationInviteCommand resendOrganizationInviteCommand,
IAutomaticallyConfirmOrganizationUserCommand automaticallyConfirmOrganizationUserCommand,
TimeProvider timeProvider)
{
_organizationRepository = organizationRepository;
_organizationUserRepository = organizationUserRepository;
Expand All @@ -122,6 +128,8 @@ public OrganizationUsersController(IOrganizationRepository organizationRepositor
_featureService = featureService;
_pricingClient = pricingClient;
_resendOrganizationInviteCommand = resendOrganizationInviteCommand;
_automaticallyConfirmOrganizationUserCommand = automaticallyConfirmOrganizationUserCommand;
_timeProvider = timeProvider;
_confirmOrganizationUserCommand = confirmOrganizationUserCommand;
_restoreOrganizationUserCommand = restoreOrganizationUserCommand;
_initPendingOrganizationCommand = initPendingOrganizationCommand;
Expand Down Expand Up @@ -691,6 +699,56 @@ public async Task PatchBulkEnableSecretsManagerAsync(Guid orgId,
await BulkEnableSecretsManagerAsync(orgId, model);
}


[Authorize<ManageUsersRequirement>]
[HttpPost("{id}/auto-confirm")]
public async Task<IActionResult> AutomaticallyConfirmOrganizationUserAsync([FromRoute] Guid orgId,
[FromRoute] Guid id,
[FromBody] OrganizationUserConfirmRequestModel model)
{
if (!_featureService.IsEnabled(FeatureFlagKeys.AutomaticConfirmUsers))
{
return NotFound();
}

var userId = _userService.GetProperUserId(User);

if (userId is null || userId.Value == Guid.Empty)
{
return Unauthorized();
}

var result = await _automaticallyConfirmOrganizationUserCommand.AutomaticallyConfirmOrganizationUserAsync(
new AutomaticallyConfirmOrganizationUserRequest
{
OrganizationId = orgId,
OrganizationUserId = id,
Key = model.Key,
DefaultUserCollectionName = model.DefaultUserCollectionName,
PerformedBy = new StandardUser(userId.Value, await _currentContext.OrganizationOwner(orgId)),
Copy link
Contributor

Choose a reason for hiding this comment

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

โ“ Authorization question: The StandardUser is created with await _currentContext.OrganizationOwner(orgId), which checks if the current user is an owner. However, the [Authorize<ManageUsersRequirement>] attribute already ensures the user can manage users (which includes Admins, not just Owners).

Is it intentional that only Owners (not Admins) are allowed to auto-confirm users? If Admins should also be able to auto-confirm, this check may be too restrictive. The ManageUsersRequirement suggests both should be allowed.

Copy link
Contributor

Choose a reason for hiding this comment

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

โŒ Authorization inconsistency: The StandardUser is created with await _currentContext.OrganizationOwner(orgId), which only returns true for Owners, not Admins.

However, the endpoint has [Authorize<ManageUsersRequirement>] which allows both Owners and Admins to manage users.

This creates an inconsistency where:

  • Admins can pass the authorization check
  • But PerformedBy will have IsOwner = false for Admins
  • If any downstream logic depends on IsOwner, Admins won't be able to use this feature despite being authorized

Questions:

  1. Is this intentional? Should only Owners be able to auto-confirm users?
  2. If Admins should also be allowed, consider using _currentContext.OrganizationAdmin(orgId) or a different property name that doesn't imply Owner-only access.

Note: This same pattern exists in other endpoints, but for auto-confirm specifically, the validator doesn't seem to check the PerformedBy.IsOwner property, so this may currently work but is semantically confusing.

Copy link
Contributor

Choose a reason for hiding this comment

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

๐Ÿ’ญ Authorization semantic question: The StandardUser is created with await _currentContext.OrganizationOwner(orgId), which only returns true for Owners, not Admins.

However, the endpoint has [Authorize<ManageUsersRequirement>] which allows both Owners and Admins to manage users.

Questions:

  1. Is this intentional? Should only Owners be able to auto-confirm users?
  2. If Admins should also be allowed, should IsOwner be true for them, or is this property only for audit purposes?

The validator doesn't check PerformedBy.IsOwner, so this currently works but is semantically confusing. Consider either documenting this is intentional or using a different property name that doesn't imply Owner-only access.

Copy link
Contributor

Choose a reason for hiding this comment

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

๐Ÿ’ญ Authorization semantics question: _currentContext.OrganizationOwner(orgId) only returns true for Owners, not Admins. However, [Authorize<ManageUsersRequirement>] allows both Owners and Admins.

This creates confusion where:

  • Admins can pass authorization and use the endpoint
  • But PerformedBy.IsOwner will be false for Admins
  • The validator doesn't check PerformedBy.IsOwner, so this works but is semantically confusing

Question: Is this intentional? If both Owners and Admins should be able to auto-confirm, consider using a different property name that doesn't imply Owner-only access, or documenting this is for audit purposes only.

Copy link
Contributor

Choose a reason for hiding this comment

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

๐Ÿ’ญ Authorization semantic question: This creates StandardUser with IsOwner based on _currentContext.OrganizationOwner(orgId), which only returns true for Owners, not Admins.

However, the endpoint has [Authorize<ManageUsersRequirement>] which allows both Owners and Admins to use this endpoint.

Questions:

  1. Is this intentional - should IsOwner be false for Admins who use this endpoint?
  2. If yes, consider renaming to IsOwnerRole or adding a comment explaining this is for audit purposes only
  3. The validator doesn't check PerformedBy.IsOwner, so functionally this works, but the semantics are confusing

Related to @eliykat's earlier feedback about Admin vs Owner access.

PerformedOn = _timeProvider.GetUtcNow()
Copy link
Member

Choose a reason for hiding this comment

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

Passing in the date from the controller level seems unexpected to me. It's not information from the request or otherwise from the API layer (like claims in currentContext). Is there any reason the command can't get this for itself? That would also make the date more accurate because it could be fetched at the time of performing the operation. (even though the difference is probably minimal)

Copy link
Member

Choose a reason for hiding this comment

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

Unaddressed feedback (ref)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My personal preference was to grab the time when the request comes in from the Controller. That pattern also allowed for the entire request to share the same date for saving database records.

I'll move this down to be a bit more consistent

});

if (result is { IsError: true })
{
return result.AsError switch
{
NotFoundError notFound => NotFound(notFound.Message),
BadRequestError badRequest => BadRequest(badRequest.Message),
InternalError internalError => Problem(
detail: internalError.Message,
statusCode: StatusCodes.Status500InternalServerError,
title: "An unexpected error occurred. Please try again or contact support."),
_ => Problem(
detail: result.AsError.Message,
statusCode: StatusCodes.Status500InternalServerError,
title: "An unexpected error occurred")
};
}

return Ok();
}

private async Task RestoreOrRevokeUserAsync(
Guid orgId,
Guid id,
Expand Down
1 change: 1 addition & 0 deletions src/Core/AdminConsole/Enums/EventType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ public enum EventType : int
OrganizationUser_RejectedAuthRequest = 1514,
OrganizationUser_Deleted = 1515, // Both user and organization user data were deleted
OrganizationUser_Left = 1516, // User voluntarily left the organization
OrganizationUser_AutomaticallyConfirmed = 1517,

Organization_Updated = 1600,
Organization_PurgedVault = 1601,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,200 @@
๏ปฟusing Bit.Core.AdminConsole.Entities;
using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.DeleteClaimedAccount;
using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements;
using Bit.Core.Entities;
using Bit.Core.Enums;
using Bit.Core.Models.Data;
using Bit.Core.Platform.Push;
using Bit.Core.Repositories;
using Bit.Core.Services;
using Microsoft.Extensions.Logging;
using OneOf.Types;
using CommandResult = Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.DeleteClaimedAccount.CommandResult;

namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.AutoConfirmUser;

public class AutomaticallyConfirmOrganizationUserCommand(IOrganizationUserRepository organizationUserRepository,
IOrganizationRepository organizationRepository,
IAutomaticallyConfirmOrganizationUsersValidator validator,
IEventService eventService,
IMailService mailService,
IUserRepository userRepository,
IPushRegistrationService pushRegistrationService,
IDeviceRepository deviceRepository,
IPushNotificationService pushNotificationService,
IFeatureService featureService,
IPolicyRequirementQuery policyRequirementQuery,
ICollectionRepository collectionRepository,
ILogger<AutomaticallyConfirmOrganizationUserCommand> logger) : IAutomaticallyConfirmOrganizationUserCommand
{
public async Task<CommandResult> AutomaticallyConfirmOrganizationUserAsync(AutomaticallyConfirmOrganizationUserRequest request)
{
var requestData = await RetrieveDataAsync(request);

if (requestData.IsError) return requestData.AsError;

var validatedData = await validator.ValidateAsync(requestData.AsSuccess);

if (validatedData.IsError) return validatedData.AsError;

var validatedRequest = validatedData.Request;

var successfulConfirmation = await organizationUserRepository.ConfirmOrganizationUserAsync(validatedRequest.OrganizationUser);

if (!successfulConfirmation) return new None();

return await validatedRequest.ToCommandResultAsync()
.MapAsync(CreateDefaultCollectionsAsync)
.MapAsync(LogOrganizationUserConfirmedEventAsync)
.MapAsync(SendConfirmedOrganizationUserEmailAsync)
.MapAsync(DeleteDeviceRegistrationAsync)
.MapAsync(PushSyncOrganizationKeysAsync)
.ToResultAsync();
}

private async Task<CommandResult<AutomaticallyConfirmOrganizationUserRequestData>> CreateDefaultCollectionsAsync(
AutomaticallyConfirmOrganizationUserRequestData request)
{
try
{
if (!featureService.IsEnabled(FeatureFlagKeys.CreateDefaultLocation)
|| string.IsNullOrWhiteSpace(request.DefaultUserCollectionName)
|| !(await policyRequirementQuery.GetAsync<OrganizationDataOwnershipPolicyRequirement>(request.UserId))
.RequiresDefaultCollectionOnConfirm(request.Organization.Id))
{
return request;
}

await collectionRepository.CreateAsync(
new Collection
{
OrganizationId = request.Organization.Id,
Name = request.DefaultUserCollectionName,
Type = CollectionType.DefaultUserCollection
},
groups: null,
[new CollectionAccessSelection
{
Id = request.OrganizationUser.Id,
Manage = true
}]);

return request;
}
catch (Exception ex)
{
logger.LogError(ex, "Failed to create default collection for user.");
return new FailedToCreateDefaultCollection();
}
}

private async Task<CommandResult<AutomaticallyConfirmOrganizationUserRequestData>> PushSyncOrganizationKeysAsync(AutomaticallyConfirmOrganizationUserRequestData request)
{
try
{
await pushNotificationService.PushSyncOrgKeysAsync(request.UserId);
return request;
}
catch (Exception ex)
{
logger.LogError(ex, "Failed to push organization keys.");
return new FailedToPushOrganizationSyncKeys();
}
}

private async Task<CommandResult<AutomaticallyConfirmOrganizationUserRequestData>> LogOrganizationUserConfirmedEventAsync(
AutomaticallyConfirmOrganizationUserRequestData request)
{
try
{
await eventService.LogOrganizationUserEventAsync(request.OrganizationUser,
EventType.OrganizationUser_AutomaticallyConfirmed,
request.PerformedOn.UtcDateTime);
return request;
}
catch (Exception ex)
{
logger.LogError(ex, "Failed to log OrganizationUser_AutomaticallyConfirmed event.");
return new FailedToWriteToEventLog();
}
}

private async Task<CommandResult<AutomaticallyConfirmOrganizationUserRequestData>> SendConfirmedOrganizationUserEmailAsync(
AutomaticallyConfirmOrganizationUserRequestData request)
{
try
{
var user = await userRepository.GetByIdAsync(request.UserId);

if (user is null) return new UserNotFoundError();

await mailService.SendOrganizationConfirmedEmailAsync(request.Organization.Name,
user.Email,
request.OrganizationUser.AccessSecretsManager);

return request;
}
catch (Exception ex)
{
logger.LogError(ex, "Failed to send OrganizationUserConfirmed.");
return new FailedToSendConfirmedUserEmail();
}
}

private async Task<CommandResult<AutomaticallyConfirmOrganizationUserRequestData>> DeleteDeviceRegistrationAsync(
AutomaticallyConfirmOrganizationUserRequestData request)
{
try
{
var devices = (await deviceRepository.GetManyByUserIdAsync(request.UserId))
.Where(d => !string.IsNullOrWhiteSpace(d.PushToken))
.Select(d => d.Id.ToString());

await pushRegistrationService.DeleteUserRegistrationOrganizationAsync(devices, request.Organization.Id.ToString());
return request;
}
catch (Exception ex)
{
logger.LogError(ex, "Failed to delete device registration.");
return new FailedToDeleteDeviceRegistration();
}
}

private async Task<CommandResult<AutomaticallyConfirmOrganizationUserRequestData>> RetrieveDataAsync(
AutomaticallyConfirmOrganizationUserRequest request)
{
var organizationUser = await GetOrganizationUserAsync(request.OrganizationUserId);

if (organizationUser.IsError) return organizationUser.AsError;

var organization = await GetOrganizationAsync(request.OrganizationId);

if (organization.IsError) return organization.AsError;

return new AutomaticallyConfirmOrganizationUserRequestData
{
OrganizationUser = organizationUser.AsSuccess,
Organization = organization.AsSuccess,
PerformedBy = request.PerformedBy,
Key = request.Key,
DefaultUserCollectionName = request.DefaultUserCollectionName,
PerformedOn = request.PerformedOn
};
}

private async Task<CommandResult<OrganizationUser>> GetOrganizationUserAsync(Guid organizationUserId)
{
var organizationUser = await organizationUserRepository.GetByIdAsync(organizationUserId);

return organizationUser is { UserId: not null } ? organizationUser : new UserNotFoundError();
}

private async Task<CommandResult<Organization>> GetOrganizationAsync(Guid organizationId)
{
var organization = await organizationRepository.GetByIdAsync(organizationId);

return organization is not null ? organization : new OrganizationNotFound();
}
}
Loading
Loading