From 383b7d474625877910e299489ccce99390807fa8 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Thu, 18 May 2023 13:13:22 +0800 Subject: [PATCH 1/5] Add debugging attributes to System.Security.Claims types --- .../ref/System.Security.Claims.cs | 10 ++++--- .../System/Security/Claims/ClaimsIdentity.cs | 14 ++++++++++ .../System/Security/Claims/ClaimsPrincipal.cs | 26 +++++++++++++++++++ 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Security.Claims/ref/System.Security.Claims.cs b/src/libraries/System.Security.Claims/ref/System.Security.Claims.cs index 4bc8d68fd834a5..fb26e65ff1eb26 100644 --- a/src/libraries/System.Security.Claims/ref/System.Security.Claims.cs +++ b/src/libraries/System.Security.Claims/ref/System.Security.Claims.cs @@ -4,6 +4,8 @@ // Changes to this file must follow the https://aka.ms/api-review process. // ------------------------------------------------------------------------------ +using System.Diagnostics; + namespace System.Security.Claims { public partial class Claim @@ -31,6 +33,7 @@ public Claim(string type, string value, string? valueType, string? issuer, strin public virtual void WriteTo(System.IO.BinaryWriter writer) { } protected virtual void WriteTo(System.IO.BinaryWriter writer, byte[]? userData) { } } + [DebuggerDisplay("{DebuggerToString(),nq}")] public partial class ClaimsIdentity : System.Security.Principal.IIdentity { public const string DefaultIssuer = "LOCAL AUTHORITY"; @@ -41,11 +44,11 @@ public ClaimsIdentity(System.Collections.Generic.IEnumerable? claims, string? authenticationType) { } public ClaimsIdentity(System.Collections.Generic.IEnumerable? claims, string? authenticationType, string? nameType, string? roleType) { } public ClaimsIdentity(System.IO.BinaryReader reader) { } - [System.ObsoleteAttribute("This API supports obsolete formatter-based serialization. It should not be called or extended by application code.", DiagnosticId = "SYSLIB0051", UrlFormat = "https://aka.ms/dotnet-warnings/{0}")] [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)] + [System.ObsoleteAttribute("This API supports obsolete formatter-based serialization. It should not be called or extended by application code.", DiagnosticId="SYSLIB0051", UrlFormat="https://aka.ms/dotnet-warnings/{0}")] protected ClaimsIdentity(System.Runtime.Serialization.SerializationInfo info) { } - [System.ObsoleteAttribute("This API supports obsolete formatter-based serialization. It should not be called or extended by application code.", DiagnosticId = "SYSLIB0051", UrlFormat = "https://aka.ms/dotnet-warnings/{0}")] [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)] + [System.ObsoleteAttribute("This API supports obsolete formatter-based serialization. It should not be called or extended by application code.", DiagnosticId="SYSLIB0051", UrlFormat="https://aka.ms/dotnet-warnings/{0}")] protected ClaimsIdentity(System.Runtime.Serialization.SerializationInfo info, System.Runtime.Serialization.StreamingContext context) { } protected ClaimsIdentity(System.Security.Claims.ClaimsIdentity other) { } public ClaimsIdentity(System.Security.Principal.IIdentity? identity) { } @@ -79,13 +82,14 @@ public virtual void RemoveClaim(System.Security.Claims.Claim? claim) { } public virtual void WriteTo(System.IO.BinaryWriter writer) { } protected virtual void WriteTo(System.IO.BinaryWriter writer, byte[]? userData) { } } + [DebuggerDisplay("{DebuggerToString(),nq}")] public partial class ClaimsPrincipal : System.Security.Principal.IPrincipal { public ClaimsPrincipal() { } public ClaimsPrincipal(System.Collections.Generic.IEnumerable identities) { } public ClaimsPrincipal(System.IO.BinaryReader reader) { } - [System.ObsoleteAttribute("This API supports obsolete formatter-based serialization. It should not be called or extended by application code.", DiagnosticId = "SYSLIB0051", UrlFormat = "https://aka.ms/dotnet-warnings/{0}")] [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)] + [System.ObsoleteAttribute("This API supports obsolete formatter-based serialization. It should not be called or extended by application code.", DiagnosticId="SYSLIB0051", UrlFormat="https://aka.ms/dotnet-warnings/{0}")] protected ClaimsPrincipal(System.Runtime.Serialization.SerializationInfo info, System.Runtime.Serialization.StreamingContext context) { } public ClaimsPrincipal(System.Security.Principal.IIdentity identity) { } public ClaimsPrincipal(System.Security.Principal.IPrincipal principal) { } diff --git a/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsIdentity.cs b/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsIdentity.cs index 9733ca86ff10b9..bf3b40d370a055 100644 --- a/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsIdentity.cs +++ b/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsIdentity.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.ComponentModel; +using System.Diagnostics; using System.IO; using System.Runtime.Serialization; using System.Security.Principal; @@ -12,6 +13,7 @@ namespace System.Security.Claims /// /// An Identity that is represented by a set of claims. /// + [DebuggerDisplay("{DebuggerToString(),nq}")] public class ClaimsIdentity : IIdentity { private enum SerializationMask @@ -933,5 +935,17 @@ protected virtual void GetObjectData(SerializationInfo info, StreamingContext co { throw new PlatformNotSupportedException(); } + + internal string DebuggerToString() + { + // DebuggerDisplayAttribute is inherited. Use virtual members instead of private fields to gather data. + int claimsCount = 0; + foreach (var item in Claims) + { + claimsCount++; + } + + return $"Identity Name = {Name}, IsAuthenticated = {IsAuthenticated}, Claims Count = {claimsCount}"; + } } } diff --git a/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsPrincipal.cs b/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsPrincipal.cs index a423bfa4ce4712..83aa39f7c9529f 100644 --- a/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsPrincipal.cs +++ b/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsPrincipal.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.ComponentModel; +using System.Diagnostics; using System.IO; using System.Runtime.Serialization; using System.Security.Principal; @@ -13,6 +14,7 @@ namespace System.Security.Claims /// /// Concrete IPrincipal supporting multiple claims-based identities /// + [DebuggerDisplay("{DebuggerToString(),nq}")] public class ClaimsPrincipal : IPrincipal { private enum SerializationMask @@ -567,5 +569,29 @@ protected virtual void GetObjectData(SerializationInfo info, StreamingContext co { throw new PlatformNotSupportedException(); } + + internal string DebuggerToString() + { + // DebuggerDisplayAttribute is inherited. Use virtual members instead of private fields to gather data. + int identitiesCount = 0; + foreach (var items in Identities) + { + identitiesCount++; + } + + int claimsCount = 0; + foreach (var item in Claims) + { + claimsCount++; + } + + // Return debug string optimized for the case of one identity. + if (identitiesCount == 1 && Identity is ClaimsIdentity claimsIdentity) + { + return claimsIdentity.DebuggerToString(); + } + + return $"Principal Identities Count: {identitiesCount}, Claims Count: {claimsCount}"; + } } } From 3da55f58fcfcfa119750441490325965399d0e7e Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Thu, 18 May 2023 13:38:29 +0800 Subject: [PATCH 2/5] Update --- .../src/System/Security/Claims/ClaimsIdentity.cs | 2 +- .../src/System/Security/Claims/ClaimsPrincipal.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsIdentity.cs b/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsIdentity.cs index bf3b40d370a055..c19d88ea106363 100644 --- a/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsIdentity.cs +++ b/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsIdentity.cs @@ -945,7 +945,7 @@ internal string DebuggerToString() claimsCount++; } - return $"Identity Name = {Name}, IsAuthenticated = {IsAuthenticated}, Claims Count = {claimsCount}"; + return $"Identity Name = {Name ?? "(null)"}, IsAuthenticated = {IsAuthenticated}, Claims Count = {claimsCount}"; } } } diff --git a/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsPrincipal.cs b/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsPrincipal.cs index 83aa39f7c9529f..0efe2dfa592cf9 100644 --- a/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsPrincipal.cs +++ b/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsPrincipal.cs @@ -588,7 +588,7 @@ internal string DebuggerToString() // Return debug string optimized for the case of one identity. if (identitiesCount == 1 && Identity is ClaimsIdentity claimsIdentity) { - return claimsIdentity.DebuggerToString(); + return $"Principal {claimsIdentity.DebuggerToString()}"; } return $"Principal Identities Count: {identitiesCount}, Claims Count: {claimsCount}"; From e9fb812d6427c8005efc097bde16a0a75889b3d2 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Thu, 18 May 2023 21:38:52 +0800 Subject: [PATCH 3/5] Update ref, add type proxies --- .../ref/System.Security.Claims.cs | 4 -- .../System/Security/Claims/ClaimsIdentity.cs | 32 +++++++++++++++ .../System/Security/Claims/ClaimsPrincipal.cs | 39 +++++++++++++++++++ 3 files changed, 71 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Security.Claims/ref/System.Security.Claims.cs b/src/libraries/System.Security.Claims/ref/System.Security.Claims.cs index fb26e65ff1eb26..2d3b6bd25df300 100644 --- a/src/libraries/System.Security.Claims/ref/System.Security.Claims.cs +++ b/src/libraries/System.Security.Claims/ref/System.Security.Claims.cs @@ -4,8 +4,6 @@ // Changes to this file must follow the https://aka.ms/api-review process. // ------------------------------------------------------------------------------ -using System.Diagnostics; - namespace System.Security.Claims { public partial class Claim @@ -33,7 +31,6 @@ public Claim(string type, string value, string? valueType, string? issuer, strin public virtual void WriteTo(System.IO.BinaryWriter writer) { } protected virtual void WriteTo(System.IO.BinaryWriter writer, byte[]? userData) { } } - [DebuggerDisplay("{DebuggerToString(),nq}")] public partial class ClaimsIdentity : System.Security.Principal.IIdentity { public const string DefaultIssuer = "LOCAL AUTHORITY"; @@ -82,7 +79,6 @@ public virtual void RemoveClaim(System.Security.Claims.Claim? claim) { } public virtual void WriteTo(System.IO.BinaryWriter writer) { } protected virtual void WriteTo(System.IO.BinaryWriter writer, byte[]? userData) { } } - [DebuggerDisplay("{DebuggerToString(),nq}")] public partial class ClaimsPrincipal : System.Security.Principal.IPrincipal { public ClaimsPrincipal() { } diff --git a/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsIdentity.cs b/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsIdentity.cs index c19d88ea106363..075a55cfee7a30 100644 --- a/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsIdentity.cs +++ b/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsIdentity.cs @@ -14,6 +14,7 @@ namespace System.Security.Claims /// An Identity that is represented by a set of claims. /// [DebuggerDisplay("{DebuggerToString(),nq}")] + [DebuggerTypeProxy(typeof(ClaimsIdentityDebugProxy))] public class ClaimsIdentity : IIdentity { private enum SerializationMask @@ -947,5 +948,36 @@ internal string DebuggerToString() return $"Identity Name = {Name ?? "(null)"}, IsAuthenticated = {IsAuthenticated}, Claims Count = {claimsCount}"; } + + private sealed class ClaimsIdentityDebugProxy + { + private readonly ClaimsIdentity _identity; + + public ClaimsIdentityDebugProxy(ClaimsIdentity identity) + { + _identity = identity; + } + + public ClaimsIdentity? Actor => _identity.Actor; + public string? AuthenticationType => _identity.AuthenticationType; + public object? BootstrapContext => _identity.BootstrapContext; + public IEnumerable Claims + { + get + { + List claims = new List(); + foreach (var c in _identity.Claims) + { + claims.Add(c); + } + return claims; + } + } + public bool IsAuthenticated => _identity.IsAuthenticated; + public string? Label => _identity.Label; + public string? Name => _identity.Name; + public string NameClaimType => _identity.NameClaimType; + public string RoleClaimType => _identity.RoleClaimType; + } } } diff --git a/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsPrincipal.cs b/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsPrincipal.cs index 0efe2dfa592cf9..3b0de846d25c78 100644 --- a/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsPrincipal.cs +++ b/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsPrincipal.cs @@ -15,6 +15,7 @@ namespace System.Security.Claims /// Concrete IPrincipal supporting multiple claims-based identities /// [DebuggerDisplay("{DebuggerToString(),nq}")] + [DebuggerTypeProxy(typeof(ClaimsPrincipalDebugProxy))] public class ClaimsPrincipal : IPrincipal { private enum SerializationMask @@ -593,5 +594,43 @@ internal string DebuggerToString() return $"Principal Identities Count: {identitiesCount}, Claims Count: {claimsCount}"; } + + private sealed class ClaimsPrincipalDebugProxy + { + private readonly ClaimsPrincipal _principal; + + public ClaimsPrincipalDebugProxy(ClaimsPrincipal principal) + { + _principal = principal; + } + + public IEnumerable Claims + { + get + { + List claims = new List(); + foreach (var c in _principal.Claims) + { + claims.Add(c); + } + return claims; + } + } + + public IEnumerable Identities + { + get + { + List identities = new List(); + foreach (var i in _principal.Identities) + { + identities.Add(i); + } + return identities; + } + } + + public IIdentity? Identity => _principal.Identity; + } } } From b50b4656971c35ded8ba8915810a888e66275f02 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Thu, 18 May 2023 21:49:48 +0800 Subject: [PATCH 4/5] PR feedback --- .../System/Security/Claims/ClaimsIdentity.cs | 15 ++------ .../System/Security/Claims/ClaimsPrincipal.cs | 34 +++---------------- 2 files changed, 7 insertions(+), 42 deletions(-) diff --git a/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsIdentity.cs b/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsIdentity.cs index 075a55cfee7a30..fab9e66b51b879 100644 --- a/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsIdentity.cs +++ b/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsIdentity.cs @@ -941,7 +941,7 @@ internal string DebuggerToString() { // DebuggerDisplayAttribute is inherited. Use virtual members instead of private fields to gather data. int claimsCount = 0; - foreach (var item in Claims) + foreach (Claim item in Claims) { claimsCount++; } @@ -961,18 +961,7 @@ public ClaimsIdentityDebugProxy(ClaimsIdentity identity) public ClaimsIdentity? Actor => _identity.Actor; public string? AuthenticationType => _identity.AuthenticationType; public object? BootstrapContext => _identity.BootstrapContext; - public IEnumerable Claims - { - get - { - List claims = new List(); - foreach (var c in _identity.Claims) - { - claims.Add(c); - } - return claims; - } - } + public IEnumerable Claims => new List(_identity.Claims); public bool IsAuthenticated => _identity.IsAuthenticated; public string? Label => _identity.Label; public string? Name => _identity.Name; diff --git a/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsPrincipal.cs b/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsPrincipal.cs index 3b0de846d25c78..db4fe864cc43c9 100644 --- a/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsPrincipal.cs +++ b/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsPrincipal.cs @@ -571,17 +571,17 @@ protected virtual void GetObjectData(SerializationInfo info, StreamingContext co throw new PlatformNotSupportedException(); } - internal string DebuggerToString() + private string DebuggerToString() { // DebuggerDisplayAttribute is inherited. Use virtual members instead of private fields to gather data. int identitiesCount = 0; - foreach (var items in Identities) + foreach (ClaimsIdentity items in Identities) { identitiesCount++; } int claimsCount = 0; - foreach (var item in Claims) + foreach (Claim item in Claims) { claimsCount++; } @@ -604,32 +604,8 @@ public ClaimsPrincipalDebugProxy(ClaimsPrincipal principal) _principal = principal; } - public IEnumerable Claims - { - get - { - List claims = new List(); - foreach (var c in _principal.Claims) - { - claims.Add(c); - } - return claims; - } - } - - public IEnumerable Identities - { - get - { - List identities = new List(); - foreach (var i in _principal.Identities) - { - identities.Add(i); - } - return identities; - } - } - + public IEnumerable Claims => new List(_principal.Claims); + public IEnumerable Identities => new List(_principal.Identities); public IIdentity? Identity => _principal.Identity; } } From ac2fdb2fa0cd1a8ec6d261d9e83aa65510dcea8a Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Thu, 18 May 2023 22:01:35 +0800 Subject: [PATCH 5/5] PR feedback --- .../src/System/Security/Claims/ClaimsIdentity.cs | 3 ++- .../src/System/Security/Claims/ClaimsPrincipal.cs | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsIdentity.cs b/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsIdentity.cs index fab9e66b51b879..c928dd88501eba 100644 --- a/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsIdentity.cs +++ b/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsIdentity.cs @@ -961,7 +961,8 @@ public ClaimsIdentityDebugProxy(ClaimsIdentity identity) public ClaimsIdentity? Actor => _identity.Actor; public string? AuthenticationType => _identity.AuthenticationType; public object? BootstrapContext => _identity.BootstrapContext; - public IEnumerable Claims => new List(_identity.Claims); + // List type has a friendly debugger view + public List Claims => new List(_identity.Claims); public bool IsAuthenticated => _identity.IsAuthenticated; public string? Label => _identity.Label; public string? Name => _identity.Name; diff --git a/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsPrincipal.cs b/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsPrincipal.cs index db4fe864cc43c9..1e1efd8eba6268 100644 --- a/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsPrincipal.cs +++ b/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsPrincipal.cs @@ -604,8 +604,9 @@ public ClaimsPrincipalDebugProxy(ClaimsPrincipal principal) _principal = principal; } - public IEnumerable Claims => new List(_principal.Claims); - public IEnumerable Identities => new List(_principal.Identities); + // List type has a friendly debugger view + public List Claims => new List(_principal.Claims); + public List Identities => new List(_principal.Identities); public IIdentity? Identity => _principal.Identity; } }