From 32a310ecacf90fa16b1087a975eb1a294a0b465c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Chalet?= Date: Fri, 3 Jul 2026 18:01:24 +0200 Subject: [PATCH] Refactor the correlation cookie extraction logic --- .../OpenIddictClientAspNetCoreHandlers.cs | 75 ++++++++++--------- .../OpenIddictClientOwinHandlers.cs | 75 ++++++++++--------- 2 files changed, 82 insertions(+), 68 deletions(-) diff --git a/src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreHandlers.cs b/src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreHandlers.cs index a26e73fb..3adc5ca6 100644 --- a/src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreHandlers.cs +++ b/src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreHandlers.cs @@ -9,6 +9,7 @@ using System.Buffers.Text; using System.Collections.Immutable; using System.ComponentModel; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Runtime.InteropServices; using System.Security.Claims; using System.Text; @@ -418,40 +419,9 @@ public static partial class OpenIddictClientAspNetCoreHandlers return ValueTask.CompletedTask; } - try - { - // Extract the payload and validate the version marker. - var payload = Base64Url.DecodeFromChars(value); - if (payload.Length < (1 + sizeof(uint)) || payload[0] is not 0x01) - { - context.Reject( - error: Errors.InvalidRequest, - description: SR.GetResourceString(SR.ID2163), - uri: SR.FormatID8000(SR.ID2163)); - - return ValueTask.CompletedTask; - } - - // Extract the length of the request forgery protection. - var length = (int) BinaryPrimitives.ReadUInt32BigEndian(payload.AsSpan(1, sizeof(uint))); - if (length is 0 || length != (payload.Length - (1 + sizeof(uint)))) - { - context.Reject( - error: Errors.InvalidRequest, - description: SR.GetResourceString(SR.ID2163), - uri: SR.FormatID8000(SR.ID2163)); - - return ValueTask.CompletedTask; - } - - // Note: since the correlation cookie is not protected against tampering, an unexpected - // value may be present in the cookie payload and this call may return a string whose - // length doesn't match the expected value. In any case, any tampering attempt will be - // detected when comparing the resolved value with the expected value stored in the state. - context.RequestForgeryProtection = Encoding.UTF8.GetString(payload, index: 5, length); - } - - catch (Exception exception) when (!OpenIddictHelpers.IsFatal(exception)) + // Try to extract the request forgery protection from the correlation cookie. If the value + // cannot be extracted, return a generic error indicating the cookie is invalid or malformed. + if (!TryGetRequestForgeryProtection(value, out string? result)) { context.Reject( error: Errors.InvalidRequest, @@ -461,12 +431,49 @@ public static partial class OpenIddictClientAspNetCoreHandlers return ValueTask.CompletedTask; } + context.RequestForgeryProtection = result; + // Return a response header asking the browser to delete the state cookie. // // Note: when deleting a cookie, the same options used when creating it MUST be specified. request.HttpContext.Response.Cookies.Delete(name, builder.Build(request.HttpContext)); return ValueTask.CompletedTask; + + static bool TryGetRequestForgeryProtection(ReadOnlySpan input, [NotNullWhen(true)] out string? output) + { + try + { + // Extract the payload and validate the version marker. + var payload = Base64Url.DecodeFromChars(input); + if (payload.Length < (1 + sizeof(uint)) || payload[0] is not 0x01) + { + output = null; + return false; + } + + // Extract the length of the request forgery protection. + var length = (int) BinaryPrimitives.ReadUInt32BigEndian(payload.AsSpan(1, sizeof(uint))); + if (length is 0 || length != (payload.Length - (1 + sizeof(uint)))) + { + output = null; + return false; + } + + // Note: since the correlation cookie is not protected against tampering, an unexpected + // value may be present in the cookie payload and this call may return a string whose + // length doesn't match the expected value. In any case, any tampering attempt will be + // detected when comparing the resolved value with the expected value stored in the state. + output = Encoding.UTF8.GetString(payload, index: 5, length); + return true; + } + + catch (Exception exception) when (!OpenIddictHelpers.IsFatal(exception)) + { + output = null; + return false; + } + } } } diff --git a/src/OpenIddict.Client.Owin/OpenIddictClientOwinHandlers.cs b/src/OpenIddict.Client.Owin/OpenIddictClientOwinHandlers.cs index af6d3e70..a29c0c0b 100644 --- a/src/OpenIddict.Client.Owin/OpenIddictClientOwinHandlers.cs +++ b/src/OpenIddict.Client.Owin/OpenIddictClientOwinHandlers.cs @@ -9,6 +9,7 @@ using System.Buffers.Text; using System.Collections.Immutable; using System.ComponentModel; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Security.Claims; using System.Text; @@ -429,40 +430,9 @@ public static partial class OpenIddictClientOwinHandlers return ValueTask.CompletedTask; } - try - { - // Extract the payload and validate the version marker. - var payload = Base64Url.DecodeFromChars(value); - if (payload.Length < (1 + sizeof(uint)) || payload[0] is not 0x01) - { - context.Reject( - error: Errors.InvalidRequest, - description: SR.GetResourceString(SR.ID2163), - uri: SR.FormatID8000(SR.ID2163)); - - return ValueTask.CompletedTask; - } - - // Extract the length of the request forgery protection. - var length = (int) BinaryPrimitives.ReadUInt32BigEndian(payload.AsSpan(1, sizeof(uint))); - if (length is 0 || length != (payload.Length - (1 + sizeof(uint)))) - { - context.Reject( - error: Errors.InvalidRequest, - description: SR.GetResourceString(SR.ID2163), - uri: SR.FormatID8000(SR.ID2163)); - - return ValueTask.CompletedTask; - } - - // Note: since the correlation cookie is not protected against tampering, an unexpected - // value may be present in the cookie payload and this call may return a string whose - // length doesn't match the expected value. In any case, any tampering attempt will be - // detected when comparing the resolved value with the expected value stored in the state. - context.RequestForgeryProtection = Encoding.UTF8.GetString(payload, index: 5, length); - } - - catch (Exception exception) when (!OpenIddictHelpers.IsFatal(exception)) + // Try to extract the request forgery protection from the correlation cookie. If the value + // cannot be extracted, return a generic error indicating the cookie is invalid or malformed. + if (!TryGetRequestForgeryProtection(value, out string? result)) { context.Reject( error: Errors.InvalidRequest, @@ -472,6 +442,8 @@ public static partial class OpenIddictClientOwinHandlers return ValueTask.CompletedTask; } + context.RequestForgeryProtection = result; + // Return a response header asking the browser to delete the state cookie. // // Note: when deleting a cookie, the same options used when creating it MUST be specified. @@ -485,6 +457,41 @@ public static partial class OpenIddictClientOwinHandlers }); return ValueTask.CompletedTask; + + static bool TryGetRequestForgeryProtection(ReadOnlySpan input, [NotNullWhen(true)] out string? output) + { + try + { + // Extract the payload and validate the version marker. + var payload = Base64Url.DecodeFromChars(input); + if (payload.Length < (1 + sizeof(uint)) || payload[0] is not 0x01) + { + output = null; + return false; + } + + // Extract the length of the request forgery protection. + var length = (int) BinaryPrimitives.ReadUInt32BigEndian(payload.AsSpan(1, sizeof(uint))); + if (length is 0 || length != (payload.Length - (1 + sizeof(uint)))) + { + output = null; + return false; + } + + // Note: since the correlation cookie is not protected against tampering, an unexpected + // value may be present in the cookie payload and this call may return a string whose + // length doesn't match the expected value. In any case, any tampering attempt will be + // detected when comparing the resolved value with the expected value stored in the state. + output = Encoding.UTF8.GetString(payload, index: 5, length); + return true; + } + + catch (Exception exception) when (!OpenIddictHelpers.IsFatal(exception)) + { + output = null; + return false; + } + } } }