Closed tom-crayon closed 1 week ago
Thanks for your feedback. I would personally have preferred if Asp.Net had a built in enabled/disabled flag of users. But now they don't and our out-of-the-box experience for working with Asp.Net Identity should be working-out-of-the-box with the features that are in Asp.Net Identity out-of-the-box. So while I appreciate that forcing an override would make it more obvious, it would also make it harder to get started.
Having said that, I think it would make a lot of sense to point out in the docs that our Asp.Net Identity integration simply implements IsUserActiveAsync
as return true
.
Which version of Duende IdentityServer are you using? 7.x
Which version of .NET are you using? 8.x
Describe the bug
This is not a bug, but an obscured default. The implementation of
ProfileService<T>.IsUserActiveAsync(T user)
means that there is no user state check by default, it always returnstrue
. The ASP.NET IdentityIdentityUser
model doesn't include any state fields, so this seems reasonable.I expect most IdentityServer with ASP.NET Identity implementations extend the default model to add user state so they can be disabled if necessary. It's then not obvious that
IsUserActiveAsync(T user)
needs to be overridden to perform the state check, which is used by several key processes.To Reproduce
Expected behavior
At step 5, the user should be prevented from obtaining a new access token. This is the case, and IdentityServer works correctly if
IsUserActiveAsync(T user)
is overridden with the proper logic.Additional context
I have suggestions to help people avoid this mistake.
I propose a change is made to
ProfileService<T>
to force implementations that extend theIdentityUser
model to also specify when the user is active. This would prompt implementers to make a conscious decision based on their setup.I also suggest expanding the ASP.NET Identity Integration docs page to call out the need to override
IsUserActiveAsync(T user)
when using a customIdentityUser
model.Many thanks!