From fab9fa095a80856987d0619fd657406834d8a866 Mon Sep 17 00:00:00 2001
From: Tom Peters <tpeters@synacor.com>
Date: Tue, 20 Feb 2018 12:41:03 -0500
Subject: [PATCH] URL decode the client_id and client_secret in Authz header

According to the OAuth2 spec
(https://tools.ietf.org/html/rfc6749#section-2.3.1), the client_id
and client_secret are first separately encoded using the
application/x-www-form-urlencoded algorithm and then base64 encoded.
---
 util.go      | 16 +++++++++++++++-
 util_test.go | 26 ++++++++++++++++++++++----
 2 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/util.go b/util.go
index 42c9565..be1e988 100644
--- a/util.go
+++ b/util.go
@@ -4,6 +4,7 @@ import (
 	"encoding/base64"
 	"errors"
 	"net/http"
+	"net/url"
 	"strings"
 )
 
@@ -51,7 +52,20 @@ func CheckBasicAuth(r *http.Request) (*BasicAuth, error) {
 		return nil, errors.New("Invalid authorization message")
 	}
 
-	return &BasicAuth{Username: pair[0], Password: pair[1]}, nil
+	// Decode the client_id and client_secret pairs as per
+	// https://tools.ietf.org/html/rfc6749#section-2.3.1
+
+	username, err := url.QueryUnescape(pair[0])
+	if err != nil {
+		return nil, err
+	}
+
+	password, err := url.QueryUnescape(pair[1])
+	if err != nil {
+		return nil, err
+	}
+
+	return &BasicAuth{Username: username, Password: password}, nil
 }
 
 // Return "Bearer" token from request. The header has precedence over query string.
diff --git a/util_test.go b/util_test.go
index 234353f..d638cc0 100644
--- a/util_test.go
+++ b/util_test.go
@@ -7,9 +7,11 @@ import (
 )
 
 const (
-	badAuthValue        = "Digest XHHHHHHH"
-	goodAuthValue       = "Basic dGVzdDp0ZXN0"
-	goodBearerAuthValue = "Bearer BGFVTDUJDp0ZXN0"
+	badAuthValue           = "Digest XHHHHHHH"
+	badUsernameInAuthValue = "Basic dSUyc2VybmFtZTpwYXNzd29yZA==" // u%2sername:password
+	badPasswordInAuthValue = "Basic dXNlcm5hbWU6cGElMnN3b3Jk"     // username:pa%2sword
+	goodAuthValue          = "Basic Y2xpZW50K25hbWU6Y2xpZW50KyUyNGVjcmV0"
+	goodBearerAuthValue    = "Bearer BGFVTDUJDp0ZXN0"
 )
 
 func TestBasicAuth(t *testing.T) {
@@ -28,6 +30,22 @@ func TestBasicAuth(t *testing.T) {
 		return
 	}
 
+	// with invalid username
+	r.Header.Set("Authorization", badUsernameInAuthValue)
+	b, err = CheckBasicAuth(r)
+	if b != nil || err == nil {
+		t.Errorf("Validated invalid auth with bad username")
+		return
+	}
+
+	// with invalid username
+	r.Header.Set("Authorization", badPasswordInAuthValue)
+	b, err = CheckBasicAuth(r)
+	if b != nil || err == nil {
+		t.Errorf("Validated invalid auth with bad password")
+		return
+	}
+
 	// with valid header
 	r.Header.Set("Authorization", goodAuthValue)
 	b, err = CheckBasicAuth(r)
@@ -37,7 +55,7 @@ func TestBasicAuth(t *testing.T) {
 	}
 
 	// check extracted auth data
-	if b.Username != "test" || b.Password != "test" {
+	if b.Username != "client name" || b.Password != "client $ecret" {
 		t.Errorf("Error decoding basic auth")
 	}
 }