Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Volo.Abp.Http.Client.IdentityModel must specify Microsoft.NET.Sdk.Web sdk ? #2294

Closed
rwecho opened this issue Dec 2, 2019 · 10 comments · Fixed by #2867
Closed

Volo.Abp.Http.Client.IdentityModel must specify Microsoft.NET.Sdk.Web sdk ? #2294

rwecho opened this issue Dec 2, 2019 · 10 comments · Fixed by #2867

Comments

@rwecho
Copy link

rwecho commented Dec 2, 2019

I use abp as my services framework. and call the api by the Volo.Abp.Http.Client.IdentityModel on the WPF application.
So we develop the wpf application to the new machine, it can not run because it lacks of asp.netcore runtimes.
Then we find the cause is Volo.Abp.Http.Client.IdentityModel. It depend on the Microsoft.NET.Sdk.Web, So need to install aspnetcore runtime to run it successful.

Can replace the Sdk.Web with Sdk ?

Thanks in advance.

@rwecho
Copy link
Author

rwecho commented Dec 2, 2019

Project Volo.Abp.Http.Client is the same issue.

@hikalkan
Copy link
Member

I will check it, but probably it will not be possible because Microsoft moved some core components to Microsoft.NET.Sdk.Web (instead of NuGet packages).

@maliming
Copy link
Member

Because the IdentityModelRemoteServiceHttpClientAuthenticator in the Volo.Abp.Http.Client.IdentityModel package is using IHttpContextAccessor. The project SDK cannot be changed at this time.

@NecatiMeral
Copy link
Contributor

The IHttpContextAccessor should be available in the Microsoft.AspNetCore.Http.Abstractions nuget package, which is compatible with .net-standard 2.0. So I think the Volo.Abp.Http.Client.IdentityModel package could reference the Microsoft.AspNetCore.Http.Abstractions package.

Could this be possible?

@NecatiMeral
Copy link
Contributor

NecatiMeral commented Feb 18, 2020

Oh I haven't noticed the 'list of packages no longer being produced' section. Thanks for pointing me into this direction.
Anyway, the Volo.Abp.Http.Client-library could be netstandard 2.0 because it doesn't have a dotnetcore 3.1 dependency (all dependencies are netstandard-compatible). I've tested it on my end and it seems to work out of the box. We could implement a dotnet standard compliant version of IdentityModelRemoteServiceHttpClientAuthenticator without the usage of the IHttpContextAccessor using a abstraction that could be used in .netcore and .netfx.

Edit: The same applies for:

  • Volo.Abp.Account.HttpApi.Client
  • Volo.Abp.FeatureManagement.HttpApi.Client
  • Volo.Abp.Identity.HttpApi.Client
  • Volo.Abp.PermissionManagement.HttpApi.Client
  • Volo.Abp.TenantManagement.HttpApi.Client
  • and so on...

@maliming
Copy link
Member

hi @NecatiMeral

If you like, you can contribute code.

@NecatiMeral
Copy link
Contributor

NecatiMeral commented Feb 19, 2020

Hi @maliming,
I'm working on a PR to remove the dotnetcore 3.1 constraint from the Volo.Abp.Http.Client.IdentityModel library but I can't find any code that sets the IdentityModelRemoteServiceHttpClientAuthenticator.HttpContextAccessor property. Since the DI isn't using Autofac as default it won't be injected by any property-injection.

Can you help me out here?

EDIT: I've read that all samples and template are shipped with Autofac enabled by default. I'm assuming that the HttpContextAccessor will be used on the server-side when using auto-generated clients to pass-through the access_token.
When using auto-generated api clients on a console app, for example, the IdentityModelRemoteServiceHttpClientAuthenticator shouldn't be necessary, right?
In this case it should be enough to remove the Volo.Abp.Http.Client.IdentityModel reference from client applications and update the exising Http.Client projects (excluding Volo.Abp.Http.Client.IdentityModel) to netstandard.

Please correct me if I'm wrong.

@maliming
Copy link
Member

I can't find any code that sets the IdentityModelRemoteServiceHttpClientAuthenticator.HttpContextAccessor property. Since the DI isn't using Autofac as default it won't be injected by any property-injection.

Yes, However, if the HttpContextAccessor is null, the GetAccessTokenFromHttpContextOrNullAsync method will return null.

When using auto-generated api clients on a console app, for example, the IdentityModelRemoteServiceHttpClientAuthenticator shouldn't be necessary, right?

Seems so, I'm not sure, @hikalkan can you help?

@NecatiMeral
Copy link
Contributor

NecatiMeral commented Feb 21, 2020

Hi @maliming,

even when updating the projects to target netstandard, the dynamic clients won't work out-of-the-box on netfx because of different type assemblies (System.Guid vs System.Private.CoreLib.Guid).

I could implement a custom IApiDescriptionFinder implementation which works as an Add-In-solution but a implementation inside Volo.Abp.Http.Client would be straight-forward.

My prefered solution would be a check on DynamicProxying/ApiDescriptionFinder.cs#L60 which handles the different namespaces for core-types.

I think the (de-)serialization mustn't be updated because it works type-independent as far I've seen.
What do you think about this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants