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

Fix service resolution in blazor server #68

Open
kamil-gorny opened this issue Dec 2, 2024 · 3 comments · Fixed by #77
Open

Fix service resolution in blazor server #68

kamil-gorny opened this issue Dec 2, 2024 · 3 comments · Fixed by #77
Assignees
Labels
area/access-token-management Issues related to Access Token Management
Milestone

Comments

@kamil-gorny
Copy link

Which version of Duende.AccessTokenManagement are you using?
Latest version pulled directly from the repository.

Which version of .NET are you using?
.NET 8

Describe the bug
When attempting to connect a Blazor Server sample app to Azure SignalR and use Duende AccessTokenManagement, the application throws a NullReferenceException. The issue seems to originate from the BlazorServerUserAccessor, where dependencies like CircuitServicesAccessor and IHttpContextAccessor are null.

To Reproduce

  1. Pull the latest version of Duende.AccessTokenManagement from the repository.
  2. Set up a sample Blazor Server app.
  3. Add Azure SignalR to the app:
builder.Services.AddSignalR().AddAzureSignalR(options =>
{
    options.ServerStickyMode = ServerStickyMode.Required;
    options.ConnectionString = "insert your connection string here";
});

Expected behavior

The application should successfully retrieve the access token and make API calls without throwing exceptions. Dependencies like CircuitServicesAccessor and IHttpContextAccessor should be properly injected.

Log output/exception with stacktrace

Microsoft.AspNetCore.Components.Server.Circuits.RemoteRenderer
Unhandled exception rendering component: Object reference not set to an instance of an object.
System.NullReferenceException: Object reference not set to an instance of an object.
   at Duende.AccessTokenManagement.OpenIdConnect.UserAccessAccessTokenManagementService.GetAccessTokenAsync(ClaimsPrincipal user, UserTokenRequestParameters parameters, CancellationToken cancellationToken) in /Users/kamilgorny/RiderProjects/Duende.AccessTokenManagement/src/Duende.AccessTokenManagement.OpenIdConnect/UserAccessTokenManagementService.cs:line 62
   at Duende.AccessTokenManagement.OpenIdConnect.OpenIdConnectUserAccessTokenHandler.GetAccessTokenAsync(Boolean forceRenewal, CancellationToken cancellationToken) in /Users/kamilgorny/RiderProjects/Duende.AccessTokenManagement/src/Duende.AccessTokenManagement.OpenIdConnect/OpenIdConnectUserAccessTokenHandler.cs:line 56
   at Duende.AccessTokenManagement.AccessTokenHandler.SetTokenAsync(HttpRequestMessage request, Boolean forceRenewal, CancellationToken cancellationToken, String dpopNonce) in /Users/kamilgorny/RiderProjects/Duende.AccessTokenManagement/src/Duende.AccessTokenManagement/AccessTokenHandler.cs:line 87
   at Duende.AccessTokenManagement.AccessTokenHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) in /Users/kamilgorny/RiderProjects/Duende.AccessTokenManagement/src/Duende.AccessTokenManagement/AccessTokenHandler.cs:line 49
   at Microsoft.Extensions.Http.Logging.LoggingScopeHttpMessageHandler.<SendCoreAsync>g__Core|5_0(HttpRequestMessage request, Boolean useAsync, CancellationToken cancellationToken)
   at System.Net.Http.HttpClient.<SendAsync>g__Core|83_0(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationTokenSource cts, Boolean disposeCts, CancellationTokenSource pendingRequestsCts, CancellationToken originalCancellationToken)
   at BlazorServer.Services.RemoteApiService.GetData() in /Users/kamilgorny/RiderProjects/Duende.AccessTokenManagement/samples/BlazorServer/Services/RemoteApiService.cs:line 25
   at BlazorServer.Pages.FetchRemoteData.OnInitializedAsync() in /Users/kamilgorny/RiderProjects/Duende.AccessTokenManagement/samples/BlazorServer/Pages/FetchRemoteData.razor:line 32
   at Microsoft.AspNetCore.Components.ComponentBase.RunInitAndSetParametersAsync()

Additional context

I’ve tried various debugging approaches but haven’t been able to resolve this. I'm reaching out here because perhaps the creators or others in the community might have insights or ideas on what could be causing this issue and how to fix it

@josephdecock
Copy link
Member

Thanks for this bug report! I've reproduced this locally, so I'm transferring it into AccessTokenManagement's repo. As soon as we have a fix, we can push out a new preview of 3.1.

@josephdecock josephdecock transferred this issue from DuendeSoftware/Support Dec 4, 2024
@josephdecock josephdecock self-assigned this Dec 4, 2024
@josephdecock josephdecock added the area/access-token-management Issues related to Access Token Management label Dec 4, 2024
@josephdecock josephdecock added this to the atm-3.1 milestone Dec 4, 2024
@josephdecock josephdecock changed the title DuendeAccessTokenManagement not working correctly, when using AzureSignalR service in blazor server sample app Fix service resolution in blazor server Dec 4, 2024
@josephdecock
Copy link
Member

@kamil-gorny - looking at this again, I'm not certain if I reproduced exactly the same problem as you had. There is an issue in our sample code, which is that the server side token store keeps tokens in memory, but the store is now scoped and so its collection of tokens gets discarded. Changing ServerSideTokenStore in our BlazorServer sample app to have the tokens be static solves that problem for me:

    private static readonly ConcurrentDictionary<string, UserToken> _tokens = new();

And I'm able to at least add signalr and continue to use the sample app:

builder.Services.AddSignalR().AddAzureSignalR(options =>
        {
            options.ServerStickyMode = ServerStickyMode.Required;
            options.ConnectionString = "<REDACTED>";
        });

But the errors you saw were a little different so I'm wondering if this will fix it for you to. Can you please try changning the sample's store to have its _tokens be static and let us know how that works? Thanks!

@josephdecock josephdecock linked a pull request Dec 16, 2024 that will close this issue
@damianh damianh modified the milestones: atm-3.1.0, atm-future Dec 17, 2024
@kamil-gorny
Copy link
Author

@josephdecock -
Apologies for the delayed response, and thank you for revisiting this issue and suggesting the change to make _tokens static. Unfortunately, I’m still experiencing the same issue.

To help you better understand the problem, I’ve attached a video that demonstrates the behavior: https://www.youtube.com/watch?v=BIC-ILADqwU&ab_channel=KamilG%C3%B3rny Hopefully, it will provide some useful insights.

If you have any further questions or need additional information, please let me know so we can work together to pinpoint the root cause.

Looking forward to your thoughts!

@josephdecock josephdecock reopened this Jan 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/access-token-management Issues related to Access Token Management
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants