-
Notifications
You must be signed in to change notification settings - Fork 1.5k
[PM-26636] - Auto Confirm Org User Command #6488
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 14 commits
14f3e88
4645222
6a29377
89b8d59
f60f0f5
0ba770d
e5f07de
5527ca6
27ce4b8
2069c9f
cb81d29
24a39d8
a1b2bac
7307454
1303b22
4bb8734
68afb17
aee2734
031c080
805eba9
ce596b2
6e5188f
d6a5813
9d47bf4
929ac41
f648953
f800119
2e80a4d
9b009b2
e1eeaf9
3af7ba4
cd1eca4
34a05f4
72fc706
781d6e6
e5ec8d7
eb5f98e
7fb8910
3ee2bf4
44a5813
a35c789
a9d4dc3
290f391
f02f761
fa18f3b
0487fb4
084849a
46b7bfd
b4d5d50
8dd84e2
b91def1
032b1f8
cdcff93
7a5fd87
c301185
4551891
6187cf0
2b0b1ee
80872b8
6b0af95
64d3b1e
3fb9810
420b63e
d0fdf44
448dd1a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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; | ||
|
|
@@ -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; | ||
|
|
@@ -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; | ||
|
|
@@ -122,6 +128,8 @@ public OrganizationUsersController(IOrganizationRepository organizationRepositor | |
| _featureService = featureService; | ||
| _pricingClient = pricingClient; | ||
| _resendOrganizationInviteCommand = resendOrganizationInviteCommand; | ||
| _automaticallyConfirmOrganizationUserCommand = automaticallyConfirmOrganizationUserCommand; | ||
| _timeProvider = timeProvider; | ||
| _confirmOrganizationUserCommand = confirmOrganizationUserCommand; | ||
| _restoreOrganizationUserCommand = restoreOrganizationUserCommand; | ||
| _initPendingOrganizationCommand = initPendingOrganizationCommand; | ||
|
|
@@ -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)), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. โ Authorization question: The 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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. โ Authorization inconsistency: The However, the endpoint has This creates an inconsistency where:
Questions:
Note: This same pattern exists in other endpoints, but for auto-confirm specifically, the validator doesn't seem to check the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ๐ญ Authorization semantic question: The However, the endpoint has Questions:
The validator doesn't check
jrmccannon marked this conversation as resolved.
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ๐ญ Authorization semantics question: This creates confusion where:
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.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ๐ญ Authorization semantic question: This creates However, the endpoint has Questions:
Related to @eliykat's earlier feedback about Admin vs Owner access. |
||
| PerformedOn = _timeProvider.GetUtcNow() | ||
|
||
| }); | ||
|
|
||
| if (result is { IsError: true }) | ||
| { | ||
| return result.AsError switch | ||
jrmccannon marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| { | ||
| 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, | ||
|
|
||
jrmccannon marked this conversation as resolved.
Show resolved
Hide resolved
|
| 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; | ||
eliykat marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| 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(); | ||
jrmccannon marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| return await validatedRequest.ToCommandResultAsync() | ||
| .MapAsync(CreateDefaultCollectionsAsync) | ||
| .MapAsync(LogOrganizationUserConfirmedEventAsync) | ||
| .MapAsync(SendConfirmedOrganizationUserEmailAsync) | ||
| .MapAsync(DeleteDeviceRegistrationAsync) | ||
| .MapAsync(PushSyncOrganizationKeysAsync) | ||
| .ToResultAsync(); | ||
eliykat marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| private async Task<CommandResult<AutomaticallyConfirmOrganizationUserRequestData>> CreateDefaultCollectionsAsync( | ||
| AutomaticallyConfirmOrganizationUserRequestData request) | ||
| { | ||
| try | ||
| { | ||
| if (!featureService.IsEnabled(FeatureFlagKeys.CreateDefaultLocation) | ||
jrmccannon marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| || 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 | ||
| }]); | ||
eliykat marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| 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; | ||
jrmccannon marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| var organization = await GetOrganizationAsync(request.OrganizationId); | ||
|
|
||
| if (organization.IsError) return organization.AsError; | ||
|
|
||
| return new AutomaticallyConfirmOrganizationUserRequestData | ||
eliykat marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| { | ||
| 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(); | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.