unstaged

ory · Feb 7, 2018 · 59e89c9 · 59e89c9
1 parent 880126f
commit 59e89c9
Show file tree

Hide file tree

Showing 7 changed files with 219 additions and 6 deletions.
diff --git a/compose/compose_pkce.go b/compose/compose_pkce.go
@@ -10,7 +10,7 @@ func OAuth2PKCEFactory(config *Config, storage interface{}, strategy interface{}
 	return &pkce.Handler{
 		AuthorizeCodeStrategy: strategy.(oauth2.AuthorizeCodeStrategy),
 		CoreStorage:           storage.(oauth2.CoreStorage),
-		Force:                 !config.AllowPublicAuthCodeFlowWithoutPKCE,
-		EnablePlainChallengeMethod: !config.EnablePKCEPlainChallengeMethod,
+		Force:                 config.EnforcePKCE,
+		EnablePlainChallengeMethod: config.EnablePKCEPlainChallengeMethod,
 	}
 }
diff --git a/compose/config.go b/compose/config.go
@@ -44,8 +44,8 @@ type Config struct {
 	// ScopeStrategy sets the scope strategy that should be supported, for example fosite.WildcardScopeStrategy.
 	ScopeStrategy fosite.ScopeStrategy
 
-	// AllowPublicAuthCodeFlowWithoutPKCE, if set to true, allows public clients to perform authorize code flows without PKCE. Defaults to false.
-	AllowPublicAuthCodeFlowWithoutPKCE bool
+	// EnforcePKCE, if set to true, requires public clients to perform authorize code flows with PKCE. Defaults to false.
+	EnforcePKCE bool
 
 	// EnablePKCEPlainChallengeMethod sets whether or not to allow the plain challenge method (S256 should be used whenever possible, plain is really discouraged). Defaults to false.
 	EnablePKCEPlainChallengeMethod bool

diff --git a/handler/pkce/handler.go b/handler/pkce/handler.go
@@ -174,3 +174,7 @@ func (c *Handler) HandleTokenEndpointRequest(ctx context.Context, request fosite
 
 	return nil
 }
+
+func (c *Handler) PopulateTokenEndpointResponse(ctx context.Context, requester fosite.AccessRequester, responder fosite.AccessResponder) error {
+	return nil
+}
diff --git a/integration/authorize_code_grant_public_client_pkce_test.go b/integration/authorize_code_grant_public_client_pkce_test.go
@@ -0,0 +1,116 @@
+// Copyright © 2017 Aeneas Rekkas <aeneas+oss@aeneas.io>
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package integration_test
+
+import (
+	"testing"
+
+	"net/http"
+
+	"fmt"
+
+	"github.com/ory/fosite"
+	"github.com/ory/fosite/compose"
+	"github.com/ory/fosite/handler/oauth2"
+	//"github.com/stretchr/testify/assert"
+	"encoding/json"
+	"net/url"
+
+	"github.com/magiconair/properties/assert"
+	"github.com/stretchr/testify/require"
+	goauth "golang.org/x/oauth2"
+)
+
+func TestAuthorizeCodeFlowWithPublicClientAndPKCE(t *testing.T) {
+	for _, strategy := range []oauth2.AccessTokenStrategy{
+		hmacStrategy,
+	} {
+		runAuthorizeCodeGrantWithPublicClientAndPKCETest(t, strategy)
+	}
+}
+
+func runAuthorizeCodeGrantWithPublicClientAndPKCETest(t *testing.T, strategy interface{}) {
+	c := new(compose.Config)
+	c.EnforcePKCE = true
+	c.EnablePKCEPlainChallengeMethod = true
+	f := compose.Compose(c, fositeStore, strategy, nil, compose.OAuth2AuthorizeExplicitFactory, compose.OAuth2PKCEFactory, compose.OAuth2TokenIntrospectionFactory)
+	ts := mockServer(t, f, &fosite.DefaultSession{})
+	defer ts.Close()
+
+	oauthClient := newOAuth2Client(ts)
+	oauthClient.ClientSecret = ""
+	oauthClient.ClientID = "public-client"
+	fositeStore.Clients["public-client"].RedirectURIs[0] = ts.URL + "/callback"
+
+	var authCodeUrl string
+	var verifier string
+	for k, c := range []struct {
+		description    string
+		setup          func()
+		authStatusCode int
+	}{
+		{
+			description: "should fail because no challenge was given",
+			setup: func() {
+				authCodeUrl = oauthClient.AuthCodeURL("12345678901234567890")
+			},
+			authStatusCode: http.StatusNotAcceptable,
+		},
+		{
+			description: "should pass",
+			setup: func() {
+				verifier = "somechallenge"
+				authCodeUrl = oauthClient.AuthCodeURL("12345678901234567890") + "&code_challenge=somechallenge"
+			},
+			authStatusCode: http.StatusOK,
+		},
+	} {
+		t.Run(fmt.Sprintf("case=%d/description=%s", k, c.description), func(t *testing.T) {
+			c.setup()
+
+			t.Logf("Got url: %s", authCodeUrl)
+
+			resp, err := http.Get(authCodeUrl)
+			require.NoError(t, err)
+			require.Equal(t, c.authStatusCode, resp.StatusCode)
+
+			if resp.StatusCode == http.StatusOK {
+				// This should fail because no verifier was given
+				_, err := oauthClient.Exchange(goauth.NoContext, resp.Request.URL.Query().Get("code"))
+				require.Error(t, err)
+				//require.Empty(t, token.AccessToken)
+
+				resp, err := http.PostForm(ts.URL+"/token", url.Values{
+					"code":          {resp.Request.URL.Query().Get("code")},
+					"grant_type":    {"authorization_code"},
+					"client_id":     {"public-client"},
+					"redirect_uri":  {ts.URL + "/callback"},
+					"code_verifier": {verifier},
+				})
+				require.NoError(t, err)
+				defer resp.Body.Close()
+
+				assert.Equal(t, resp.StatusCode, http.StatusOK)
+				token := goauth.Token{}
+				require.NoError(t, json.NewDecoder(resp.Body).Decode(&token))
+
+				httpClient := oauthClient.Client(goauth.NoContext, &token)
+				resp, err = httpClient.Get(ts.URL + "/info")
+				require.NoError(t, err)
+				assert.Equal(t, http.StatusNoContent, resp.StatusCode)
+			}
+		})
+	}
+}
diff --git a/integration/authorize_code_grant_public_client_test.go b/integration/authorize_code_grant_public_client_test.go
@@ -0,0 +1,83 @@
+// Copyright © 2017 Aeneas Rekkas <aeneas+oss@aeneas.io>
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package integration_test
+
+import (
+	"testing"
+
+	"net/http"
+
+	"fmt"
+
+	"github.com/ory/fosite"
+	"github.com/ory/fosite/compose"
+	"github.com/ory/fosite/handler/oauth2"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	goauth "golang.org/x/oauth2"
+)
+
+func TestAuthorizeCodeFlowWithPublicClient(t *testing.T) {
+	for _, strategy := range []oauth2.AccessTokenStrategy{
+		hmacStrategy,
+	} {
+		runAuthorizeCodeGrantWithPublicClientTest(t, strategy)
+	}
+}
+
+func runAuthorizeCodeGrantWithPublicClientTest(t *testing.T, strategy interface{}) {
+	f := compose.Compose(new(compose.Config), fositeStore, strategy, nil, compose.OAuth2AuthorizeExplicitFactory, compose.OAuth2TokenIntrospectionFactory)
+	ts := mockServer(t, f, &fosite.DefaultSession{})
+	defer ts.Close()
+
+	oauthClient := newOAuth2Client(ts)
+	oauthClient.ClientSecret = ""
+	oauthClient.ClientID = "public-client"
+	fositeStore.Clients["public-client"].RedirectURIs[0] = ts.URL + "/callback"
+
+	var state string
+	for k, c := range []struct {
+		description    string
+		setup          func()
+		authStatusCode int
+	}{
+		{
+			description: "should pass",
+			setup: func() {
+				state = "12345678901234567890"
+			},
+			authStatusCode: http.StatusOK,
+		},
+	} {
+		t.Run(fmt.Sprintf("case=%d/description=%s", k, c.description), func(t *testing.T) {
+			c.setup()
+
+			resp, err := http.Get(oauthClient.AuthCodeURL(state))
+			require.NoError(t, err)
+			require.Equal(t, c.authStatusCode, resp.StatusCode)
+
+			if resp.StatusCode == http.StatusOK {
+				token, err := oauthClient.Exchange(goauth.NoContext, resp.Request.URL.Query().Get("code"))
+				require.NoError(t, err)
+				require.NotEmpty(t, token.AccessToken)
+
+				httpClient := oauthClient.Client(goauth.NoContext, token)
+				resp, err := httpClient.Get(ts.URL + "/info")
+				require.NoError(t, err)
+				assert.Equal(t, http.StatusNoContent, resp.StatusCode)
+			}
+		})
+	}
+}
diff --git a/integration/helper_endpoints_test.go b/integration/helper_endpoints_test.go
@@ -97,9 +97,10 @@ func authEndpointHandler(t *testing.T, oauth2 fosite.OAuth2Provider, session fos
 
 		response, err := oauth2.NewAuthorizeResponse(ctx, ar, session)
 		if err != nil {
-			t.Logf("Access request failed because %s.", err.Error())
+			ca := errors.Cause(err).(*fosite.RFC6749Error)
+			t.Logf("Access request failed because %s - %s - %s.", err, ca.Description, ca.Debug)
 			t.Logf("Request: %s.", ar)
-			// t.Logf("Stack: %s.", err.(stackTracer).StackTrace())
+			t.Logf("Stack: %+v.", err.(stackTracer).StackTrace())
 			oauth2.WriteAuthorizeError(rw, ar, err)
 			return
 		}

diff --git a/integration/helper_setup_test.go b/integration/helper_setup_test.go
@@ -41,6 +41,15 @@ var fositeStore = &storage.MemoryStore{
 			GrantTypes:    []string{"implicit", "refresh_token", "authorization_code", "password", "client_credentials"},
 			Scopes:        []string{"fosite", "offline", "openid"},
 		},
+		"public-client": {
+			ID:            "public-client",
+			Secret:        []byte{},
+			Public:        true,
+			RedirectURIs:  []string{"http://localhost:3846/callback"},
+			ResponseTypes: []string{"id_token", "code"},
+			GrantTypes:    []string{"refresh_token", "authorization_code"},
+			Scopes:        []string{"fosite", "offline", "openid"},
+		},
 	},
 	Users: map[string]storage.MemoryUserRelation{
 		"peter": {