Skip to content
This repository has been archived by the owner on Jul 31, 2024. It is now read-only.

Make it easier to change the IsActiveAsync bahavior #3434

Closed
ulrichb opened this issue Jul 15, 2019 · 8 comments
Closed

Make it easier to change the IsActiveAsync bahavior #3434

ulrichb opened this issue Jul 15, 2019 · 8 comments

Comments

@ulrichb
Copy link

ulrichb commented Jul 15, 2019

Is your feature request related to a problem? Please describe.

The current ProfileService.IsActiveAsync() API:

public virtual async Task IsActiveAsync(IsActiveContext context)
{
var sub = context.Subject?.GetSubjectId();
if (sub == null) throw new Exception("No subject Id claim present");
var user = await UserManager.FindByIdAsync(sub);
if (user == null)
{
Logger?.LogWarning("No user found matching subject Id: {0}", sub);
}
context.IsActive = user != null;
}

To extend checks on the user-object one must either override the complete method including the null-checking and logging code or call the base method and load the user again (calling UserManager.FindByIdAsync which hits the database).

Describe the solution you'd like

Please separate the data loading / logging code from the check-predicate.

For example: Extract the context.IsActive = user != null line (106) into a protected virtual member which can be overridden in a derived class. (+ allow it to be "async")

Maybe better extract a boolean-returing predicate (only the user != null part, not the context.IsActive = ... part).

@ulrichb
Copy link
Author

ulrichb commented Jul 15, 2019

Btw, my use case would be:

Task<bool> async CustomIsActivePredicate(user: ApplicationUser) => 
    user != null && !await UserManager.IsLockedOutAsync(user)

@ulrichb
Copy link
Author

ulrichb commented Sep 9, 2019

Just have the same problem when overriding ProfileService.GetProfileDataAsync(). I have to do a second silly UserManager.FindByIdAsync access :/

@stale
Copy link

stale bot commented Jan 10, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jan 10, 2020
@stale stale bot removed the wontfix label Jan 10, 2020
@brockallen brockallen self-assigned this Mar 26, 2020
@brockallen
Copy link
Member

How about something like this:

afb90cb

@brockallen
Copy link
Member

PR submitted. please have a look.

leastprivilege pushed a commit that referenced this issue Mar 31, 2020
* update host to latest config

* make profile service more extensible #3434

* cleanup
@brockallen
Copy link
Member

merged? yay!

@ulrichb
Copy link
Author

ulrichb commented Sep 25, 2020

@brockallen Finally found time to upgrade to IS4 4.0 and it works great. Many thanks!

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants