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

Added AddUserSession extension method #4026

Merged
merged 1 commit into from
Mar 24, 2020

Conversation

eric-davis
Copy link
Contributor

@eric-davis eric-davis commented Jan 28, 2020

What issue does this PR address?
Adds a missing IIdentityServerBuilder extension method to add a custom IUserSession implementation.

Does this PR introduce a breaking change?
No.

Please check if the PR fulfills these requirements

  • The commit follows our guidelines
  • Unit Tests for the changes have been added (for bug fixes / features)

Other information:
No Unit Tests were added since none of the other existing extension methods include test coverage.

@stale
Copy link

stale bot commented Feb 11, 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.
Questions are community supported only and the authors/maintainers may or may not have time to reply. If you or your company would like commercial support, please see here for more information.

@brockallen
Copy link
Member

brockallen commented Mar 24, 2020

I'm curious what your custom user session implementation needs to do?

@eric-davis
Copy link
Contributor Author

I am attempting to implement multitenancy support with IdentityServer. The custom IUserSession implementation comes into play around cookie naming; specifically the IssueSessionIdCookie method and the idsrv.session cookie.

When 2 independent sessions against 2 different tenants are active on the same machine & browser there seems to be a conflict regarding the session id cookie. So, I had to add some logic to the IssueSessionIdCookie method to add a tenant id suffix to the cookie name.

    public class MultitenantUserSession : DefaultUserSession
    {
        public MultitenantUserSession(
            IHttpContextAccessor httpContextAccessor,
            IAuthenticationSchemeProvider schemes,
            IAuthenticationHandlerProvider handlers,
            IdentityServerOptions options,
            ISystemClock clock,
            ILogger<IUserSession> logger)
            : base(httpContextAccessor, schemes, handlers, options, clock, logger)
        {
            var tenantContext = httpContextAccessor.HttpContext.GetMultiTenantContext();
            var tenantId = tenantContext?.TenantInfo?.Id;
            if (tenantId.IsNullOrEmpty() == false)
            {
                this.Options.Authentication.CheckSessionCookieName = $"idsrv.session.{tenantId}";
            }
        }

        public override void IssueSessionIdCookie(string sid)
        {
            if (this.Options.Endpoints.EnableCheckSessionEndpoint == false)
            {
                return;
            }

            if (this.HttpContext.Request.Cookies[this.CheckSessionCookieName] != sid)
            {
                this.HttpContext.Response.Cookies.Append(
                    this.CheckSessionCookieName,
                    sid,
                    this.CreateSessionIdCookieOptions());
            }
        }
    }

Is there a better way to manage this?

@brockallen
Copy link
Member

Hmm, ok.

Is there a better way to manage this?

No idea, to be honest. I've not tried to tackle that.

@brockallen brockallen added this to the 4.0 milestone Mar 24, 2020
@brockallen brockallen merged commit e6607f7 into IdentityServer:master Mar 24, 2020
@brockallen
Copy link
Member

Anyway, thx for the PR.

@eric-davis eric-davis deleted the ext-method branch March 24, 2020 18:57
@lock
Copy link

lock bot commented Apr 25, 2020

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

@lock lock bot locked as resolved and limited conversation to collaborators Apr 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants