AbpProfileService.IsActiveAsync doesn't check if the user is Active

[Estar1]

I've only just started using this boiler templates and its been good for a lot of things but I'm unsure of the issue with implementation of this code in the AbpProfileService

[UnitOfWork] public override async Task IsActiveAsync(IsActiveContext context) { var tenantId = context.Subject.Identity.GetTenantId(); using (_unitOfWorkManager.Current.SetTenantId(tenantId)) { await base.IsActiveAsync(context); } }

It doesn't check if the user is active or not and hence my Identitysever4 integration always validates the user.

I thought referencing the class would automatically do the job or I'm I meant to use it as an example to implement my own logic. Apologies for my ignorance on the usage of some of this modules.

Here is the code where I have used the AbpProfileService class

services.AddIdentityServer() .AddDeveloperSigningCredential() .AddInMemoryIdentityResources(IdentityServerConfig.GetIdentityResources()) .AddInMemoryApiResources(IdentityServerConfig.GetApiResources()) .AddInMemoryClients(IdentityServerConfig.GetClients(configuration)) .AddAbpPersistedGrants<IAbpPersistedGrantDbContext>() .AddAbpIdentityServer<User>() .AddProfileService<AbpProfileService<User>>();

Please could someone explain the best approach to use the AbpProfileService such that it can check the user is inactive or Active? This is meant for IdentityServer4 intergration

Thanks

[Mardoxx]

It should do. Impl is here https://github.com/IdentityServer/IdentityServer4.AspNetIdentity/blob/dev/src/ProfileService.cs

If you want to check if the user is locked out before issuing a token then change the logic here.

I'm unsure if it should. I'll have to have a think about it. Does it make sense to restrict a locked out user from receiving tokens? Or does it make sense to issue a token from the STS since they are authenticated, then throw unauthorised if the user is inactive when trying to access protected resource.

[Estar1]

@Mardoxx thanks for looking into this.
The logic in the Profile service is a virtual method and it would make more sense to implement the check for the additional logic in the override.
This is why it was strange to me that the AbpProfileService doesn't check the Active state of the user in the override.
Also in response to your question, I think it makes sense to restrict a locked out user from receiving tokens in my opinion.
For example in the default implementation of the TokenAuthController of ABP template, you will see that access token is generated after all the various checks are done.

[Mardoxx]

I've created a PR on IDSrv's library.. IdentityServer/IdentityServer4.AspNetIdentity#59

Will have to see what they have to say. Possibly best to keep implementation in line with their library and not change any behaviour - if you want to chance that then inherit from AbpProfileService and do something similar to what is in that PR then register with AddProfileservice<CustomProfileservice>())!

Unless, of course, ABP's maintainers want to be a a little opinionated on it!

I believe changing the behaviour to disallow tokens for locked out users, the user will never receive feedback for why they aren't getting a token back. So that is one negative point there.

[Estar1]

@Mardoxx thanks again for your promptness on this.

I like your idea. Mostly importantly you've helped me establish I wasn't that stupid and missed something obvious on the ABP framework.

However I cannot inherit from the AbpProfileService because they've already ovverriden the IsActiveAsync. So I would probably just use your logic in a CustomProfileservice that inherits from the ProfileService directly.

It would be nice to know the opinion of the ABP's maintainers on this.

Thanks!

[Mardoxx]

If the method is marked virtual on the first concrete implementation on the super class (or if it is defined in an interface) you should be able to override it on any sub class.... I have got that right haven't I?

…

On Sat, 18 Aug 2018, 16:27 Estar1, ***@***.***> wrote: @Mardoxx <https://github.com/Mardoxx> thanks again for your promptness on this. I like your idea. Mostly importantly you've helped me establish I wasn't that stupid and missed something obvious on the ABP framework. However I cannot inherit from the AbpProfileService because they've already ovverriden the IsActiveAsync. So I would probably just use your logic in a CustomProfileservice that inherits from the ProfileService directly. It would be nice to know the opinion of the ABP's maintainers on this. Thanks! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3762 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAOoMUFn0nmTsEpmZ77bEwJC00sHFQLFks5uSDJKgaJpZM4WCUIA> .

[Estar1]

@Mardoxx I think you might be right. It's embarassing I've never tried this since over a decade of dev! lol

So to basically inherit from this class and override the already ovverriden method
https://github.com/aspnetboilerplate/aspnetboilerplate/blob/dev/src/Abp.ZeroCore.IdentityServer4/IdentityServer4/AbpProfileService.cs

Overriding the override of the base class (I like it). I will give it a go..thanks!

[Mardoxx]

Awesome! Please, however, see Brock's response in my PR IdentityServer/IdentityServer4.AspNetIdentity#59 (comment)

…

On Sat, 18 Aug 2018, 16:53 Estar1, ***@***.***> wrote: @Mardoxx <https://github.com/Mardoxx> I think you are might be right. It's embarassing I've never tried this since over a decade of dev! lol So to basically inherit from this class and override the already ovverriden method https://github.com/aspnetboilerplate/aspnetboilerplate/blob/dev/src/Abp.ZeroCore.IdentityServer4/IdentityServer4/AbpProfileService.cs <http://url> Overriding the override of the base class (I like it). I will give it a go..thanks! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3762 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAOoMU0gtdG6cqypFL6UH1HyMAWl3qI4ks5uSDhrgaJpZM4WCUIA> .