Merged r114 fix and tests for TrustRoot into 0.1 branch.

git-svn-id: https://dotnetopenid.googlecode.com/svn/branches/0.1@115 01efa1a6-402a-0410-b0ae-47b76eba00f0

5 files changed, 213 insertions, 35 deletions

diff --git a/source/Janrain.OpenId/Properties/AssemblyInfo.cs b/source/Janrain.OpenId/Properties/AssemblyInfo.cs
index 4d4b029..0171e55 100644
--- a/source/Janrain.OpenId/Properties/AssemblyInfo.cs
+++ b/source/Janrain.OpenId/Properties/AssemblyInfo.cs
@@ -31,5 +31,5 @@ using System.Runtime.InteropServices;
 //

 // You can specify all the values or you can default the Revision and Build Numbers 

 // by using the '*' as shown below:

-[assembly: AssemblyVersion("1.0.0.0")]

-[assembly: AssemblyFileVersion("1.0.0.0")]

+[assembly: AssemblyVersion("0.1.*")]

+[assembly: AssemblyFileVersion("0.1.*")]

diff --git a/source/Janrain.OpenId/Server/CheckIdRequest.cs b/source/Janrain.OpenId/Server/CheckIdRequest.cs
index f4bb567..e0a4da0 100644
--- a/source/Janrain.OpenId/Server/CheckIdRequest.cs
+++ b/source/Janrain.OpenId/Server/CheckIdRequest.cs
@@ -3,6 +3,7 @@ using System.Collections;
 using System.Collections.Specialized;

 using System.Text;

 using Janrain.OpenId.RegistrationExtension;

+using System.Diagnostics;

 

 namespace Janrain.OpenId.Server

 {

@@ -92,7 +93,6 @@ namespace Janrain.OpenId.Server
                 throw new MalformedReturnUrl(query, return_to);

             }

 

-            // TODO This just seems wonky to me

             _trust_root = query.Get(QueryStringArgs.openid.trust_root);

             if (_trust_root == null)

                 _trust_root = _return_to.AbsoluteUri;

@@ -390,23 +390,13 @@ namespace Janrain.OpenId.Server
         {

             get

             {

-                // TODO this doesn't seem right to me

-                if (_trust_root == null)

-                    return true;

+                Debug.Assert(_trust_root != null, "The constructor should have guaranteed this.");

 

-                try

-                {

-                    TrustRoot tr = new TrustRoot(_trust_root);

-                }

-                catch(ArgumentException)

-                {

-                    throw new MalformedTrustRoot(null, _trust_root);

+                try {

+                    return new TrustRoot(_trust_root).ValidateUrl(_return_to);

+                } catch (MalformedTrustRoot) {

+                    return false;

                 }

-

-                // TODO (willem.muller) - The trust code on 04/04/07 is dodgy. So returing true so all trust roots are valid for now.

-                return true;

-                

-                //return tr.ValidateUrl(_return_to);

             }

         }

 

diff --git a/source/Janrain.OpenId/Server/TrustRoot.cs b/source/Janrain.OpenId/Server/TrustRoot.cs
index 15e1ca2..1f618c2 100644
--- a/source/Janrain.OpenId/Server/TrustRoot.cs
+++ b/source/Janrain.OpenId/Server/TrustRoot.cs
@@ -1,16 +1,20 @@
 using System;

-using System.Collections.Generic;

 using System.Text.RegularExpressions;

+using System.Diagnostics;

 

 namespace Janrain.OpenId.Server

 {

+    /// <summary>

+    /// A trust root to validate requests and match return URLs against.

+    /// </summary>

+    /// <!-- http://openid.net/specs/openid-authentication-1_1.html#anchor16 -->

+    /// <!-- http://openid.net/specs/openid-authentication-1_1.html#anchor21 -->

     public class TrustRoot

     {

-

         #region Private Members

 

         private static Regex _tr_regex = new Regex("^(?<scheme>https?)://((?<wildcard>\\*)|(?<wildcard>\\*\\.)?(?<host>[a-zA-Z0-9-]+(\\.[a-zA-Z0-9-]+)*)\\.?)(:(?<port>[0-9]+))?(?<path>(/.*|$))");

-        private static string[] _top_level_domains =    {"com", "edu", "gov", "int", "mil", "net", "org", "biz", "info", "name", "museum", "coop", "aero", "ac", "ad", "ae", "" +

+        private static string[] _top_level_domains =    {"com", "edu", "gov", "int", "mil", "net", "org", "biz", "info", "name", "museum", "coop", "aero", "ac", "ad", "ae",

                                                         "af", "ag", "ai", "al", "am", "an", "ao", "aq", "ar", "as", "at", "au", "aw", "az", "ba", "bb", "bd", "be", "bf", "bg", "bh", "bi", "bj", "" +

                                                         "bm", "bn", "bo", "br", "bs", "bt", "bv", "bw", "by", "bz", "ca", "cc", "cd", "cf", "cg", "ch", "ci", "ck", "cl", "cm", "cn", "co", "cr", "" +

                                                         "cu", "cv", "cx", "cy", "cz", "de", "dj", "dk", "dm", "do", "dz", "ec", "ee", "eg", "eh", "er", "es", "et", "fi", "fj", "fk", "fm", "fo", "" +

@@ -58,7 +62,7 @@ namespace Janrain.OpenId.Server
             }

             else

             {

-                throw new ArgumentException(unparsed + " does not appear to be a valid TrustRoot");

+                throw new MalformedTrustRoot(null, unparsed + " does not appear to be a valid TrustRoot");

             }

         }

 

@@ -102,6 +106,23 @@ namespace Janrain.OpenId.Server
 

         #region Methods

 

+        /// <summary>

+        /// Validates a URL against this trust root.

+        /// </summary>

+        /// <param name="url">A string specifying URL to check.</param>

+        /// <returns>Whether the given URL is within this trust root.</returns>

+        public bool ValidateUrl(string url)

+        {

+            Uri uri = new Uri(url);

+

+            return ValidateUrl(uri);

+        }

+

+        /// <summary>

+        /// Validates a URL against this trust root.

+        /// </summary>

+        /// <param name="url">The URL to check.</param>

+        /// <returns>Whether the given URL is within this trust root.</returns>

         public bool ValidateUrl(Uri url)

         {

             if (url.Scheme != _scheme)

@@ -117,39 +138,68 @@ namespace Janrain.OpenId.Server
                     return false;

                 }

             }

-            else if (_host != String.Empty)

+            else

             {

+                Debug.Assert(_host != string.Empty, "The host part of the Regex should evaluate to at least one char for successful parsed trust roots.");

                 string[] host_parts = _host.Split('.');

                 string[] url_parts = url.Host.Split('.');

-                string end_parts = url_parts[url_parts.Length - host_parts.Length];

 

-                for (int i = 0; i < end_parts.Length; i++)

+                // If the domain contain the wildcard has more parts than the URL to match against,

+                // it naturally can't be valid.

+                // Unless *.example.com actually matches example.com too.

+                if (host_parts.Length > url_parts.Length)

+                    return false;

+

+                int offset = url_parts.Length - host_parts.Length;

+

+                // Compare last part first and move forward.

+                // Could be done by using EndsWith, but this solution seems more elegant.

+                for (int i = host_parts.Length - 1; i >= 0; i--)

                 {

-                    if (end_parts != host_parts[i])

+                    /*

+                    if (host_parts[i].Equals("*", StringComparison.Ordinal))

+                    {

+                        break;

+                    }

+                     */

+

+                    if (!host_parts[i].Equals(url_parts[i + 1], StringComparison.OrdinalIgnoreCase))

+                    {

                         return false;

+                    }

                 }

             }

 

-            if (url.PathAndQuery == _path)

+            // If path matches or is specified to root ...

+            if (_path.Equals(url.PathAndQuery, StringComparison.Ordinal)

+                || _path.Equals("/", StringComparison.Ordinal))

                 return true;

 

+            // If trust root has a longer path, the return URL must be invalid.

+            if (_path.Length > url.PathAndQuery.Length)

+                return false;

+

+            // The following code assures that http://example.com/directory isn't below http://example.com/dir,

+            // but makes sure http://example.com/dir/ectory is below http://example.com/dir

+            // Maybe this addition should be removed, as it is hard to see, if a path is below (/dir.html would accept /dir.html/something too)

             int path_len = _path.Length;

             string url_prefix = url.PathAndQuery.Substring(0, path_len);

 

             if (_path != url_prefix)

                 return false;

 

-            string allowed = "";

+            // If trust root includes a query string ...

             if (_path.Contains("?"))

-                allowed = "&";

-            else

-                allowed = "?";

+            {

+                // ... make sure return URL begins with a new argument

+                return url.PathAndQuery[path_len] == '&';

+            }

 

-            

-            return (allowed.IndexOf(_path[_path.Length - 1]) >= 0  || allowed.IndexOf(url.PathAndQuery[path_len]) >= 0);

+            // Or make sure a query string is introduced or a path below trust root

+            return url.PathAndQuery[path_len] == '?'

+                || url.PathAndQuery[path_len] == '/';

         }

 

         #endregion

-

     }

-}

+}
\ No newline at end of file
diff --git a/source/OpenIdTests/OpenIdTests.csproj b/source/OpenIdTests/OpenIdTests.csproj
index 51f6781..12a7a99 100644
--- a/source/OpenIdTests/OpenIdTests.csproj
+++ b/source/OpenIdTests/OpenIdTests.csproj
@@ -42,6 +42,7 @@
     <Compile Include="MemoryStoreTestSuite.cs" />

     <Compile Include="OpenIdProfileFieldsTestSuite.cs" />

     <Compile Include="Properties\AssemblyInfo.cs" />

+    <Compile Include="TrustRootTestSuite.cs" />

   </ItemGroup>

   <ItemGroup>

     <Content Include="dhpriv.txt" />

diff --git a/source/OpenIdTests/TrustRootTestSuite.cs b/source/OpenIdTests/TrustRootTestSuite.cs
new file mode 100644
index 0000000..1ab0539
--- /dev/null
+++ b/source/OpenIdTests/TrustRootTestSuite.cs
@@ -0,0 +1,137 @@
+using System;

+using System.Collections.Generic;

+using System.Text;

+using NUnit.Framework;

+using Janrain.OpenId.Server;

+

+namespace OpenIdTests {

+	[TestFixture]

+	public class TrustRootTestSuite {

+		[Test]

+		public void ValidTrustRootsTest() {

+			// Just create these.  If any are determined to be invalid,

+			// an exception should be thrown that would fail this test.

+			new TrustRoot("http://www.myopenid.com");

+			new TrustRoot("http://www.myopenid.com/");

+			new TrustRoot("http://www.myopenid.com:5221/");

+			new TrustRoot("https://www.myopenid.com");

+			new TrustRoot("http://www.myopenid.com/abc");

+			new TrustRoot("http://www.myopenid.com/abc/");

+			new TrustRoot("http://*.myopenid.com/");

+			new TrustRoot("http://*.com/");

+			new TrustRoot("http://*.guest.myopenid.com/");

+		}

+

+		[Test]

+		[ExpectedException(typeof(ArgumentNullException))]

+		public void InvalidTrustRootNull() {

+			new TrustRoot(null);

+		}

+

+		[Test]

+		[ExpectedException(typeof(MalformedTrustRoot))]

+		public void InvalidTrustRootEmpty() {

+			new TrustRoot("");

+		}

+

+		[Test]

+		[ExpectedException(typeof(MalformedTrustRoot))]

+		public void InvalidTrustRootBadProtocol() {

+			new TrustRoot("asdf://www.microsoft.com/");

+		}

+

+		[Test]

+		[ExpectedException(typeof(MalformedTrustRoot))]

+		public void InvalidTrustRootNoScheme() {

+			new TrustRoot("www.guy.com");

+		}

+

+		[Test]

+		[ExpectedException(typeof(MalformedTrustRoot))]

+		public void InvalidTrustRootBadWildcard() {

+			new TrustRoot("http://*www.my.com");

+		}

+

+		[Test]

+		[ExpectedException(typeof(MalformedTrustRoot))]

+		public void InvalidTrustRootTwoWildcards1() {

+			new TrustRoot("http://**.my.com");

+		}

+

+		[Test]

+		[ExpectedException(typeof(MalformedTrustRoot))]

+		public void InvalidTrustRootTwoWildcards2() {

+			new TrustRoot("http://*.*.my.com");

+		}

+

+		[Test]

+		public void IsSaneTest() {

+			Assert.IsTrue(new TrustRoot("http://www.myopenid.com").IsSane);

+			Assert.IsTrue(new TrustRoot("http://myopenid.com").IsSane);

+			Assert.IsTrue(new TrustRoot("http://localhost").IsSane);

+			Assert.IsTrue(new TrustRoot("http://localhost:33532/dab").IsSane);

+			Assert.IsTrue(new TrustRoot("http://www.myopenid.com").IsSane);

+

+			Assert.IsFalse(new TrustRoot("http://*.com").IsSane);

+			Assert.IsFalse(new TrustRoot("http://*.co.uk").IsSane);

+		}

+

+		[Test]

+		public void IsValidRootTests() {

+			/* 

+			 * The openid.return_to URL MUST descend from the openid.trust_root, or the 

+			 * Identity Provider SHOULD return an error. Namely, the URL scheme and port 

+			 * MUST match. The path, if present, MUST be equal to or below the value of 

+			 * openid.trust_root, and the domains on both MUST match, or, the 

+			 * openid.trust_root value contain a wildcard like http://*.example.com. 

+			 * The wildcard SHALL only be at the beginning. It is RECOMMENDED Identity 

+			 * Provider's protect their End Users from requests for things like 

+			 * http://*.com/ or http://*.co.uk/.

+			 */

+

+			// Schemes must match

+			Assert.IsFalse(new TrustRoot("https://www.my.com/").ValidateUrl("http://www.my.com/"));

+			Assert.IsFalse(new TrustRoot("http://www.my.com/").ValidateUrl("https://www.my.com/"));

+

+			// Ports must match

+			Assert.IsTrue(new TrustRoot("http://www.my.com/").ValidateUrl("http://www.my.com:80/boo"));

+			Assert.IsTrue(new TrustRoot("http://www.my.com:80/").ValidateUrl("http://www.my.com/boo"));

+			Assert.IsFalse(new TrustRoot("http://www.my.com:79/").ValidateUrl("http://www.my.com/boo"));

+			Assert.IsFalse(new TrustRoot("https://www.my.com/").ValidateUrl("http://www.my.com:79/boo"));

+

+			// Path must be (at or) below trust root

+			Assert.IsTrue(new TrustRoot("http://www.my.com/").ValidateUrl("http://www.my.com/"));

+			Assert.IsTrue(new TrustRoot("http://www.my.com/").ValidateUrl("http://www.my.com/boo"));

+			Assert.IsTrue(new TrustRoot("http://www.my.com/bah").ValidateUrl("http://www.my.com/bah/bah"));

+			Assert.IsTrue(new TrustRoot("http://www.my.com/bah").ValidateUrl("http://www.my.com/bah/bah"));

+			Assert.IsTrue(new TrustRoot("http://www.my.com/bah.html").ValidateUrl("http://www.my.com/bah.html/bah"));

+			Assert.IsFalse(new TrustRoot("http://www.my.com/bah").ValidateUrl("http://www.my.com/bahbah"));

+			Assert.IsTrue(new TrustRoot("http://www.my.com/bah").ValidateUrl("http://www.my.com/bah?q=a"));

+			Assert.IsTrue(new TrustRoot("http://www.my.com/bah?q=a").ValidateUrl("http://www.my.com/bah?q=a"));

+			Assert.IsTrue(new TrustRoot("http://www.my.com/bah?a=b&c=d").ValidateUrl("http://www.my.com/bah?a=b&c=d&e=f"));

+			Assert.IsFalse(new TrustRoot("http://www.my.com/bah?a=b&c=d").ValidateUrl("http://www.my.com/bah?a=b"));

+

+			// Domains MUST match

+			Assert.IsFalse(new TrustRoot("http://www.my.com/").ValidateUrl("http://yours.com/"));

+			Assert.IsFalse(new TrustRoot("http://www.my.com/").ValidateUrl("http://www.yours.com/"));

+			Assert.IsFalse(new TrustRoot("http://www.my.com/").ValidateUrl("http://q.www.my.com/"));

+			Assert.IsFalse(new TrustRoot("http://www.my.com/").ValidateUrl("http://wwww.my.com/"));

+			Assert.IsFalse(new TrustRoot("http://www.my.com/").ValidateUrl("http://www.my.com.uk/"));

+			Assert.IsFalse(new TrustRoot("http://www.my.com/").ValidateUrl("http://www.my.comm/"));

+

+			// Allow for wildcards

+			Assert.IsTrue(new TrustRoot("http://*.www.my.com/").ValidateUrl("http://bah.www.my.com/"));

+			Assert.IsTrue(new TrustRoot("http://*.www.my.com/").ValidateUrl("http://bah.www.my.com/boo"));

+			// These are tested against by the constructor test, as these are invalid wildcard positions.

+			//Assert.IsFalse(new TrustRoot("http://*www.my.com/").ValidateUrl("http://bah.www.my.com/"));

+			//Assert.IsFalse(new TrustRoot("http://*www.my.com/").ValidateUrl("http://wwww.my.com/"));

+

+			// Among those that should return true, mix up character casing to test for case sensitivity.

+			// Host names should be case INSENSITIVE, but paths should probably be case SENSITIVE,

+			// because in some systems they are case sensitive and to ignore this would open

+			// security holes.

+			Assert.IsTrue(new TrustRoot("http://www.my.com/").ValidateUrl("http://WWW.MY.COM/"));

+			Assert.IsFalse(new TrustRoot("http://www.my.com/abc").ValidateUrl("http://www.my.com/ABC"));

+		}

+	}

+}