Prevent monetization from overriding administrator account lockout (#7523)
What changed, and why it matters
This change fixes a bug where BTCPay Server's paid-subscription ('monetization') system could automatically re-enable an administrator account that had been manually disabled. Previously, when a subscription renewed or was bypassed, the monetization service would call SetDisabled(user, false) without knowing why the account was disabled. The patch adds a 'source' trail so monetization can tell whether it was the one that disabled the account, and it now suspends the subscription instead of overriding an admin lockout. In short: admin-imposed account lockouts can no longer be accidentally undone by billing automation.
Review whether other subsystems call SetDisabled and ensure they also supply a source. Verify that subscription suspension is reversible only by administrators and that suspended subscriptions cannot be reactivated by the user through a new checkout. Consider adding an audit log entry when an admin lockout causes subscription suspension.
Security signals we found
Privilege/authorization bypass: automated subsystem overriding an administrative account-disable action
Missing provenance/audit trail in security-sensitive state change (account disabled flag)
Business-logic flaw in subscription lifecycle interacting with identity lockout
Patch adds event sourcing and source attribution to cross-cutting SetDisabled operation
Evidence from the diff
The commit introduces UserEvent.DisabledChanged with an optional Source field and updates UserService.SetDisabled to publish that event. MonetizationHostedService now tags its own SetDisabled calls with nameof(MonetizationHostedService) and listens for DisabledChanged events from other sources. When an account is disabled by a non-monetization source while an active subscription exists, the service suspends the subscription rather than allowing future monetization events to re-enable the user. This prevents monetization lifecycle events from overriding administrator-initiated lockouts.
Changed components
BTCPayServer.Services.UserServiceBTCPayServer.Plugins.Monetization.MonetizationHostedServiceBTCPayServer.Events.UserEventAccount lockout / disable subsystemSubscriptions / monetization pluginInspect captured patch +28 / −6
### BTCPayServer/Events/UserEvent.cs
@@ -61,6 +61,11 @@ public class BypassMonetizationChanged(ApplicationUser user, bool bypass, Reques
public bool Bypass { get; } = bypass;
public RequestBaseUrl RequestBaseUrl { get; } = requestBaseUrl;
}
+ public class DisabledChanged(ApplicationUser user, bool disabled, string? source = null) : UserEvent(user)
+ {
+ public bool Disabled { get; } = disabled;
+ public string? Source { get; } = source;
+ }
public class Approved(ApplicationUser user, string loginLink) : UserEvent(user)
{
public string LoginLink { get; set; } = loginLink;
### BTCPayServer/Plugins/Monetization/MonetizationHostedService.cs
@@ -3,7 +3,6 @@
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
-using BTCPayServer.Abstractions;
using BTCPayServer.Data;
using BTCPayServer.Data.Subscriptions;
using BTCPayServer.Events;
@@ -27,6 +26,7 @@ public class MonetizationHostedService(
BTCPayServerSecurityStampValidator.SecurityStampInvalidator securityStampInvalidator,
ISettingsAccessor<MonetizationSettings> monetizationSettingsAccessor,
IServiceScopeFactory serviceScopeFactory,
+ SubscriptionHostedService subsService,
Logs logger) : EventHostedServiceBase(eventAggregator, logger)
{
public class MonetizationLockoutUpdated((string UserId, bool LockoutEnabled)[] updated)
@@ -44,6 +44,7 @@ protected override void SubscribeToEvents()
this.SubscribeAny<UserEvent.BypassMonetizationChanged>();
this.SubscribeAny<UserEvent.Registered>();
this.SubscribeAny<UserEvent.Deleted>();
+ this.SubscribeAny<UserEvent.DisabledChanged>();
}
protected override async Task ProcessEvent(object evt, CancellationToken cancellationToken)
@@ -112,7 +113,7 @@ protected override async Task ProcessEvent(object evt, CancellationToken cancell
var userSub = await ctx.Subscribers.GetBySelector(byPassOfferingId, CustomerSelector.ByIdentity(SubscriberDataExtensions.IdentityType, changed.User.Id));
if (userSub is not null)
{
- await userService.SetDisabled(changed.User.Id, false);
+ await userService.SetDisabled(changed.User.Id, false, nameof(MonetizationHostedService));
EventAggregator.Publish(new MonetizationLockoutUpdated([(changed.User.Id, false)]));
}
}
@@ -125,7 +126,7 @@ protected override async Task ProcessEvent(object evt, CancellationToken cancell
var shouldBeLocked = !userSub.IsActive || userSub.Phase == SubscriberData.PhaseTypes.Expired;
if (shouldBeLocked)
{
- await userService.SetDisabled(changed.User.Id, true);
+ await userService.SetDisabled(changed.User.Id, true, nameof(MonetizationHostedService));
securityStampInvalidator.Invalidate(changed.User.Id);
EventAggregator.Publish(new MonetizationLockoutUpdated([(changed.User.Id, true)]));
}
@@ -148,6 +149,17 @@ protected override async Task ProcessEvent(object evt, CancellationToken cancell
}
}
+ if (evt is UserEvent.DisabledChanged { Disabled: true, Source: not nameof(MonetizationHostedService) } disabledChanged &&
+ monetizationSettingsAccessor.Settings is { OfferingId: { } dcOfferingId })
+ {
+ await using var ctx = dbContextFactory.CreateContext();
+ var sub = await ctx.Subscribers.GetBySelector(dcOfferingId,
+ CustomerSelector.ByIdentity(SubscriberDataExtensions.IdentityType, disabledChanged.User.Id));
+ if (sub is { IsSuspended: false })
+ {
+ await subsService.Suspend(sub.Id, "Account disabled by administrator");
+ }
+ }
if (evt is UserEvent.Registered reg && monetizationSettingsAccessor.Settings is
{
@@ -198,7 +210,7 @@ private async Task UpdateUserLockout(SubscriptionEvent.SubscriberEvent evt, User
var userId = evt.Subscriber.GetApplicationUserId();
var user = await userManager.FindByIdAsync(userId ?? "");
if (user is not null &&
- await userService.SetDisabled(user.Id, !activated) is not UserService.SetDisabledResult.Error)
+ await userService.SetDisabled(user.Id, !activated, nameof(MonetizationHostedService)) is not UserService.SetDisabledResult.Error)
{
EventAggregator.Publish(new MonetizationLockoutUpdated([(user.Id, !activated)]));
}
### BTCPayServer/Plugins/Subscriptions/Controllers/UIPlanCheckoutController.cs
@@ -1,4 +1,4 @@
-#nullable enable
+#nullable enable
using System.Threading;
using System.Threading.Tasks;
using BTCPayServer.Client.Models;
@@ -7,6 +7,7 @@
using BTCPayServer.Services;
using BTCPayServer.Views.UIStoreMembership;
using Microsoft.AspNetCore.Authorization;
+using Microsoft.AspNetCore.Identity;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.Localization;
### BTCPayServer/Services/UserService.cs
@@ -16,6 +16,7 @@
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Localization;
using Microsoft.Extensions.Logging;
+using static BTCPayServer.Events.UserEvent;
namespace BTCPayServer.Services
{
@@ -206,7 +207,9 @@ public record Success : SetDisabledResult;
public record Error(IdentityError[] Errors) : SetDisabledResult;
}
- public async Task<SetDisabledResult> SetDisabled(string userId, bool disabled)
+ public Task<SetDisabledResult> SetDisabled(string userId, bool disabled)
+ => SetDisabled(userId, disabled, null);
+ public async Task<SetDisabledResult> SetDisabled(string userId, bool disabled, string? source)
{
using var scope = _serviceProvider.CreateScope();
var userManager = scope.ServiceProvider.GetRequiredService<UserManager<ApplicationUser>>();
@@ -229,6 +232,7 @@ public async Task<SetDisabledResult> SetDisabled(string userId, bool disabled)
}
await using var ctx = _applicationDbContextFactory.CreateContext();
await ctx.Users.UpdateStoreNoActiveUserForUsers([userId]);
+ _eventAggregator.Publish(new UserEvent.DisabledChanged(user, disabled, source));
}
return res.Succeeded ? new SetDisabledResult.Success() : new SetDisabledResult.Error(res.Errors.ToArray());Why this scored 61/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.