Sustainsys / Saml2

Saml2 Authentication services for ASP.NET
Other
961 stars 603 forks source link

SignOut with HttpPost binding results in exception "The response headers cannot be modified because the response has already started" #1417

Closed 54mu3l closed 11 months ago

54mu3l commented 1 year ago

Problem

When we use HttpPost as SingleLogoutServiceBinding we get the following error: The response headers cannot be modified because the response has already started.

When we use HttpRedirect as SingleLogoutServiceBinding it works flawlessly.

Has anyone seen something similar? Is this a know bug? Or just misconfiguration on our side?

Code

idp.SingleLogoutServiceBinding = Sustainsys.Saml2.WebSso.Saml2BindingType.HttpPost;
[HttpGet]
public IActionResult Logout()
{
    var props = new AuthenticationProperties()
    {
        RedirectUri = Configuration.GetValue<string>("Saml2:ReturnUrlAfterLogout")
    };

    return SignOut(props, Saml2Defaults.Scheme, CookieAuthenticationDefaults.AuthenticationScheme);
}

Log

2023-11-14 14:46:42,410 [19] DEBUG Sustainsys.Saml2.AspNetCore2.Saml2Handler - Initiating logout, checking requirements for federated logout
  Issuer of LogoutNameIdentifier claim (should be Idp entity id): https://stubidp.sustainsys.com/XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX/Metadata
  Issuer is a known Idp: True
  Session index claim (should have a value): http://Sustainsys.se/Saml2/SessionIndex: 42
  Idp has SingleLogoutServiceUrl: https://stubidp.sustainsys.com/XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX/
  There is a signingCertificate in SPOptions: True
  Idp configured to DisableOutboundLogoutRequests (should be false): False
2023-11-14 14:46:42,412 [19] DEBUG Sustainsys.Saml2.AspNetCore2.Saml2Handler - Sending message over Http POST binding
<saml2p:LogoutRequest xmlns:saml2p="urn:oasis:names:tc:SAML:2.0:protocol" xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion" ID="id964b3468c7b14ed7874fff95f2fb8c0e" Version="2.0" IssueInstant="2023-11-14T13:46:42Z" Destination="https://stubidp.sustainsys.com/XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX/">
  <saml2:Issuer>urn:XXXXXXXXXXXXXXXXXX:sp:XXXX:XXXXX</saml2:Issuer><Signature xmlns="http://www.w3.org/2000/09/xmldsig#"><SignedInfo><CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#" /><SignatureMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-sha256" /><Reference URI="#id964b3468c7b14ed7874fff95f2fb8c0e"><Transforms><Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature" /><Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#" /></Transforms><DigestMethod Algorithm="http://www.w3.org/2001/04/xmlenc#sha256" /><DigestValue>XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX=</DigestValue></Reference></SignedInfo><SignatureValue>XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX=</SignatureValue><KeyInfo><X509Data><X509Certificate>XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX</X509Certificate></X509Data></KeyInfo></Signature>
  <saml2:NameID Format="urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified">TestUser</saml2:NameID>
  <saml2p:SessionIndex>42</saml2p:SessionIndex>
</saml2p:LogoutRequest>
2023-11-14 14:46:42,413 [19] DEBUG Sustainsys.Saml2.AspNetCore2.Saml2Handler - Expanded Saml2Url
  AssertionConsumerServiceUrl: https://localhost/XXXXXXXXXXXXXXXX/AuthServices/Acs
  SignInUrl: https://localhost/XXXXXXXXXXXXXXXX/AuthServices/SignIn
  LogoutUrl: https://localhost/XXXXXXXXXXXXXXXX/AuthServices/Logout
  ApplicationUrl: https://localhost/XXXXXXXXXXXXXXXX/
2023-11-14 14:46:42,413 [19] INFO  Sustainsys.Saml2.AspNetCore2.Saml2Handler - Sending logout request to https://stubidp.sustainsys.com/XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX/Metadata
2023-11-14 14:46:42,414 [19] ERROR Microsoft.AspNetCore.Server.IIS.Core.IISHttpServer - Connection ID "15420325159281623277", Request ID "400000ee-0008-d600-b63f-84710c7967bb": An unhandled exception was thrown by the application.
System.InvalidOperationException: The response headers cannot be modified because the response has already started.
   at Microsoft.AspNetCore.HttpSys.Internal.HeaderCollection.ThrowIfReadOnly()
   at Microsoft.AspNetCore.HttpSys.Internal.HeaderCollection.set_Item(String key, StringValues value)
   at Microsoft.AspNetCore.Authentication.Cookies.ChunkingCookieManager.DeleteCookie(HttpContext context, String key, CookieOptions options)
   at Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationHandler.HandleSignOutAsync(AuthenticationProperties properties)
   at Microsoft.AspNetCore.Authentication.AuthenticationService.SignOutAsync(HttpContext context, String scheme, AuthenticationProperties properties)
   at Microsoft.AspNetCore.Mvc.SignOutResult.ExecuteAsync(HttpContext httpContext)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeNextResultFilterAsync>g__Awaited|30_0[TFilter,TFilterAsync](ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Rethrow(ResultExecutedContextSealed context)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.ResultNext[TFilter,TFilterAsync](State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.InvokeResultFilters()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeNextResourceFilter>g__Awaited|25_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Rethrow(ResourceExecutedContextSealed context)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.InvokeFilterPipelineAsync()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Awaited|17_0(ResourceInvoker invoker, Task task, IDisposable scope)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Awaited|17_0(ResourceInvoker invoker, Task task, IDisposable scope)
   at Microsoft.AspNetCore.Routing.EndpointMiddleware.<Invoke>g__AwaitRequestTask|6_0(Endpoint endpoint, Task requestTask, ILogger logger)
   at XXXXXXXXXXXXXXXX.Middleware.XXMiddleware.InvokeAsync(HttpContext context) in C:\XXXX\XXXX\XXXX\XXXXXXXXXXXXXXXX\Middleware\XXMiddleware.cs:line 23
   at XXXXXXXXXXXXXXXX.Middleware.XXMiddleware.Invoke(HttpContext context, X x, Y y) in C:\XXXX\XXXX\XXXX\XXXXXXXXXXXXXXXX\Middleware\XXMiddleware.cs:line 18
   at XXXXXXXXXXXXXXXX.Middleware.XXMiddleware.Invoke(HttpContext context, X x) in C:\XXXX\XXXX\XXXX\XXXXXXXXXXXXXXXX\Middleware\XXMiddleware.cs:line 45
   at XXXXXXXXXXXXXXXX.Middleware.XXMiddleware.Invoke(HttpContext context) in C:\XXXX\XXXX\XXXX\XXXXXXXXXXXXXXXX\Middleware\XXMiddleware.cs:line 20
   at Program.<>c.<<<Main>$>b__0_9>d.MoveNext() in C:\XXXX\XXXX\XXXX\XXXXXXXXXXXXXXXX\Program.cs:line 196
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Server.IIS.Core.IISHttpContextOfT`1.ProcessRequestAsync()

Current workaround

The following code seems to work (for now):

[HttpGet]
public async Task<IActionResult> Logout()
{
    var props = new AuthenticationProperties()
    {
        RedirectUri = Configuration.GetValue<string>("Saml2:ReturnUrlAfterLogout")
    };

    await HttpContext.SignOutAsync(CookieAuthenticationDefaults.AuthenticationScheme, props);
    await HttpContext.SignOutAsync(Saml2Defaults.Scheme, props);

    return new EmptyResult();
}

Additional info

Sustainsys.Saml2.AspNetCore2 2.9.0 .NET 7.0

AndersAbel commented 1 year ago

The POST binding writes to the body and that causes the headers to be flushed. Signing out of the cookie scheme means that a set-cookie header is set to clear the cookie.

As you've found out, the order becomes important here. The cookie signout must be done first, to be able to write the headers. After that, the Saml2 signout using POST can run.

There's no need to do the full workaround though, simply reordering the parameters should work:

return SignOut(props, CookieAuthenticationDefaults.AuthenticationScheme, Saml2Defaults.Scheme);
54mu3l commented 1 year ago

Before I opened this issue we tried reordering the parameters. But without success.

But now it works with the reordered parameters (the reason was another bug). The POST works now with the following line of code:

return SignOut(props, CookieAuthenticationDefaults.AuthenticationScheme, Saml2Defaults.Scheme);

It might be worth mentioning, that the example signs-out saml first and then the cookie: https://github.com/Sustainsys/Saml2.Samples/blob/dcbcc42093e12a988da45782446e538ca33e45eb/v2/AspNetCore/Pages/Logout.cshtml.cs#L20

Mabye the example needs an update.

AndersAbel commented 1 year ago

Good point. I've updated the samples and added a comment on why the order matters. When I wrote the sample, I probably only used HttpRedirect for the logout.