Introduce OpenIdAuthenticationMessage.GetParameters() and make Parameters protected

kevinchalet · kevinchalet · commit a1936973ad02 · 2018-03-07T19:12:38.000+01:00
diff --git a/src/AspNet.Security.OpenId.Steam/SteamAuthenticationHandler.cs b/src/AspNet.Security.OpenId.Steam/SteamAuthenticationHandler.cs
@@ -22,13 +22,18 @@ namespace AspNet.Security.OpenId.Steam
 {
     public class SteamAuthenticationHandler : OpenIdAuthenticationHandler<SteamAuthenticationOptions>
     {
-        public SteamAuthenticationHandler(IOptionsMonitor<SteamAuthenticationOptions> options, ILoggerFactory logger, UrlEncoder encoder, ISystemClock clock)
+        public SteamAuthenticationHandler(
+            [NotNull] IOptionsMonitor<SteamAuthenticationOptions> options,
+            [NotNull] ILoggerFactory logger,
+            [NotNull] UrlEncoder encoder,
+            [NotNull] ISystemClock clock)
             : base(options, logger, encoder, clock)
-        { }
+        {
+        }
 
         protected override async Task<AuthenticationTicket> CreateTicketAsync(
             [NotNull] ClaimsIdentity identity, [NotNull] AuthenticationProperties properties,
-            [NotNull] string identifier, [NotNull] IDictionary<string, string> attributes)
+            [NotNull] string identifier, [NotNull] IReadOnlyDictionary<string, string> attributes)
         {
             var principal = new ClaimsPrincipal(identity);
             var ticket = new AuthenticationTicket(principal, properties, Scheme.Name);
diff --git a/src/AspNet.Security.OpenId/OpenIdAuthenticationHandler.cs b/src/AspNet.Security.OpenId/OpenIdAuthenticationHandler.cs
@@ -27,7 +27,9 @@ public OpenIdAuthenticationHandler(
             [NotNull] ILoggerFactory logger,
             [NotNull] UrlEncoder encoder,
             [NotNull] ISystemClock clock)
-            : base(options, logger, encoder, clock) { }
+            : base(options, logger, encoder, clock)
+        {
+        }
     }
 
     public class OpenIdAuthenticationHandler<TOptions> : RemoteAuthenticationHandler<TOptions>
@@ -38,7 +40,9 @@ public OpenIdAuthenticationHandler(
             [NotNull] ILoggerFactory logger,
             [NotNull] UrlEncoder encoder,
             [NotNull] ISystemClock clock)
-            : base(options, logger, encoder, clock) { }
+            : base(options, logger, encoder, clock)
+        {
+        }
 
         protected override async Task<HandleRequestResult> HandleRemoteAuthenticateAsync()
         {
@@ -145,7 +149,7 @@ protected override async Task<HandleRequestResult> HandleRemoteAuthenticateAsync
                                                       name: OpenIdAuthenticationConstants.Parameters.State, value: state);
 
             // Validate the return_to parameter by comparing it to the address stored in the properties.
-            // See http://openid.net/specs/openid-authentication-2_0.html#verify_return_to
+            // See http://openid.net/specs/openid-authentication-2_0.html#verify_return_to for more information.
             if (!string.Equals(message.ReturnTo, address, StringComparison.Ordinal))
             {
                 return HandleRequestResult.Fail("The authentication response was rejected because the return_to parameter was invalid.");
@@ -216,7 +220,7 @@ protected override async Task<HandleRequestResult> HandleRemoteAuthenticateAsync
 
         protected virtual async Task<AuthenticationTicket> CreateTicketAsync(
             [NotNull] ClaimsIdentity identity, [NotNull] AuthenticationProperties properties,
-            [NotNull] string identifier, [NotNull] IDictionary<string, string> attributes)
+            [NotNull] string identifier, [NotNull] IReadOnlyDictionary<string, string> attributes)
         {
             var principal = new ClaimsPrincipal(identity);
             var ticket = new AuthenticationTicket(principal, properties, Scheme.Name);
@@ -319,7 +323,10 @@ protected override async Task HandleChallengeAsync(AuthenticationProperties prop
                     value: string.Join(",", Options.Attributes.Select(attribute => attribute.Key)));
             }
 
-            var address = QueryHelpers.AddQueryString(configuration.AuthenticationEndpoint, message.Parameters);
+            var address = QueryHelpers.AddQueryString(configuration.AuthenticationEndpoint, 
+                message.GetParameters()
+                    .ToDictionary(parameter => parameter.Key,
+                                  parameter => parameter.Value));
 
             Response.Redirect(address);
         }
@@ -349,7 +356,7 @@ private async Task<bool> VerifyAssertionAsync([NotNull] OpenIdAuthenticationMess
             };
 
             // Copy the parameters extracted from the assertion.
-            foreach (var parameter in message.Parameters)
+            foreach (var parameter in message.GetParameters())
             {
                 if (string.Equals(parameter.Key, $"{OpenIdAuthenticationConstants.Prefixes.OpenId}." +
                                                     OpenIdAuthenticationConstants.Parameters.Mode, StringComparison.Ordinal))
diff --git a/src/AspNet.Security.OpenId/OpenIdAuthenticationMessage.cs b/src/AspNet.Security.OpenId/OpenIdAuthenticationMessage.cs
@@ -6,7 +6,7 @@
 
 using System;
 using System.Collections.Generic;
-using System.Linq;
+using System.Collections.ObjectModel;
 using JetBrains.Annotations;
 using Microsoft.Extensions.Primitives;
 
@@ -20,10 +20,7 @@ public class OpenIdAuthenticationMessage
         /// <summary>
         /// Initializes a new OpenID message.
         /// </summary>
-        public OpenIdAuthenticationMessage()
-            : this(new Dictionary<string, string>())
-        {
-        }
+        public OpenIdAuthenticationMessage() { }
 
         /// <summary>
         /// Initializes a new OpenID message.
@@ -36,7 +33,15 @@ public OpenIdAuthenticationMessage([NotNull] IDictionary<string, string> paramet
                 throw new ArgumentNullException(nameof(parameters));
             }
 
-            Parameters = parameters;
+            foreach (var parameter in parameters)
+            {
+                if (string.IsNullOrEmpty(parameter.Key))
+                {
+                    continue;
+                }
+
+                Parameters.Add(parameter.Key, parameter.Value);
+            }
         }
 
         /// <summary>
@@ -50,92 +55,134 @@ public OpenIdAuthenticationMessage([NotNull] IEnumerable<KeyValuePair<string, St
                 throw new ArgumentNullException(nameof(parameters));
             }
 
-            Parameters = parameters.ToDictionary(
-                parameter => parameter.Key,
-                parameter => (string) parameter.Value);
+            foreach (var parameter in parameters)
+            {
+                if (string.IsNullOrEmpty(parameter.Key))
+                {
+                    continue;
+                }
+
+                Parameters.Add(parameter.Key, parameter.Value);
+            }
         }
 
         /// <summary>
         /// Gets or sets the openid.claimed_id property.
         /// </summary>
         public string ClaimedIdentifier
         {
-            get { return GetParameter(OpenIdAuthenticationConstants.Parameters.ClaimedId); }
-            set { SetParameter(OpenIdAuthenticationConstants.Parameters.ClaimedId, value); }
+            get => GetParameter(OpenIdAuthenticationConstants.Parameters.ClaimedId);
+            set => SetParameter(OpenIdAuthenticationConstants.Parameters.ClaimedId, value);
         }
 
         /// <summary>
         /// Gets or sets the openid.identity property.
         /// </summary>
         public string Identity
         {
-            get { return GetParameter(OpenIdAuthenticationConstants.Parameters.Identity); }
-            set { SetParameter(OpenIdAuthenticationConstants.Parameters.Identity, value); }
+            get => GetParameter(OpenIdAuthenticationConstants.Parameters.Identity);
+            set => SetParameter(OpenIdAuthenticationConstants.Parameters.Identity, value);
         }
 
         /// <summary>
         /// Gets or sets the openid.error property.
         /// </summary>
         public string Error
         {
-            get { return GetParameter(OpenIdAuthenticationConstants.Parameters.Error); }
-            set { SetParameter(OpenIdAuthenticationConstants.Parameters.Error, value); }
+            get => GetParameter(OpenIdAuthenticationConstants.Parameters.Error);
+            set => SetParameter(OpenIdAuthenticationConstants.Parameters.Error, value);
         }
 
         /// <summary>
         /// Gets or sets the openid.mode property.
         /// </summary>
         public string Mode
         {
-            get { return GetParameter(OpenIdAuthenticationConstants.Parameters.Mode); }
-            set { SetParameter(OpenIdAuthenticationConstants.Parameters.Mode, value); }
+            get => GetParameter(OpenIdAuthenticationConstants.Parameters.Mode);
+            set => SetParameter(OpenIdAuthenticationConstants.Parameters.Mode, value);
         }
 
         /// <summary>
         /// Gets or sets the openid.ns property.
         /// </summary>
         public string Namespace
         {
-            get { return GetParameter(OpenIdAuthenticationConstants.Parameters.Namespace); }
-            set { SetParameter(OpenIdAuthenticationConstants.Parameters.Namespace, value); }
+            get => GetParameter(OpenIdAuthenticationConstants.Parameters.Namespace);
+            set => SetParameter(OpenIdAuthenticationConstants.Parameters.Namespace, value);
         }
 
         /// <summary>
         /// Gets or sets the openid.realm property.
         /// </summary>
         public string Realm
         {
-            get { return GetParameter(OpenIdAuthenticationConstants.Parameters.Realm); }
-            set { SetParameter(OpenIdAuthenticationConstants.Parameters.Realm, value); }
+            get => GetParameter(OpenIdAuthenticationConstants.Parameters.Realm);
+            set => SetParameter(OpenIdAuthenticationConstants.Parameters.Realm, value);
         }
 
         /// <summary>
         /// Gets or sets the openid.return_to property.
         /// </summary>
         public string ReturnTo
         {
-            get { return GetParameter(OpenIdAuthenticationConstants.Parameters.ReturnTo); }
-            set { SetParameter(OpenIdAuthenticationConstants.Parameters.ReturnTo, value); }
+            get => GetParameter(OpenIdAuthenticationConstants.Parameters.ReturnTo);
+            set => SetParameter(OpenIdAuthenticationConstants.Parameters.ReturnTo, value);
         }
 
         /// <summary>
         /// Gets the parameters associated with this OpenID message.
         /// </summary>
-        public IDictionary<string, string> Parameters { get; }
+        protected IDictionary<string, string> Parameters { get; } =
+            new Dictionary<string, string>(StringComparer.Ordinal);
+
+        /// <summary>
+        /// Adds a parameter using the default prefix.
+        /// </summary>
+        /// <param name="name">The parameter name.</param>
+        /// <param name="value">The parameter value.</param>
+        /// <returns>The current instance, which allows chaining calls.</returns>
+        public OpenIdAuthenticationMessage AddParameter([NotNull] string name, [CanBeNull] string value)
+            => AddParameter(OpenIdAuthenticationConstants.Prefixes.OpenId, name, value);
+
+        /// <summary>
+        /// Adds a parameter using the specified prefix.
+        /// </summary>
+        /// <param name="prefix">The prefix used to discriminate the parameter.</param>
+        /// <param name="name">The parameter to store.</param>
+        /// <param name="value">The value associated with the parameter.</param>
+        /// <returns>The current instance, which allows chaining calls.</returns>
+        public OpenIdAuthenticationMessage AddParameter([NotNull] string prefix, [NotNull] string name, [CanBeNull] string value)
+        {
+            if (string.IsNullOrEmpty(prefix))
+            {
+                throw new ArgumentException("The prefix cannot be null or empty.", nameof(prefix));
+            }
+
+            if (string.IsNullOrEmpty(name))
+            {
+                throw new ArgumentException("The parameter name cannot be null or empty.", nameof(name));
+            }
+
+            if (!Parameters.ContainsKey($"{prefix}.{name}"))
+            {
+                Parameters.Add($"{prefix}.{name}", value);
+            }
+
+            return this;
+        }
 
         /// <summary>
         /// Gets the attributes returned by the identity provider, or an empty
         /// dictionary if the message doesn't expose an attribute exchange alias.
         /// </summary>
         /// <returns>The attributes contained in this message.</returns>
-        public IDictionary<string, string> GetAttributes()
+        public IReadOnlyDictionary<string, string> GetAttributes()
         {
             var attributes = new Dictionary<string, string>(StringComparer.Ordinal);
 
-            string alias;
-            var extensions = GetExtensions();
             // If the ax alias cannot be found, return an empty dictionary.
-            if (!extensions.TryGetValue(OpenIdAuthenticationConstants.Namespaces.Ax, out alias))
+            var extensions = GetExtensions();
+            if (!extensions.TryGetValue(OpenIdAuthenticationConstants.Namespaces.Ax, out string alias))
             {
                 return attributes;
             }
@@ -165,9 +212,8 @@ public IDictionary<string, string> GetAttributes()
                 }
 
                 // Exclude attributes whose value is missing.
-                string value;
                 if (!Parameters.TryGetValue($"{OpenIdAuthenticationConstants.Prefixes.OpenId}.{alias}." +
-                                            $"{OpenIdAuthenticationConstants.Suffixes.Value}.{name}", out value))
+                                            $"{OpenIdAuthenticationConstants.Suffixes.Value}.{name}", out string value))
                 {
                     continue;
                 }
@@ -188,7 +234,7 @@ public IDictionary<string, string> GetAttributes()
         /// Gets the extensions and their corresponding alias.
         /// </summary>
         /// <returns>The extensions contained in this message.</returns>
-        public IDictionary<string, string> GetExtensions()
+        public IReadOnlyDictionary<string, string> GetExtensions()
         {
             var extensions = new Dictionary<string, string>(StringComparer.Ordinal);
 
@@ -212,9 +258,7 @@ public IDictionary<string, string> GetExtensions()
         /// <param name="name">The parameter to retrieve.</param>
         /// <returns>The value extracted from the parameter.</returns>
         public string GetParameter([NotNull] string name)
-        {
-            return GetParameter(OpenIdAuthenticationConstants.Prefixes.OpenId, name);
-        }
+            => GetParameter(OpenIdAuthenticationConstants.Prefixes.OpenId, name);
 
         /// <summary>
         /// Gets the parameter corresponding to the requested name and the given
@@ -235,25 +279,30 @@ public string GetParameter([NotNull] string prefix, [NotNull] string name)
                 throw new ArgumentNullException(nameof(name));
             }
 
-            string value;
-            if (Parameters.TryGetValue($"{prefix}.{name}", out value))
+            if (Parameters.TryGetValue($"{prefix}.{name}", out string value))
             {
                 return value;
             }
 
             return null;
         }
 
+        /// <summary>
+        /// Gets all the parameters associated with this instance.
+        /// </summary>
+        /// <returns>The parameters associated with this instance.</returns>
+        public IReadOnlyDictionary<string, string> GetParameters()
+            => new ReadOnlyDictionary<string, string>(Parameters);
+
         /// <summary>
         /// Adds, replaces or removes the parameter corresponding
         /// to the requested name and the default prefix.
         /// </summary>
         /// <param name="name">The parameter to store.</param>
         /// <param name="value">The value associated with the parameter.</param>
-        public void SetParameter([NotNull] string name, [CanBeNull] string value)
-        {
-            SetParameter(OpenIdAuthenticationConstants.Prefixes.OpenId, name, value);
-        }
+        /// <returns>The current instance, which allows chaining calls.</returns>
+        public OpenIdAuthenticationMessage SetParameter([NotNull] string name, [CanBeNull] string value)
+            => SetParameter(OpenIdAuthenticationConstants.Prefixes.OpenId, name, value);
 
         /// <summary>
         /// Adds, replaces or removes the parameter corresponding
@@ -262,28 +311,32 @@ public void SetParameter([NotNull] string name, [CanBeNull] string value)
         /// <param name="prefix">The prefix used to discriminate the parameter.</param>
         /// <param name="name">The parameter to store.</param>
         /// <param name="value">The value associated with the parameter.</param>
-        public void SetParameter([NotNull] string prefix, [NotNull] string name, [CanBeNull] string value)
+        /// <returns>The current instance, which allows chaining calls.</returns>
+        public OpenIdAuthenticationMessage SetParameter([NotNull] string prefix, [NotNull] string name, [CanBeNull] string value)
         {
             if (string.IsNullOrEmpty(prefix))
             {
-                throw new ArgumentNullException(nameof(prefix));
+                throw new ArgumentException("The prefix cannot be null or empty.", nameof(prefix));
             }
 
             if (string.IsNullOrEmpty(name))
             {
-                throw new ArgumentNullException(nameof(name));
+                throw new ArgumentException("The parameter name cannot be null or empty.", nameof(name));
             }
 
             // If the parameter value is null, remove
             // it from the parameters dictionary.
             if (string.IsNullOrEmpty(value))
             {
                 Parameters.Remove($"{prefix}.{name}");
+            }
 
-                return;
+            else
+            {
+                Parameters[$"{prefix}.{name}"] = value;
             }
 
-            Parameters[$"{prefix}.{name}"] = value;
+            return this;
         }
     }
 }
