From 65dcd1a5e7932049e588a5390b0d497808f38a53 Mon Sep 17 00:00:00 2001 From: Jonas Franz Date: Sat, 9 Mar 2019 21:11:52 +0100 Subject: [PATCH 1/3] Add support for client basic auth for exchanging access tokens --- integrations/oauth_test.go | 60 ++++++++++++++++++++++++++++++++++++++ routers/user/oauth.go | 26 +++++++++++++++++ 2 files changed, 86 insertions(+) diff --git a/integrations/oauth_test.go b/integrations/oauth_test.go index 53b83bb01a43a..2e6eac38dd02b 100644 --- a/integrations/oauth_test.go +++ b/integrations/oauth_test.go @@ -5,10 +5,12 @@ package integrations import ( + "context" "encoding/json" "testing" "github.com/stretchr/testify/assert" + "golang.org/x/oauth2" ) const defaultAuthorize = "/login/oauth/authorize?client_id=da7da3ba-9a13-4167-856f-3899de0b0138&redirect_uri=a&response_type=code&state=thestate" @@ -136,3 +138,61 @@ func TestAccessTokenExchangeWithInvalidCredentials(t *testing.T) { }) MakeRequest(t, req, 400) } + +func TestAccessTokenExchangeWithBasicAuth(t *testing.T) { + prepareTestEnv(t) + req := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + "grant_type": "authorization_code", + "redirect_uri": "a", + "code": "authcode", + "code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt", // test PKCE additionally + }) + req.Header.Set("Authorization", "Basic ZGE3ZGEzYmEtOWExMy00MTY3LTg1NmYtMzg5OWRlMGIwMTM4OjRNSzhOYTZSNTVzbWRDWTBXdUNDdW1aNmhqUlBuR1k1c2FXVlJISGpKaUE9") + resp := MakeRequest(t, req, 200) + type response struct { + AccessToken string `json:"access_token"` + TokenType string `json:"token_type"` + ExpiresIn int64 `json:"expires_in"` + RefreshToken string `json:"refresh_token"` + } + parsed := new(response) + assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsed)) + assert.True(t, len(parsed.AccessToken) > 10) + assert.True(t, len(parsed.RefreshToken) > 10) + + // use wrong client_secret + req = NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + "grant_type": "authorization_code", + "redirect_uri": "a", + "code": "authcode", + "code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt", // test PKCE additionally + }) + req.Header.Set("Authorization", "Basic ZGE3ZGEzYmEtOWExMy00MTY3LTg1NmYtMzg5OWRlMGIwMTM4OmJsYWJsYQ==") + resp = MakeRequest(t, req, 400) + + // missing header + req = NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + "grant_type": "authorization_code", + "redirect_uri": "a", + "code": "authcode", + "code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt", // test PKCE additionally + }) + resp = MakeRequest(t, req, 400) +} + +func TestAccessTokenWithGolangClient(t *testing.T) { + config := &oauth2.Config{ + RedirectURL: "a", + ClientID: "da7da3ba-9a13-4167-856f-3899de0b0138", + ClientSecret: "4MK8Na6R55smdCY0WuCCumZ6hjRPnGY5saWVRHHjJiA=", + Endpoint: oauth2.Endpoint{ + AuthURL: "/login/oauth/authorize", + TokenURL: "/login/oauth/access_token", + }, + Scopes: []string{}, + } + token, err := config.Exchange(context.Background(), "authcode") + assert.NoError(t, err) + assert.True(t, len(token.AccessToken) > 10) + assert.True(t, len(token.RefreshToken) > 10) +} diff --git a/routers/user/oauth.go b/routers/user/oauth.go index dbb3c4a39166b..fc5d308062ea6 100644 --- a/routers/user/oauth.go +++ b/routers/user/oauth.go @@ -5,8 +5,10 @@ package user import ( + "encoding/base64" "fmt" "net/url" + "strings" "github.com/dgrijalva/jwt-go" "github.com/go-macaron/binding" @@ -357,6 +359,30 @@ func handleRefreshToken(ctx *context.Context, form auth.AccessTokenForm) { } func handleAuthorizationCode(ctx *context.Context, form auth.AccessTokenForm) { + if form.ClientID == "" && form.ClientSecret == "" { + authHeader := ctx.Header().Get("Authorization") + authContent := strings.SplitN(authHeader, " ", 2) + if len(authContent) == 2 && authContent[0] == "Basic" { + payload, err := base64.StdEncoding.DecodeString(authContent[1]) + if err != nil { + handleAccessTokenError(ctx, AccessTokenError{ + ErrorCode: AccessTokenErrorCodeInvalidRequest, + ErrorDescription: "cannot parse basic auth header", + }) + return + } + pair := strings.SplitN(string(payload), ":", 2) + if len(pair) != 2 { + handleAccessTokenError(ctx, AccessTokenError{ + ErrorCode: AccessTokenErrorCodeInvalidRequest, + ErrorDescription: "cannot parse basic auth header", + }) + return + } + form.ClientID = pair[0] + form.ClientSecret = pair[1] + } + } app, err := models.GetOAuth2ApplicationByClientID(form.ClientID) if err != nil { handleAccessTokenError(ctx, AccessTokenError{ From 74499f3d25e8c28d7ad98338d02a48d07931fa86 Mon Sep 17 00:00:00 2001 From: Jonas Franz Date: Sat, 9 Mar 2019 21:22:25 +0100 Subject: [PATCH 2/3] Improve error messages --- routers/user/oauth.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/user/oauth.go b/routers/user/oauth.go index fc5d308062ea6..c967305d0df09 100644 --- a/routers/user/oauth.go +++ b/routers/user/oauth.go @@ -387,7 +387,7 @@ func handleAuthorizationCode(ctx *context.Context, form auth.AccessTokenForm) { if err != nil { handleAccessTokenError(ctx, AccessTokenError{ ErrorCode: AccessTokenErrorCodeInvalidClient, - ErrorDescription: "cannot load client", + ErrorDescription: fmt.Sprintf("cannot load client with client id: '%s'", form.ClientID), }) return } From b16a00422e03391942e5be14354baa9114a3f65f Mon Sep 17 00:00:00 2001 From: Jonas Franz Date: Sun, 10 Mar 2019 13:02:52 +0100 Subject: [PATCH 3/3] Fix tests --- integrations/oauth_test.go | 23 ++---------------- routers/user/oauth.go | 48 +++++++++++++++++++------------------- 2 files changed, 26 insertions(+), 45 deletions(-) diff --git a/integrations/oauth_test.go b/integrations/oauth_test.go index 2e6eac38dd02b..9674146f8ba21 100644 --- a/integrations/oauth_test.go +++ b/integrations/oauth_test.go @@ -5,12 +5,10 @@ package integrations import ( - "context" "encoding/json" "testing" "github.com/stretchr/testify/assert" - "golang.org/x/oauth2" ) const defaultAuthorize = "/login/oauth/authorize?client_id=da7da3ba-9a13-4167-856f-3899de0b0138&redirect_uri=a&response_type=code&state=thestate" @@ -147,7 +145,7 @@ func TestAccessTokenExchangeWithBasicAuth(t *testing.T) { "code": "authcode", "code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt", // test PKCE additionally }) - req.Header.Set("Authorization", "Basic ZGE3ZGEzYmEtOWExMy00MTY3LTg1NmYtMzg5OWRlMGIwMTM4OjRNSzhOYTZSNTVzbWRDWTBXdUNDdW1aNmhqUlBuR1k1c2FXVlJISGpKaUE9") + req.Header.Add("Authorization", "Basic ZGE3ZGEzYmEtOWExMy00MTY3LTg1NmYtMzg5OWRlMGIwMTM4OjRNSzhOYTZSNTVzbWRDWTBXdUNDdW1aNmhqUlBuR1k1c2FXVlJISGpKaUE9") resp := MakeRequest(t, req, 200) type response struct { AccessToken string `json:"access_token"` @@ -167,7 +165,7 @@ func TestAccessTokenExchangeWithBasicAuth(t *testing.T) { "code": "authcode", "code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt", // test PKCE additionally }) - req.Header.Set("Authorization", "Basic ZGE3ZGEzYmEtOWExMy00MTY3LTg1NmYtMzg5OWRlMGIwMTM4OmJsYWJsYQ==") + req.Header.Add("Authorization", "Basic ZGE3ZGEzYmEtOWExMy00MTY3LTg1NmYtMzg5OWRlMGIwMTM4OmJsYWJsYQ==") resp = MakeRequest(t, req, 400) // missing header @@ -179,20 +177,3 @@ func TestAccessTokenExchangeWithBasicAuth(t *testing.T) { }) resp = MakeRequest(t, req, 400) } - -func TestAccessTokenWithGolangClient(t *testing.T) { - config := &oauth2.Config{ - RedirectURL: "a", - ClientID: "da7da3ba-9a13-4167-856f-3899de0b0138", - ClientSecret: "4MK8Na6R55smdCY0WuCCumZ6hjRPnGY5saWVRHHjJiA=", - Endpoint: oauth2.Endpoint{ - AuthURL: "/login/oauth/authorize", - TokenURL: "/login/oauth/access_token", - }, - Scopes: []string{}, - } - token, err := config.Exchange(context.Background(), "authcode") - assert.NoError(t, err) - assert.True(t, len(token.AccessToken) > 10) - assert.True(t, len(token.RefreshToken) > 10) -} diff --git a/routers/user/oauth.go b/routers/user/oauth.go index c967305d0df09..110fa93b3dc7d 100644 --- a/routers/user/oauth.go +++ b/routers/user/oauth.go @@ -307,6 +307,30 @@ func GrantApplicationOAuth(ctx *context.Context, form auth.GrantApplicationForm) // AccessTokenOAuth manages all access token requests by the client func AccessTokenOAuth(ctx *context.Context, form auth.AccessTokenForm) { + if form.ClientID == "" { + authHeader := ctx.Req.Header.Get("Authorization") + authContent := strings.SplitN(authHeader, " ", 2) + if len(authContent) == 2 && authContent[0] == "Basic" { + payload, err := base64.StdEncoding.DecodeString(authContent[1]) + if err != nil { + handleAccessTokenError(ctx, AccessTokenError{ + ErrorCode: AccessTokenErrorCodeInvalidRequest, + ErrorDescription: "cannot parse basic auth header", + }) + return + } + pair := strings.SplitN(string(payload), ":", 2) + if len(pair) != 2 { + handleAccessTokenError(ctx, AccessTokenError{ + ErrorCode: AccessTokenErrorCodeInvalidRequest, + ErrorDescription: "cannot parse basic auth header", + }) + return + } + form.ClientID = pair[0] + form.ClientSecret = pair[1] + } + } switch form.GrantType { case "refresh_token": handleRefreshToken(ctx, form) @@ -359,30 +383,6 @@ func handleRefreshToken(ctx *context.Context, form auth.AccessTokenForm) { } func handleAuthorizationCode(ctx *context.Context, form auth.AccessTokenForm) { - if form.ClientID == "" && form.ClientSecret == "" { - authHeader := ctx.Header().Get("Authorization") - authContent := strings.SplitN(authHeader, " ", 2) - if len(authContent) == 2 && authContent[0] == "Basic" { - payload, err := base64.StdEncoding.DecodeString(authContent[1]) - if err != nil { - handleAccessTokenError(ctx, AccessTokenError{ - ErrorCode: AccessTokenErrorCodeInvalidRequest, - ErrorDescription: "cannot parse basic auth header", - }) - return - } - pair := strings.SplitN(string(payload), ":", 2) - if len(pair) != 2 { - handleAccessTokenError(ctx, AccessTokenError{ - ErrorCode: AccessTokenErrorCodeInvalidRequest, - ErrorDescription: "cannot parse basic auth header", - }) - return - } - form.ClientID = pair[0] - form.ClientSecret = pair[1] - } - } app, err := models.GetOAuth2ApplicationByClientID(form.ClientID) if err != nil { handleAccessTokenError(ctx, AccessTokenError{