Claim Issuer not persisted when using Server Side Sessions

[dheardal]

Which version of Duende IdentityServer are you using? 7

Which version of .NET are you using?

8

Describe the bug

When using Server Side Sessions, the issuer of the claim is not persisted, which means on rehydration the issuer is set to LOCAL_AUTHORITY which is the default for a claim.

This causes issues with the Sustainsys.SAML2 Single Logout as it uses the issuer from the claim in order to redirect to the identity provider. See https://github.com/Sustainsys/Saml2/blob/3780695449868fdeb708f0665eadd042a1a27153/Sustainsys.Saml2/WebSSO/LogOutCommand.cs#L168

To Reproduce

Setup identity server that uses Sustainsys.SAML2 as an external identity provider with single logout enabled and enable server side sessions. Then attempt to logout.

Expected behavior

The claim should be rehydrated with the original issuer in order to allow single logout.

--

The question is, was the omission of persisting the issuer a design choice and this should be handled in a different manner?

[AndersAbel]

Thanks for opening this. As the author of the Sustainsys.Saml2 package I can for sure confirm that the library expects the issuers to be persisted. I will have to discuss with my Duende colleagues if it is best to solve this on the IdentityServer side or if the Saml2 library should change. The LogoutNameIdentifier is already a structured string so the issuer could be moved from the Issuer property and into the payload.

[brockallen]

I think there would be a very easy fix -- when we persist, include the issuer if it's not the default ("LOCAL AUTHORITY") and then when deserializing we see if that's non-null and set it into the claim when it's created. @AndersAbel perhaps you can investigate if this would be viable? I'd like to not store the default if possible, as that's additional bloat.

[AndersAbel]

@dheardal We discussed this and as Brock implies above, this is a fix to be done on the Duende side. This time it was the Sustainsys.Saml2 library that failed, but it could be anything that relies on the issuer being persisted. We want the change to server side sessions to be transparent. That means we should store exactly those properties that the cookie handler normally does - no less, no more.

@brockallen I'm transferring this issue to the IdentityServer repo and assigning it to me.

[kevinmcody]

Other than turning off server side sessions, is there a workaround for this that would still allow SAML2 Single Logout?

[JscNZ]

I just found this change as it was causing some unexpected behaviour in our application after updating to 7.1.0 where we were ending up with duplicate claims in tokens.

Turns out the DefaultTokenService was now not removing inadvertently added duplicate claims which when added to the principal had different issuers. After the upgrade the ClaimComparer in the default token service saw them as distinct where before the duplicate claim was stripped because the issuers were both LOCAL_AUTHORITY.

Hopefully if anyone comes across similar going forwards this might help explain the odd behaviour as originally when reading the release notes I thought it was only applicable to server side sessions not the stored authorization code persisted grant too.