From 18d21687ccb37285cd1ef0336aa99c4046243d1a Mon Sep 17 00:00:00 2001 From: martincostello Date: Wed, 31 Mar 2021 18:37:52 +0100 Subject: [PATCH 1/3] Do not serialize Parameters Explicitly prevent the Parameters dictionary from being included in the deserialized payload. See https://github.com/dotnet/aspnetcore/pull/31330#discussion_r605078290. --- .../src/AuthenticationProperties.cs | 1 + .../test/AuthenticationPropertiesTests.cs | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/src/Http/Authentication.Abstractions/src/AuthenticationProperties.cs b/src/Http/Authentication.Abstractions/src/AuthenticationProperties.cs index 3a72df17bda8..00856d05ba0f 100644 --- a/src/Http/Authentication.Abstractions/src/AuthenticationProperties.cs +++ b/src/Http/Authentication.Abstractions/src/AuthenticationProperties.cs @@ -65,6 +65,7 @@ public AuthenticationProperties Clone() /// Collection of parameters that are passed to the authentication handler. These are not intended for /// serialization or persistence, only for flowing data between call sites. /// + [JsonIgnore] public IDictionary Parameters { get; } /// diff --git a/src/Http/Authentication.Core/test/AuthenticationPropertiesTests.cs b/src/Http/Authentication.Core/test/AuthenticationPropertiesTests.cs index e486ce3255a7..62bbf30a4e66 100644 --- a/src/Http/Authentication.Core/test/AuthenticationPropertiesTests.cs +++ b/src/Http/Authentication.Core/test/AuthenticationPropertiesTests.cs @@ -320,6 +320,12 @@ public void Roundtrip_Serializes_With_SystemTextJson() props.Parameters.Add("baz", "quux"); var json = JsonSerializer.Serialize(props); + + // Verify that Parameters was not serialized + Assert.NotNull(json); + Assert.DoesNotContain("baz", json); + Assert.DoesNotContain("quux", json); + var deserialized = JsonSerializer.Deserialize(json); Assert.NotNull(deserialized); @@ -339,6 +345,20 @@ public void Roundtrip_Serializes_With_SystemTextJson() Assert.Equal(0, deserialized.Parameters.Count); } + [Fact] + public void Parameters_Is_Not_Deserialized_With_SystemTextJson() + { + var json = @"{""Parameters"":{""baz"":""quux""}}"; + + var deserialized = JsonSerializer.Deserialize(json); + + Assert.NotNull(deserialized); + + // Ensure that parameters is not deserialized from a raw payload + Assert.NotNull(deserialized!.Parameters); + Assert.Equal(0, deserialized.Parameters.Count); + } + public class MyAuthenticationProperties : AuthenticationProperties { public new DateTimeOffset? GetDateTimeOffset(string key) From 4cf811664eea634c1a17df2bfc691ed669a020f1 Mon Sep 17 00:00:00 2001 From: martincostello Date: Wed, 31 Mar 2021 18:52:42 +0100 Subject: [PATCH 2/3] Ignore props backed by Items Also ignore the properties backed by the Items dictionary to minimise the size of the serialized JSON payload. --- .../src/AuthenticationProperties.cs | 5 ++++ .../test/AuthenticationPropertiesTests.cs | 30 +++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/src/Http/Authentication.Abstractions/src/AuthenticationProperties.cs b/src/Http/Authentication.Abstractions/src/AuthenticationProperties.cs index 00856d05ba0f..383ace3de04c 100644 --- a/src/Http/Authentication.Abstractions/src/AuthenticationProperties.cs +++ b/src/Http/Authentication.Abstractions/src/AuthenticationProperties.cs @@ -71,6 +71,7 @@ public AuthenticationProperties Clone() /// /// Gets or sets whether the authentication session is persisted across multiple requests. /// + [JsonIgnore] public bool IsPersistent { get => GetString(IsPersistentKey) != null; @@ -80,6 +81,7 @@ public bool IsPersistent /// /// Gets or sets the full path or absolute URI to be used as an http redirect response value. /// + [JsonIgnore] public string? RedirectUri { get => GetString(RedirectUriKey); @@ -89,6 +91,7 @@ public string? RedirectUri /// /// Gets or sets the time at which the authentication ticket was issued. /// + [JsonIgnore] public DateTimeOffset? IssuedUtc { get => GetDateTimeOffset(IssuedUtcKey); @@ -98,6 +101,7 @@ public DateTimeOffset? IssuedUtc /// /// Gets or sets the time at which the authentication ticket expires. /// + [JsonIgnore] public DateTimeOffset? ExpiresUtc { get => GetDateTimeOffset(ExpiresUtcKey); @@ -107,6 +111,7 @@ public DateTimeOffset? ExpiresUtc /// /// Gets or sets if refreshing the authentication session should be allowed. /// + [JsonIgnore] public bool? AllowRefresh { get => GetBool(RefreshKey); diff --git a/src/Http/Authentication.Core/test/AuthenticationPropertiesTests.cs b/src/Http/Authentication.Core/test/AuthenticationPropertiesTests.cs index 62bbf30a4e66..37a83e35e45c 100644 --- a/src/Http/Authentication.Core/test/AuthenticationPropertiesTests.cs +++ b/src/Http/Authentication.Core/test/AuthenticationPropertiesTests.cs @@ -359,6 +359,36 @@ public void Parameters_Is_Not_Deserialized_With_SystemTextJson() Assert.Equal(0, deserialized.Parameters.Count); } + [Fact] + public void Serialization_Is_Minimised_With_SystemTextJson() + { + var props = new AuthenticationProperties() + { + AllowRefresh = true, + ExpiresUtc = new DateTimeOffset(2021, 03, 28, 13, 47, 00, TimeSpan.Zero), + IssuedUtc = new DateTimeOffset(2021, 03, 28, 12, 47, 00, TimeSpan.Zero), + IsPersistent = true, + RedirectUri = "/foo/bar" + }; + + props.Items.Add("foo", "bar"); + + var options = new JsonSerializerOptions() { WriteIndented = true }; // Indented for readability if test fails + var json = JsonSerializer.Serialize(props, options); + + // Verify that the payload doesn't duplicate the properties backed by Items + Assert.Equal(@"{ + ""Items"": { + "".refresh"": ""True"", + "".expires"": ""Sun, 28 Mar 2021 13:47:00 GMT"", + "".issued"": ""Sun, 28 Mar 2021 12:47:00 GMT"", + "".persistent"": """", + "".redirect"": ""/foo/bar"", + ""foo"": ""bar"" + } +}", json); + } + public class MyAuthenticationProperties : AuthenticationProperties { public new DateTimeOffset? GetDateTimeOffset(string key) From 6d186ce78c3bcc0ee99dda0e26f4b66030439e48 Mon Sep 17 00:00:00 2001 From: martincostello Date: Wed, 31 Mar 2021 21:14:16 +0100 Subject: [PATCH 3/3] Add ignoreLineEndingDifferences Stop tests from failing on non-Windows OSs due to different line endings. --- .../Authentication.Core/test/AuthenticationPropertiesTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Http/Authentication.Core/test/AuthenticationPropertiesTests.cs b/src/Http/Authentication.Core/test/AuthenticationPropertiesTests.cs index 37a83e35e45c..f69c988fd362 100644 --- a/src/Http/Authentication.Core/test/AuthenticationPropertiesTests.cs +++ b/src/Http/Authentication.Core/test/AuthenticationPropertiesTests.cs @@ -386,7 +386,7 @@ public void Serialization_Is_Minimised_With_SystemTextJson() "".redirect"": ""/foo/bar"", ""foo"": ""bar"" } -}", json); +}", json, ignoreLineEndingDifferences: true); } public class MyAuthenticationProperties : AuthenticationProperties