Skip to content

Commit

Permalink
fix: basic scheme rejects special characters
Browse files Browse the repository at this point in the history
  • Loading branch information
james-d-elliott committed Aug 4, 2023
1 parent 039746c commit fde4d25
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 11 deletions.
47 changes: 41 additions & 6 deletions client_authentication.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ import (
"context"
"crypto/ecdsa"
"crypto/rsa"
"encoding/base64"
"encoding/json"
"fmt"
"net/http"
"net/url"
"strings"
"time"

"github.com/ory/x/errorsx"
Expand Down Expand Up @@ -293,15 +295,48 @@ func findPublicKey(t *jwt.Token, set *jose.JSONWebKeySet, expectsRSAKey bool) (i
}

func clientCredentialsFromRequest(r *http.Request, form url.Values) (clientID, clientSecret string, err error) {
if id, secret, ok := r.BasicAuth(); !ok {
var ok bool

switch clientID, clientSecret, ok, err = clientCredentialsFromBasicAuth(r); {
case err != nil:
return "", "", errorsx.WithStack(ErrInvalidRequest.WithHint("The client credentials in the HTTP authorization header could not be parsed. Either the scheme was missing, the scheme was invalid, or the value had malformed data.").WithWrap(err).WithDebug(err.Error()))
case ok:
return clientID, clientSecret, nil
default:
return clientCredentialsFromRequestBody(form, true)
} else if clientID, err = url.QueryUnescape(id); err != nil {
return "", "", errorsx.WithStack(ErrInvalidRequest.WithHint("The client id in the HTTP authorization header could not be decoded from 'application/x-www-form-urlencoded'.").WithWrap(err).WithDebug(err.Error()))
} else if clientSecret, err = url.QueryUnescape(secret); err != nil {
return "", "", errorsx.WithStack(ErrInvalidRequest.WithHint("The client secret in the HTTP authorization header could not be decoded from 'application/x-www-form-urlencoded'.").WithWrap(err).WithDebug(err.Error()))
}
}

return clientID, clientSecret, nil
func clientCredentialsFromBasicAuth(r *http.Request) (clientID, clientSecret string, ok bool, err error) {
auth := r.Header.Get("Authorization")

if auth == "" {
return "", "", false, nil
}

scheme, value, ok := strings.Cut(auth, " ")

if !ok {
return "", "", false, errors.New("failed to parse http authorization header: invalid scheme: the scheme was missing")
}

if !strings.EqualFold(scheme, "Basic") {
return "", "", false, fmt.Errorf("failed to parse http authorization header: invalid scheme: expected the Basic scheme but received %s", scheme)
}

c, err := base64.StdEncoding.DecodeString(value)
if err != nil {
return "", "", false, fmt.Errorf("failed to parse http authorization header: invalid value: malformed base64 data: %w", err)
}

cs := string(c)

clientID, clientSecret, ok = strings.Cut(cs, ":")
if !ok {
return "", "", false, errors.New("failed to parse http authorization header: invalid value: the basic scheme separator was missing")
}

return clientID, clientSecret, true, nil
}

func clientCredentialsFromRequestBody(form url.Values, forceID bool) (clientID, clientSecret string, err error) {
Expand Down
16 changes: 11 additions & 5 deletions client_authentication_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func mustGenerateNoneAssertion(t *testing.T, claims jwt.MapClaims, key *rsa.Priv

// returns an http basic authorization header, encoded using application/x-www-form-urlencoded
func clientBasicAuthHeader(clientID, clientSecret string) http.Header {
creds := url.QueryEscape(clientID) + ":" + url.QueryEscape(clientSecret)
creds := clientID + ":" + clientSecret
return http.Header{
"Authorization": {
"Basic " + base64.StdEncoding.EncodeToString([]byte(creds)),
Expand Down Expand Up @@ -221,6 +221,12 @@ func TestAuthenticateClient(t *testing.T) {
form: url.Values{},
r: &http.Request{Header: clientBasicAuthHeader("foo", "bar")},
},
{
d: "should pass because client is confidential and id and secret match in header with special characters",
client: &DefaultOpenIDConnectClient{DefaultClient: &DefaultClient{ID: "foo", Secret: complexSecret}, TokenEndpointAuthMethod: "client_secret_basic"},
form: url.Values{},
r: &http.Request{Header: clientBasicAuthHeader("foo", "foo %66%6F%6F@$<§!✓")},
},
{
d: "should pass because client is confidential and id and rotated secret match in header",
client: &DefaultOpenIDConnectClient{DefaultClient: &DefaultClient{ID: "foo", Secret: []byte("invalid_hash"), RotatedSecrets: [][]byte{barSecret}}, TokenEndpointAuthMethod: "client_secret_basic"},
Expand Down Expand Up @@ -255,18 +261,18 @@ func TestAuthenticateClient(t *testing.T) {
expectErr: ErrInvalidClient,
},
{
d: "should fail because client id is not encoded using application/x-www-form-urlencoded",
d: "should fail because client id is not valid",
client: &DefaultOpenIDConnectClient{DefaultClient: &DefaultClient{ID: "foo", Secret: barSecret}, TokenEndpointAuthMethod: "client_secret_basic"},
form: url.Values{},
r: &http.Request{Header: http.Header{"Authorization": {"Basic " + base64.StdEncoding.EncodeToString([]byte("%%%%%%:foo"))}}},
expectErr: ErrInvalidRequest,
expectErr: ErrInvalidClient,
},
{
d: "should fail because client secret is not encoded using application/x-www-form-urlencoded",
d: "should fail because client secret is not valid",
client: &DefaultOpenIDConnectClient{DefaultClient: &DefaultClient{ID: "foo", Secret: barSecret}, TokenEndpointAuthMethod: "client_secret_basic"},
form: url.Values{},
r: &http.Request{Header: http.Header{"Authorization": {"Basic " + base64.StdEncoding.EncodeToString([]byte("foo:%%%%%%%"))}}},
expectErr: ErrInvalidRequest,
expectErr: ErrInvalidClient,
},
{
d: "should fail because client is confidential and id does not exist in header",
Expand Down

0 comments on commit fde4d25

Please sign in to comment.