Add isAdminOperation to ResetPasswordAsync

[avraimakis]

Updated Directory.Build.props to increment version values. Enhanced ResetPasswordAsync in ExtendedUserManager<TUser> to include isAdminOperation for admin/non-admin resets. Adjusted event publishing logic based on isAdminOperation. Updated UserHandlers and BasePasswordExpiredModel to pass appropriate isAdminOperation values.

[Copilot]

Pull request overview

This PR extends ExtendedUserManager<TUser>.ResetPasswordAsync with an isAdminOperation flag so the identity event emitted differs between admin-initiated password sets and non-admin password changes, and updates call sites accordingly. It also bumps Identity-related version prefixes.

Changes:

• Added isAdminOperation parameter to the ExtendedUserManager<TUser>.ResetPasswordAsync(user, newPassword, ...) overload and switched event publishing based on admin vs non-admin resets.
• Updated UI and server handlers to pass the correct isAdminOperation value for password resets.
• Incremented Identity/Identity.UI version prefixes in Directory.Build.props.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
src/Indice.Features.Identity.UI/Pages/PasswordExpired.cs	Passes isAdminOperation: false for non-admin password-expired reset flow.
src/Indice.Features.Identity.Server/Manager/UserHandlers.cs	Updates admin “set password” handler to mark the operation as admin-driven (but currently has a compile issue).
src/Indice.Features.Identity.Core/ExtendedUserManager.cs	Introduces isAdminOperation and conditional event publishing for password reset overload; updates XML docs.
src/Directory.Build.props	Bumps Identity and Identity.UI version prefixes.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

src/Indice.Features.Identity.Core/ExtendedUserManager.cs:314

• If the user is locked out and SetLockoutEndDateAsync fails, the method still publishes a PasswordSet/PasswordChanged event and then returns a failed IdentityResult. That can emit “success” side effects even though the returned result indicates failure (and it can be confusing because the password hash has already been updated).

        if (await IsLockedOutAsync(user)) {
            result = await SetLockoutEndDateAsync(user, null);
        }
        if (isAdminOperation) {
            await _eventService.Publish(new PasswordSetEvent(UserEventContext.InitializeFromUser(user), suppressNotification));

src/Indice.Features.Identity.Core/ExtendedUserManager.cs:298

• The XML docs still describe this overload as “administrator reset password”, but the method now supports both admin and non-admin flows (via isAdminOperation) and suppressNotification only applies to admin resets. This is easy to misuse/misinterpret for future callers (e.g., passing suppressNotification without setting isAdminOperation).

    /// <param name="suppressNotification">Whether to suppress the security notification triggered after an administrator password reset.</param>
    /// <param name="isAdminOperation">Whether the reset was executed by an administrator.</param>
    /// <returns>Whether the password was successfully updated.</returns>
    /// <remarks>This overload is used for administrator reset password. Bypasses token requirement of default <see cref="UserManager{TUser}.ResetPasswordAsync(TUser, string, string)"/></remarks>

[travlos]

🏄

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/Indice.Features.Identity.Core/ExtendedUserManager.cs:299

• ResetPasswordAsync(TUser user, string newPassword, ...) is documented as an administrator reset (and suppressNotification only makes sense in that context), but isAdminOperation defaults to false, which flips the previous/expected default behavior and makes a call like ResetPasswordAsync(user, newPassword) publish PasswordChangedEvent instead of the admin PasswordSetEvent. Consider defaulting isAdminOperation to true (preserving prior semantics) and update the remarks to reflect that this overload can also be used for non-admin flows when explicitly opted-in via the flag.

    /// <param name="suppressNotification">Whether to suppress the security notification triggered after an administrator password reset.</param>
    /// <param name="isAdminOperation">Whether the reset was executed by an administrator.</param>
    /// <returns>Whether the password was successfully updated.</returns>
    /// <remarks>This overload is used for administrator reset password. Bypasses token requirement of default <see cref="UserManager{TUser}.ResetPasswordAsync(TUser, string, string)"/></remarks>
    public async Task<IdentityResult> ResetPasswordAsync(TUser user, string newPassword, bool validatePassword = true, bool suppressNotification = false, bool isAdminOperation = false) {

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

src/Indice.Features.Identity.Core/ExtendedUserManager.cs:299

• The new optional parameter changes the default behavior of this overload: prior to this change it always behaved like an admin reset (publishing PasswordSetEvent and honoring suppressNotification). With isSelfServiceReset defaulting to true, existing callers that don’t pass the new argument will now publish PasswordChangedEvent and ignore suppressNotification, which is a behavior-breaking change.

    public async Task<IdentityResult> ResetPasswordAsync(TUser user, string newPassword, bool validatePassword = true, bool suppressNotification = false, bool isSelfServiceReset = true) {

src/Indice.Features.Identity.Core/ExtendedUserManager.cs:298

• The XML docs still describe this overload as “administrator reset password” only, but it’s now also used for authenticated self-service resets (PasswordExpired page). Also the new isSelfServiceReset parameter doc has a leading space and is missing a trailing period, and suppressNotification only applies when isSelfServiceReset is false.

This issue also appears on line 299 of the same file.

    /// <param name="suppressNotification">Whether to suppress the security notification triggered after an administrator password reset.</param>
    /// <param name="isSelfServiceReset"> Whether the reset was executed by the user themselves</param>
    /// <returns>Whether the password was successfully updated.</returns>
    /// <remarks>This overload is used for administrator reset password. Bypasses token requirement of default <see cref="UserManager{TUser}.ResetPasswordAsync(TUser, string, string)"/></remarks>

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

src/Indice.Features.Identity.Core/ExtendedUserManager.cs:299

• The new optional parameter selfServicePasswordReset defaults to true, which changes the effective default behavior of this overload compared to the previous signature (admin-style reset emitting PasswordSetEvent). Any existing callers (including external consumers) that omit the new argument will now emit PasswordChangedEvent and ignore suppressNotification, which is a behavioral breaking change.

    public async Task<IdentityResult> ResetPasswordAsync(TUser user, string newPassword, bool validatePassword = true, bool suppressNotification = false, bool selfServicePasswordReset = true) {

src/Indice.Features.Identity.Core/ExtendedUserManager.cs:299

• The PR title/description refer to adding isAdminOperation, but the API adds selfServicePasswordReset instead (inverse semantics). Aligning the naming/semantics (or updating the PR description) would help prevent accidental misuse when calling this overload.

    public async Task<IdentityResult> ResetPasswordAsync(TUser user, string newPassword, bool validatePassword = true, bool suppressNotification = false, bool selfServicePasswordReset = true) {

src/Indice.Features.Identity.Core/ExtendedUserManager.cs:317

• The new conditional event publishing path (PasswordChangedEvent vs PasswordSetEvent) is behaviorally significant but appears untested in this repo’s Identity test suite (no tests reference PasswordSetEvent, PasswordChangedEvent, or the tokenless ResetPasswordAsync(user, newPassword, ...) overload). Adding tests for both branches would help prevent regressions in notifications/event handling.

        if (selfServicePasswordReset) {
            await _eventService.Publish(new PasswordChangedEvent(UserEventContext.InitializeFromUser(user)));
        } else {
            await _eventService.Publish(new PasswordSetEvent(UserEventContext.InitializeFromUser(user), suppressNotification));
        }

src/Indice.Features.Identity.Core/ExtendedUserManager.cs:296

• XML docs are now internally inconsistent: the new parameter description has a leading space/missing period, and the remarks state this overload is for administrator resets even though it is used for self-service flows (and has a self-service flag). Consider updating the summary/remarks to describe the tokenless reset and clarify what selfServicePasswordReset controls.

This issue also appears in the following locations of the same file:

• line 299
• line 299
• line 313

    /// <param name="suppressNotification">Whether to suppress the security notification triggered after an administrator password reset.</param>
    /// <param name="selfServicePasswordReset"> Whether the reset was executed by the user themselves</param>

[travlos]

🏄🚀