Open Closed

AuthServer 8.2.1 - /connect/token with client credentials causes error "SessionId is null. It's not possible to save the session" #8017


User avatar
0
Josh.Cunningham created
  • ABP Framework version: v8.2.1
  • UI Type: MVC
  • Database System: EF Core (SQL Server)
  • Tiered (for MVC) or Auth Server Separated (for Angular): yes
  • Exception message and full stack trace: SessionId is null. It's not possible to save the session. [OpenIddict.Server.OpenIddictServerDispatcher]
  • Steps to reproduce the issue: Call /connect/token for openiddict application using client credentials flow

Error is the same as https://abp.io/support/questions/7977/AuthServer-820-SessionId-is-null-error but the steps differ.

When logging in as a user, I can see that IdentitySessionClaimsPrincipalContributor is correctly setting the sessionId, and the error does not occur.

This error is occurring during integrations we have running as background jobs, it occurs when /connect/token is called with client credentials flow. I have been investigating the error by making the api call using Postman, examining the returned access token I there is no session id, but I'm not sure if there should.

In the course of trying to understand the error I added an IOpenIddictServerHandler<OpenIddictServerEvents.ProcessSignInContext> just before OpenIddictCreateIdentitySession (where I believe the error is occurring) to confirm that there is indeed no session id (there wasn't). I subsequently modified my handler to add in session id, not believing this to be the fix but to see what would happen; this caused an exception in OpenIddictCreateIdentitySession when it tries to get the user id, which makes sense as there isn't a user.

So I am assuming that either: there shouldn't be a session id but this handler shouldn't be being called for my client credentials, or that there should be a session id but something in the way I have configured the application is causing it to not be created. Looking at the functionality provided by the session management it doesn't seem relevant (at least with our usage) to client applications, in combination with the fact that it is trying to get the user id, I am assuming at the moment that there shouldn't be a session, but this is not my area of expertise.

I do not believe there is anything particularly special with how the application has been configured: I am not currently worried that this is causing any problems in our application, based on the fact that the handler simply logs the error and then returns, and if it didn't then the following call to IdentitySessionManager.CreateAsync would cause an exception. But the error is causing a lot of noise. I could inherit and then replace the handler, and only call the base implementation when it is not the client causing thin question; this would reduce the noise but is patently not the correct approach.

Any help would be greatly appreciated. Thanks in advance.


4 Answer(s)
  • User Avatar
    0
    liangshiwei created
    Support Team Fullstack Developer

    Hi,

    This is by designed https://github.com/abpframework/abp/pull/20045

  • User Avatar
    0
    liangshiwei created
    Support Team Fullstack Developer

    Hi,

    We will eliminate this log for client credentials

  • User Avatar
    0
    Josh.Cunningham created

    Thank you.

    Since this is creating a lot of noise for us I am intending to make the following change in the mean time to prevent the error log.

    Could you please confirm whether or not you believe this is appropriate, or suggest an alternative if not.

    public class OpenIddictCreateIdentitySessionExceptForClientCredentials : 
        OpenIddictCreateIdentitySession,
        IOpenIddictServerHandler<OpenIddictServerEvents.ProcessSignInContext>
    {
        private static readonly OpenIddictServerHandlerDescriptor _descriptor = OpenIddictServerHandlerDescriptor
            .CreateBuilder<OpenIddictServerEvents.ProcessSignInContext>()
            .UseSingletonHandler<OpenIddictCreateIdentitySessionExceptForClientCredentials>()
            .SetOrder(100000)
            .SetType(OpenIddictServerHandlerType.Custom)
            .Build();
    
        public OpenIddictCreateIdentitySessionExceptForClientCredentials(
            IdentitySessionManager identitySessionManager, 
            IWebClientInfoProvider webClientInfoProvider, 
            IOptions<AbpAccountOpenIddictOptions> options) : base(identitySessionManager, webClientInfoProvider, options)
        {
        }
    
        public new static OpenIddictServerHandlerDescriptor Descriptor
        {
            get
            {
                return _descriptor;
            }
        }
    
        public new ValueTask HandleAsync(OpenIddictServerEvents.ProcessSignInContext context)
        {    
            if (context == null)
                throw new ArgumentNullException("context");
                
            if (context.Request.IsClientCredentialsGrantType())
                return ValueTask.CompletedTask;
    
            return base.HandleAsync(context);
        }
    }
    
    public override void PreConfigureServices(ServiceConfigurationContext context)
    {
        PreConfigure<OpenIddictBuilder>(builder =>
        {
            builder.AddServer(options =>
            {
                options.RemoveEventHandler(OpenIddictCreateIdentitySession.Descriptor);
                options.AddEventHandler(OpenIddictCreateIdentitySessionExceptForClientCredentials.Descriptor);
            });
        });
    }
    

    I would also appreciate it if you could provide us with a link to any relevant issue or pull request on this ticket so that we may follow the progress of this fix and remove this code once it is redundant.

    Kind regards, Josh

  • User Avatar
    1
    liangshiwei created
    Support Team Fullstack Developer

    Hi,

    Your code is ok.

    I would also appreciate it if you could provide us with a link to any relevant issue or pull request on this ticket so that we may follow the progress of this fix and remove this code once it is redundant.

    This is an internal repo,

Made with ❤️ on ABP v9.0.0-preview Updated on September 30, 2024, 13:13