Allows the authorization server to store merely the hashes of client secrets.

Fixes #92

7 files changed, 152 insertions, 76 deletions

diff --git a/projecttemplates/RelyingPartyLogic/Model.Client.cs b/projecttemplates/RelyingPartyLogic/Model.Client.cs
index a2d895e..472db1e 100644
--- a/projecttemplates/RelyingPartyLogic/Model.Client.cs
+++ b/projecttemplates/RelyingPartyLogic/Model.Client.cs
@@ -7,20 +7,13 @@
 namespace RelyingPartyLogic {
 	using System;
 	using System.Collections.Generic;
-
+	using DotNetOpenAuth.Messaging;
 	using DotNetOpenAuth.OAuth2;
 
 	public partial class Client : IClientDescription {
 		#region IConsumerDescription Members
 
 		/// <summary>
-		/// Gets the client secret.
-		/// </summary>
-		string IClientDescription.Secret {
-			get { return this.ClientSecret; }
-		}
-
-		/// <summary>
 		/// Gets the callback to use when an individual authorization request
 		/// does not include an explicit callback URI.
 		/// </summary>
@@ -39,6 +32,26 @@ namespace RelyingPartyLogic {
 		}
 
 		/// <summary>
+		/// Gets a value indicating whether a non-empty secret is registered for this client.
+		/// </summary>
+		bool IClientDescription.HasNonEmptySecret {
+			get { return !string.IsNullOrEmpty(this.ClientSecret); }
+		}
+
+		/// <summary>
+		/// Checks whether the specified client secret is correct.
+		/// </summary>
+		/// <param name="secret">The secret obtained from the client.</param>
+		/// <returns><c>true</c> if the secret matches the one in the authorization server's record for the client; <c>false</c> otherwise.</returns>
+		/// <remarks>
+		/// All string equality checks, whether checking secrets or their hashes,
+		/// should be done using <see cref="MessagingUtilites.EqualsConstantTime"/> to mitigate timing attacks.
+		/// </remarks>
+		bool IClientDescription.IsValidClientSecret(string secret) {
+			return MessagingUtilities.EqualsConstantTime(secret, this.ClientSecret);
+		}
+
+		/// <summary>
 		/// Determines whether a callback URI included in a client's authorization request
 		/// is among those allowed callbacks for the registered client.
 		/// </summary>
diff --git a/samples/OAuthAuthorizationServer/Code/Client.cs b/samples/OAuthAuthorizationServer/Code/Client.cs
index 0013f27..7495869 100644
--- a/samples/OAuthAuthorizationServer/Code/Client.cs
+++ b/samples/OAuthAuthorizationServer/Code/Client.cs
@@ -1,7 +1,7 @@
 namespace OAuthAuthorizationServer.Code {
 	using System;
 	using System.Collections.Generic;
-
+	using DotNetOpenAuth.Messaging;
 	using DotNetOpenAuth.OAuth2;
 
 	/// <summary>
@@ -11,13 +11,6 @@
 		#region IConsumerDescription Members
 
 		/// <summary>
-		/// Gets the client secret.
-		/// </summary>
-		string IClientDescription.Secret {
-			get { return this.ClientSecret; }
-		}
-
-		/// <summary>
 		/// Gets the callback to use when an individual authorization request
 		/// does not include an explicit callback URI.
 		/// </summary>
@@ -36,6 +29,13 @@
 		}
 
 		/// <summary>
+		/// Gets a value indicating whether a non-empty secret is registered for this client.
+		/// </summary>
+		bool IClientDescription.HasNonEmptySecret {
+			get { return !string.IsNullOrEmpty(this.ClientSecret); }
+		}
+
+		/// <summary>
 		/// Determines whether a callback URI included in a client's authorization request
 		/// is among those allowed callbacks for the registered client.
 		/// </summary>
@@ -59,6 +59,19 @@
 			return false;
 		}
 
+		/// <summary>
+		/// Checks whether the specified client secret is correct.
+		/// </summary>
+		/// <param name="secret">The secret obtained from the client.</param>
+		/// <returns><c>true</c> if the secret matches the one in the authorization server's record for the client; <c>false</c> otherwise.</returns>
+		/// <remarks>
+		/// All string equality checks, whether checking secrets or their hashes,
+		/// should be done using <see cref="MessagingUtilites.EqualsConstantTime"/> to mitigate timing attacks.
+		/// </remarks>
+		bool IClientDescription.IsValidClientSecret(string secret) {
+			return MessagingUtilities.EqualsConstantTime(secret, this.ClientSecret);
+		}
+
 		#endregion
 	}
 }
\ No newline at end of file
diff --git a/src/DotNetOpenAuth.Core/Messaging/MessagingUtilities.cs b/src/DotNetOpenAuth.Core/Messaging/MessagingUtilities.cs
index b26deeb..42ba18e 100644
--- a/src/DotNetOpenAuth.Core/Messaging/MessagingUtilities.cs
+++ b/src/DotNetOpenAuth.Core/Messaging/MessagingUtilities.cs
@@ -290,6 +290,43 @@ namespace DotNetOpenAuth.Messaging {
 		}
 
 		/// <summary>
+		/// Compares to string values for ordinal equality in such a way that its execution time does not depend on how much of the value matches.
+		/// </summary>
+		/// <param name="value1">The first value.</param>
+		/// <param name="value2">The second value.</param>
+		/// <returns>A value indicating whether the two strings share ordinal equality.</returns>
+		/// <remarks>
+		/// In signature equality checks, a difference in execution time based on how many initial characters match MAY
+		/// be used as an attack to figure out the expected signature.  It is therefore important to make a signature
+		/// equality check's execution time independent of how many characters match the expected value.
+		/// See http://codahale.com/a-lesson-in-timing-attacks/ for more information.
+		/// </remarks>
+		public static bool EqualsConstantTime(string value1, string value2) {
+			// If exactly one value is null, they don't match.
+			if (value1 == null ^ value2 == null) {
+				return false;
+			}
+
+			// If both values are null (since if one is at this point then they both are), it's a match.
+			if (value1 == null) {
+				return true;
+			}
+
+			if (value1.Length != value2.Length) {
+				return false;
+			}
+
+			// This looks like a pretty crazy way to compare values, but it provides a constant time equality check,
+			// and is more resistant to compiler optimizations than simply setting a boolean flag and returning the boolean after the loop.
+			int result = 0;
+			for (int i = 0; i < value1.Length; i++) {
+				result |= value1[i] ^ value2[i];
+			}
+
+			return result == 0;
+		}
+
+		/// <summary>
 		/// Clears any existing elements in a collection and fills the collection with a given set of values.
 		/// </summary>
 		/// <typeparam name="T">The type of value kept in the collection.</typeparam>
@@ -868,43 +905,6 @@ namespace DotNetOpenAuth.Messaging {
 		}
 
 		/// <summary>
-		/// Compares to string values for ordinal equality in such a way that its execution time does not depend on how much of the value matches.
-		/// </summary>
-		/// <param name="value1">The first value.</param>
-		/// <param name="value2">The second value.</param>
-		/// <returns>A value indicating whether the two strings share ordinal equality.</returns>
-		/// <remarks>
-		/// In signature equality checks, a difference in execution time based on how many initial characters match MAY
-		/// be used as an attack to figure out the expected signature.  It is therefore important to make a signature
-		/// equality check's execution time independent of how many characters match the expected value.
-		/// See http://codahale.com/a-lesson-in-timing-attacks/ for more information.
-		/// </remarks>
-		internal static bool EqualsConstantTime(string value1, string value2) {
-			// If exactly one value is null, they don't match.
-			if (value1 == null ^ value2 == null) {
-				return false;
-			}
-
-			// If both values are null (since if one is at this point then they both are), it's a match.
-			if (value1 == null) {
-				return true;
-			}
-
-			if (value1.Length != value2.Length) {
-				return false;
-			}
-
-			// This looks like a pretty crazy way to compare values, but it provides a constant time equality check,
-			// and is more resistant to compiler optimizations than simply setting a boolean flag and returning the boolean after the loop.
-			int result = 0;
-			for (int i = 0; i < value1.Length; i++) {
-				result |= value1[i] ^ value2[i];
-			}
-
-			return result == 0;
-		}
-
-		/// <summary>
 		/// Adds a set of HTTP headers to an <see cref="HttpResponse"/> instance,
 		/// taking care to set some headers to the appropriate properties of
 		/// <see cref="HttpResponse" />
diff --git a/src/DotNetOpenAuth.OAuth2.AuthorizationServer/OAuth2/AuthorizationServer.cs b/src/DotNetOpenAuth.OAuth2.AuthorizationServer/OAuth2/AuthorizationServer.cs
index 9696402..c7a1a23 100644
--- a/src/DotNetOpenAuth.OAuth2.AuthorizationServer/OAuth2/AuthorizationServer.cs
+++ b/src/DotNetOpenAuth.OAuth2.AuthorizationServer/OAuth2/AuthorizationServer.cs
@@ -63,7 +63,7 @@ namespace DotNetOpenAuth.OAuth2 {
 				if (message.ResponseType == EndUserAuthorizationResponseType.AuthorizationCode) {
 					// Clients with no secrets can only request implicit grant types.
 					var client = this.AuthorizationServerServices.GetClientOrThrow(message.ClientIdentifier);
-					ErrorUtilities.VerifyProtocol(!string.IsNullOrEmpty(client.Secret), Protocol.EndUserAuthorizationRequestErrorCodes.UnauthorizedClient);
+					ErrorUtilities.VerifyProtocol(client.HasNonEmptySecret, Protocol.EndUserAuthorizationRequestErrorCodes.UnauthorizedClient);
 				}
 			}
 
diff --git a/src/DotNetOpenAuth.OAuth2.AuthorizationServer/OAuth2/ChannelElements/MessageValidationBindingElement.cs b/src/DotNetOpenAuth.OAuth2.AuthorizationServer/OAuth2/ChannelElements/MessageValidationBindingElement.cs
index be4f70d..23dcbf5 100644
--- a/src/DotNetOpenAuth.OAuth2.AuthorizationServer/OAuth2/ChannelElements/MessageValidationBindingElement.cs
+++ b/src/DotNetOpenAuth.OAuth2.AuthorizationServer/OAuth2/ChannelElements/MessageValidationBindingElement.cs
@@ -81,9 +81,8 @@ namespace DotNetOpenAuth.OAuth2.ChannelElements {
 			var authenticatedClientRequest = message as AuthenticatedClientRequestBase;
 			if (authenticatedClientRequest != null) {
 				var client = this.AuthorizationServer.GetClientOrThrow(authenticatedClientRequest.ClientIdentifier);
-				string secret = client.Secret;
-				AuthServerUtilities.TokenEndpointVerify(!string.IsNullOrEmpty(secret), Protocol.AccessTokenRequestErrorCodes.UnauthorizedClient); // an empty secret is not allowed for client authenticated calls.
-				AuthServerUtilities.TokenEndpointVerify(MessagingUtilities.EqualsConstantTime(secret, authenticatedClientRequest.ClientSecret), Protocol.AccessTokenRequestErrorCodes.InvalidClient, AuthServerStrings.ClientSecretMismatch);
+				AuthServerUtilities.TokenEndpointVerify(client.HasNonEmptySecret, Protocol.AccessTokenRequestErrorCodes.UnauthorizedClient); // an empty secret is not allowed for client authenticated calls.
+				AuthServerUtilities.TokenEndpointVerify(client.IsValidClientSecret(authenticatedClientRequest.ClientSecret), Protocol.AccessTokenRequestErrorCodes.InvalidClient, AuthServerStrings.ClientSecretMismatch);
 
 				if (clientCredentialOnly != null) {
 					clientCredentialOnly.CredentialsValidated = true;
diff --git a/src/DotNetOpenAuth.OAuth2.AuthorizationServer/OAuth2/ClientDescription.cs b/src/DotNetOpenAuth.OAuth2.AuthorizationServer/OAuth2/ClientDescription.cs
index 76c3ea6..1ec9789 100644
--- a/src/DotNetOpenAuth.OAuth2.AuthorizationServer/OAuth2/ClientDescription.cs
+++ b/src/DotNetOpenAuth.OAuth2.AuthorizationServer/OAuth2/ClientDescription.cs
@@ -9,6 +9,7 @@ namespace DotNetOpenAuth.OAuth2 {
 	using System.Collections.Generic;
 	using System.Linq;
 	using System.Text;
+	using DotNetOpenAuth.Messaging;
 
 	/// <summary>
 	/// A default implementation of the <see cref="IClientDescription"/> interface.
@@ -20,6 +21,11 @@ namespace DotNetOpenAuth.OAuth2 {
 		private readonly Func<Uri, bool> isCallbackAllowed;
 
 		/// <summary>
+		/// The client's secret, if any.
+		/// </summary>
+		private readonly string secret;
+
+		/// <summary>
 		/// Initializes a new instance of the <see cref="ClientDescription"/> class.
 		/// </summary>
 		/// <param name="secret">The secret.</param>
@@ -27,18 +33,13 @@ namespace DotNetOpenAuth.OAuth2 {
 		/// <param name="clientType">Type of the client.</param>
 		/// <param name="isCallbackAllowed">A delegate that determines whether the callback is allowed.</param>
 		public ClientDescription(string secret, Uri defaultCallback, ClientType clientType, Func<Uri, bool> isCallbackAllowed = null) {
-			this.Secret = secret;
+			this.secret = secret;
 			this.DefaultCallback = defaultCallback;
 			this.ClientType = clientType;
 			this.isCallbackAllowed = isCallbackAllowed;
 		}
 
 		/// <summary>
-		/// Gets the client secret.
-		/// </summary>
-		public string Secret { get; private set; }
-
-		/// <summary>
 		/// Gets the callback to use when an individual authorization request
 		/// does not include an explicit callback URI.
 		/// </summary>
@@ -53,6 +54,13 @@ namespace DotNetOpenAuth.OAuth2 {
 		public ClientType ClientType { get; private set; }
 
 		/// <summary>
+		/// Gets a value indicating whether a non-empty secret is registered for this client.
+		/// </summary>
+		public bool HasNonEmptySecret {
+			get { return !string.IsNullOrEmpty(this.secret); }
+		}
+
+		/// <summary>
 		/// Determines whether a callback URI included in a client's authorization request
 		/// is among those allowed callbacks for the registered client.
 		/// </summary>
@@ -67,5 +75,24 @@ namespace DotNetOpenAuth.OAuth2 {
 
 			return EqualityComparer<Uri>.Default.Equals(this.DefaultCallback, callback);
 		}
+
+		#region IClientDescription Members
+
+		/// <summary>
+		/// Checks whether the specified client secret is correct.
+		/// </summary>
+		/// <param name="secret">The secret obtained from the client.</param>
+		/// <returns><c>true</c> if the secret matches the one in the authorization server's record for the client; <c>false</c> otherwise.</returns>
+		/// <remarks>
+		/// All string equality checks, whether checking secrets or their hashes,
+		/// should be done using <see cref="MessagingUtilites.EqualsConstantTime"/> to mitigate timing attacks.
+		/// </remarks>
+		public bool IsValidClientSecret(string secret) {
+			Requires.NotNullOrEmpty(secret, "secret");
+
+			return MessagingUtilities.EqualsConstantTime(secret, this.secret);
+		}
+
+		#endregion
 	}
 }
diff --git a/src/DotNetOpenAuth.OAuth2.ClientAuthorization/OAuth2/IClientDescription.cs b/src/DotNetOpenAuth.OAuth2.ClientAuthorization/OAuth2/IClientDescription.cs
index d30151b..bcef28b 100644
--- a/src/DotNetOpenAuth.OAuth2.ClientAuthorization/OAuth2/IClientDescription.cs
+++ b/src/DotNetOpenAuth.OAuth2.ClientAuthorization/OAuth2/IClientDescription.cs
@@ -15,11 +15,6 @@ namespace DotNetOpenAuth.OAuth2 {
 	[ContractClass(typeof(IClientDescriptionContract))]
 	public interface IClientDescription {
 		/// <summary>
-		/// Gets the client secret.
-		/// </summary>
-		string Secret { get; }
-
-		/// <summary>
 		/// Gets the callback to use when an individual authorization request
 		/// does not include an explicit callback URI.
 		/// </summary>
@@ -32,6 +27,11 @@ namespace DotNetOpenAuth.OAuth2 {
 		ClientType ClientType { get; }
 
 		/// <summary>
+		/// Gets a value indicating whether a non-empty secret is registered for this client.
+		/// </summary>
+		bool HasNonEmptySecret { get; }
+
+		/// <summary>
 		/// Determines whether a callback URI included in a client's authorization request 
 		/// is among those allowed callbacks for the registered client.
 		/// </summary>
@@ -56,6 +56,17 @@ namespace DotNetOpenAuth.OAuth2 {
 		/// </para>
 		/// </remarks>
 		bool IsCallbackAllowed(Uri callback);
+
+		/// <summary>
+		/// Checks whether the specified client secret is correct.
+		/// </summary>
+		/// <param name="secret">The secret obtained from the client.</param>
+		/// <returns><c>true</c> if the secret matches the one in the authorization server's record for the client; <c>false</c> otherwise.</returns>
+		/// <remarks>
+		/// All string equality checks, whether checking secrets or their hashes,
+		/// should be done using <see cref="MessagingUtilites.EqualsConstantTime"/> to mitigate timing attacks.
+		/// </remarks>
+		bool IsValidClientSecret(string secret);
 	}
 
 	/// <summary>
@@ -66,14 +77,6 @@ namespace DotNetOpenAuth.OAuth2 {
 		#region IClientDescription Members
 
 		/// <summary>
-		/// Gets the client secret.
-		/// </summary>
-		/// <value></value>
-		string IClientDescription.Secret {
-			get { throw new NotImplementedException(); }
-		}
-
-		/// <summary>
 		/// Gets the type of the client.
 		/// </summary>
 		ClientType IClientDescription.ClientType {
@@ -95,6 +98,13 @@ namespace DotNetOpenAuth.OAuth2 {
 		}
 
 		/// <summary>
+		/// Gets a value indicating whether a non-empty secret is registered for this client.
+		/// </summary>
+		bool IClientDescription.HasNonEmptySecret {
+			get { throw new NotImplementedException(); }
+		}
+
+		/// <summary>
 		/// Determines whether a callback URI included in a client's authorization request
 		/// is among those allowed callbacks for the registered client.
 		/// </summary>
@@ -108,6 +118,20 @@ namespace DotNetOpenAuth.OAuth2 {
 			throw new NotImplementedException();
 		}
 
+		/// <summary>
+		/// Checks whether the specified client secret is correct.
+		/// </summary>
+		/// <param name="secret">The secret obtained from the client.</param>
+		/// <returns><c>true</c> if the secret matches the one in the authorization server's record for the client; <c>false</c> otherwise.</returns>
+		/// <remarks>
+		/// All string equality checks, whether checking secrets or their hashes,
+		/// should be done using <see cref="MessagingUtilites.EqualsConstantTime"/> to mitigate timing attacks.
+		/// </remarks>
+		bool IClientDescription.IsValidClientSecret(string secret) {
+			Requires.NotNullOrEmpty(secret, "secret");
+			throw new NotImplementedException();
+		}
+
 		#endregion
 	}
 }