Merge pull request #240 from stormpath/235-fix-sauthc1-parens

Fix URL encoding for OAuth requests
stormpath · Dec 28, 2016 · d05125f · d05125f
2 parents e2834e4 + 4af80a4
commit d05125f
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 16 deletions.
diff --git a/src/Stormpath.SDK.Abstractions/Http/UrlEncoding.cs b/src/Stormpath.SDK.Abstractions/Http/UrlEncoding.cs
@@ -39,18 +39,19 @@ public static string Encode(string value, bool isPath = false, bool canonicalize
 
             var encoded = WebUtility.UrlEncode(value);
 
-            // WebUtility doesn't escape ! by default
-            encoded = encoded.Replace("!", "%21");
+            // WebUtility doesn't escape some characters by default
+            encoded = encoded
+                .Replace("!", "%21")
+                .Replace("(", "%28")
+                .Replace(")", "%29");
 
             // Perform some custom Stormpath encoding
             if (canonicalize)
             {
                 encoded = encoded
                     .Replace("+", "%20") // Spaces as %20
                     .Replace("*", "%2A")
-                    .Replace("%7E", "~") // Tildes stay as they are
-                    .Replace("(", "%28")
-                    .Replace(")", "%29");
+                    .Replace("%7E", "~"); // Tildes stay unencoded
 
                 if (isPath)
                 {

diff --git a/test/Stormpath.SDK.Tests.Integration/Async/Oauth_tests.cs b/test/Stormpath.SDK.Tests.Integration/Async/Oauth_tests.cs
@@ -151,7 +151,7 @@ public async Task Password_grant_with_special_characters(TestClientProvider clie
 
             // Add the test accounts
             var randomEmail = new RandomEmail("testmail.stormpath.com");
-            var password = "P@sword#123$!";
+            var password = "P@ss* word#123$!()~";
             await createdApplication.CreateAccountAsync("Test", "testerman", randomEmail, password);
 
             var passwordGrantRequest = OauthRequests.NewPasswordGrantRequest()

diff --git a/test/Stormpath.SDK.Tests.Integration/Sync/Oauth_tests.cs b/test/Stormpath.SDK.Tests.Integration/Sync/Oauth_tests.cs
@@ -147,7 +147,7 @@ public void Password_grant_with_special_characters(TestClientProvider clientBuil
 
             // Add the test accounts
             var randomEmail = new RandomEmail("testmail.stormpath.com");
-            var password = "P@sword#123$!";
+            var password = "P@ss* word#123$!()~";
             createdApplication.CreateAccount("Test", "testerman", randomEmail, password);
 
             var passwordGrantRequest = OauthRequests.NewPasswordGrantRequest()

diff --git a/test/Stormpath.SDK.Tests/FormUrlEncoder_tests.cs b/test/Stormpath.SDK.Tests/FormUrlEncoder_tests.cs
@@ -46,5 +46,27 @@ public void Encodes_password_grant_attempt()
             result.ShouldContain("password=Secret1");
             result.ShouldContain("accountStore=https%3A%2F%2Fapi.stormpath.com%2Fv1%2Fdirectories%2F1bcd23ec1d0aEXAMPLE");
         }
+
+        /// <summary>
+        /// Regression test for https://github.com/stormpath/stormpath-sdk-dotnet/issues/235
+        /// </summary>
+        [Fact]
+        public void Encodes_parenthesis_correctly()
+        {
+            var dataStore = TestDataStore.Create();
+
+            var createGrantAttempt = dataStore.Instantiate<IPasswordGrantAuthenticationAttempt>();
+            createGrantAttempt.SetLogin("[email protected]");
+            createGrantAttempt.SetPassword("Testing123()");
+            createGrantAttempt.SetAccountStore("https://api.stormpath.com/v1/directories/1bcd23ec1d0aEXAMPLE");
+
+            var properties = (createGrantAttempt as AbstractResource).GetResourceData().GetUpdatedProperties().ToDictionary();
+            var result = new FormUrlEncoder(properties)
+                .ToString()
+                .Split('&');
+
+            result.ShouldContain("username=nate%40stormpath.com");
+            result.ShouldContain("password=Testing123%28%29");
+        }
     }
 }
diff --git a/test/Stormpath.SDK.Tests/UrlEncoding_tests.cs b/test/Stormpath.SDK.Tests/UrlEncoding_tests.cs
@@ -60,18 +60,10 @@ public void Canononicalizes_path_correctly()
             canonicalizedWithPath.ShouldBe("/");
         }
 
-        [Fact]
-        public void Does_not_escape_parenthesis()
-        {
-            var escaped = UrlEncoding.Encode("()");
-
-            escaped.ShouldBe("()");
-        }
-
         [Fact]
         public void Canonicalizes_parenthesis()
         {
-            var canonicalized = UrlEncoding.Encode("()", canonicalize: true);
+            var canonicalized = UrlEncoding.Encode("()");
 
             canonicalized.ShouldBe("%28%29");
         }