pcibraro / hawknet

Hawk protocol implementation for .NET
MIT License
114 stars 35 forks source link

Problems validating bewit when url contains spaces encoded as %20 #29

Open nicosabena opened 9 years ago

nicosabena commented 9 years ago

Hi Pablo. When using urls that have spaces encoded as %20 instead of a plus sign (both are valid, apparently), the code is failing. This is because of a behavior in the object return from HttpUtility.ParseQueryString(uri.Query) in the method RemoveBewitFromQuery. This object, when ToString() is invoked, replaces %20 with the "+" sign. As a consequence, the resulting url doesn't match the original used to generate the mac, and the check fails.

A failing test:

       [TestMethod]
        public void ShouldAuthenticateBewitWithEncodedUrlWithPercentSpaces()
        {
            var credential = new HawkCredential
            {
                Id = "1",
                Algorithm = "sha1",
                Key = "werxhqb98rpaxn39848xrunpaw3489ruxnpa98w4rxn",
            };

            var bewit = Hawk.GetBewit("example.com", new Uri("http://example.com:8080/resource/4?path=%2Fmovie%2Fcomedy%20club%2F2014"), credential,
                200, "hello");

            var claims = Hawk.AuthenticateBewit(bewit, "example.com", new Uri("http://example.com:8080/resource/4?path=%2Fmovie%2Fcomedy%20club%2F2014&bewit=" + bewit),
                s => credential);

            Assert.IsNotNull(claims);
        }
nicosabena commented 9 years ago

Further testing shows that foreign characters also cause problems. For example, the word "Solución" is encoded as Soluci%C3%B3n by some frameworks (including .NET in the new Uri() constructor), but HttpValueCollection.ToString() encodes it as %uxxxx, which is not even correctly parsed by the Uri constructor.

        [TestMethod]
        public void ShouldAuthenticateBewitWithEncodedUrlWithForeignChars()
        {
            var credential = new HawkCredential
            {
                Id = "1",
                Algorithm = "sha1",
                Key = "werxhqb98rpaxn39848xrunpaw3489ruxnpa98w4rxn",
            };

            var bewit = Hawk.GetBewit("example.com", new Uri("http://example.com:8080/resource/4?path=Soluci%C3%B3n.docx"), credential,
                200, "hello");

            var claims = Hawk.AuthenticateBewit(bewit, "example.com", new Uri("http://example.com:8080/resource/4?path=Soluci%C3%B3n.docx&bewit=" + bewit),
                s => credential);

            Assert.IsNotNull(claims);
        }
pcibraro commented 9 years ago

Thanks for the repro. Let me work on a fix as soon as possible.

nicosabena commented 9 years ago

HttpValueCollection.ToString() should probably be replaced, as it encodes in a not very standard way. Could this be a remplacement?

        private static Uri RemoveBewitFromQuery(Uri uri)
        {
            ...             
            var resultingQuery = string.Join(
                "&",
                parsedQueryString.AllKeys.Select(x => x + "=" + HttpUtility.UrlEncode(parsedQueryString[x])).ToArray());

But even after that, we're left with the problem of %20 and + equivalences for encoding a space. The normalization in CalculateMac() could include a replacement of all "%20" with a plus sign.

nicosabena commented 9 years ago

Thanks! Will you be updating the NuGet package?