Merge branch 'master' into fix-oidc-redirect-uri

.github/ISSUE_TEMPLATE/BUG-REPORT.yml

@@ -21,12 +21,17 @@ body:
         - label: "I have read and am following this repository's [Contribution
             Guidelines](https://github.com/ory/fosite/blob/master/CONTRIBUTING.md)."
           required: true
-        - label: "This issue affects my [Ory Network](https://www.ory.sh/) project."
         - label: "I have joined the [Ory Community Slack](https://slack.ory.sh)."
         - label: "I am signed up to the [Ory Security Patch
             Newsletter](https://ory.us10.list-manage.com/subscribe?u=ffb1a878e4ec6c0ed312a3480&id=f605a41b53)."
     id: checklist
     type: checkboxes
+  - attributes:
+      description: "Enter the slug or API URL of the affected Ory Network project. Leave empty when you are self-hosting."
+      label: "Ory Network Project"
+      placeholder: "https://<your-project-slug>.projects.oryapis.com"
+    id: ory-network-project
+    type: input
   - attributes:
       description: "A clear and concise description of what the bug is."
       label: "Describe the bug"

.github/ISSUE_TEMPLATE/DESIGN-DOC.yml

@@ -31,12 +31,17 @@ body:
         - label: "I have read and am following this repository's [Contribution
             Guidelines](https://github.com/ory/fosite/blob/master/CONTRIBUTING.md)."
           required: true
-        - label: "This issue affects my [Ory Network](https://www.ory.sh/) project."
         - label: "I have joined the [Ory Community Slack](https://slack.ory.sh)."
         - label: "I am signed up to the [Ory Security Patch
             Newsletter](https://ory.us10.list-manage.com/subscribe?u=ffb1a878e4ec6c0ed312a3480&id=f605a41b53)."
     id: checklist
     type: checkboxes
+  - attributes:
+      description: "Enter the slug or API URL of the affected Ory Network project. Leave empty when you are self-hosting."
+      label: "Ory Network Project"
+      placeholder: "https://<your-project-slug>.projects.oryapis.com"
+    id: ory-network-project
+    type: input
   - attributes:
       description: |
         This section gives the reader a very rough overview of the landscape in which the new system is being built and what is actually being built. This isn’t a requirements doc. Keep it succinct! The goal is that readers are brought up to speed but some previous knowledge can be assumed and detailed info can be linked to. This section should be entirely focused on objective background facts.

.github/ISSUE_TEMPLATE/FEATURE-REQUEST.yml

@@ -24,12 +24,17 @@ body:
         - label: "I have read and am following this repository's [Contribution
             Guidelines](https://github.com/ory/fosite/blob/master/CONTRIBUTING.md)."
           required: true
-        - label: "This issue affects my [Ory Network](https://www.ory.sh/) project."
         - label: "I have joined the [Ory Community Slack](https://slack.ory.sh)."
         - label: "I am signed up to the [Ory Security Patch
             Newsletter](https://ory.us10.list-manage.com/subscribe?u=ffb1a878e4ec6c0ed312a3480&id=f605a41b53)."
     id: checklist
     type: checkboxes
+  - attributes:
+      description: "Enter the slug or API URL of the affected Ory Network project. Leave empty when you are self-hosting."
+      label: "Ory Network Project"
+      placeholder: "https://<your-project-slug>.projects.oryapis.com"
+    id: ory-network-project
+    type: input
   - attributes:
       description: "Is your feature request related to a problem? Please describe."
       label: "Describe your problem"

.github/workflows/format.yml

@@ -11,7 +11,7 @@ jobs:
       - uses: actions/checkout@v3
       - uses: actions/setup-go@v3
         with:
-          go-version: 1.19
+          go-version: "1.20"
       - run: make format
       - name: Indicate formatting issues
         run: git diff HEAD --exit-code --color

.github/workflows/licenses.yml

@@ -14,7 +14,7 @@ jobs:
       - uses: actions/checkout@v2
       - uses: actions/setup-go@v2
         with:
-          go-version: "1.18"
+          go-version: "1.20"
       - uses: actions/setup-node@v2
         with:
           node-version: "18"

.github/workflows/oidc-conformity-master.yml

@@ -17,7 +17,7 @@ jobs:
           ref: master
       - uses: actions/setup-go@v2
         with:
-          go-version: "^1.19.0"
+          go-version: "1.20"
       - name: Update fosite
         run: |
           go mod edit -replace github.com/ory/fosite=github.com/ory/fosite@${{ github.sha }}

.github/workflows/oidc-conformity.yml

@@ -17,7 +17,7 @@ jobs:
           ref: master
       - uses: actions/setup-go@v2
         with:
-          go-version: "^1.19.0"
+          go-version: "1.20"
       - name: Update fosite
         run: |
           go mod edit -replace github.com/ory/fosite=github.com/${{ github.event.pull_request.head.repo.full_name }}@${{ github.event.pull_request.head.sha }}

.github/workflows/test.yml

@@ -11,5 +11,5 @@ jobs:
       - uses: actions/checkout@v3
       - uses: actions/setup-go@v3
         with:
-          go-version: 1.19
+          go-version: "1.20"
       - run: make test

access_request_handler.go

@@ -10,6 +10,8 @@ import (
 
 	"github.com/ory/fosite/i18n"
 	"github.com/ory/x/errorsx"
+	"github.com/ory/x/otelx"
+	"go.opentelemetry.io/otel/trace"
 
 	"github.com/pkg/errors"
 )
@@ -39,7 +41,10 @@ import (
 //     credentials (or assigned other authentication requirements), the
 //     client MUST authenticate with the authorization server as described
 //     in Section 3.2.1.
-func (f *Fosite) NewAccessRequest(ctx context.Context, r *http.Request, session Session) (AccessRequester, error) {
+func (f *Fosite) NewAccessRequest(ctx context.Context, r *http.Request, session Session) (_ AccessRequester, err error) {
+	ctx, span := trace.SpanFromContext(ctx).TracerProvider().Tracer("github.com/ory/fosite").Start(ctx, "Fosite.NewAccessRequest")
+	defer otelx.End(span, &err)
+
 	accessRequest := NewAccessRequest(session)
 	accessRequest.Request.Lang = i18n.GetLangFromRequest(f.Config.GetMessageCatalog(ctx), r)
 

access_request_handler_test.go

@@ -4,7 +4,6 @@
 package fosite_test
 
 import (
-	"context"
 	"encoding/base64"
 	"fmt"
 	"net/http"
@@ -29,8 +28,6 @@ func TestNewAccessRequest(t *testing.T) {
 	hasher := internal.NewMockHasher(ctrl)
 	defer ctrl.Finish()
 
-	ctx := gomock.AssignableToTypeOf(context.WithValue(context.TODO(), ContextKey("test"), nil))
-
 	client := &DefaultClient{}
 	config := &Config{ClientSecretsHasher: hasher, AudienceMatchingStrategy: DefaultAudienceMatchingStrategy}
 	fosite := &Fosite{Store: store, Config: config}
@@ -121,7 +118,7 @@ func TestNewAccessRequest(t *testing.T) {
 				store.EXPECT().GetClient(gomock.Any(), gomock.Eq("foo")).Return(client, nil)
 				client.Public = false
 				client.Secret = []byte("foo")
-				hasher.EXPECT().Compare(ctx, gomock.Eq([]byte("foo")), gomock.Eq([]byte("bar"))).Return(errors.New(""))
+				hasher.EXPECT().Compare(gomock.Any(), gomock.Eq([]byte("foo")), gomock.Eq([]byte("bar"))).Return(errors.New(""))
 			},
 			handlers: TokenEndpointHandlers{handler},
 		},
@@ -138,7 +135,7 @@ func TestNewAccessRequest(t *testing.T) {
 				store.EXPECT().GetClient(gomock.Any(), gomock.Eq("foo")).Return(client, nil)
 				client.Public = false
 				client.Secret = []byte("foo")
-				hasher.EXPECT().Compare(ctx, gomock.Eq([]byte("foo")), gomock.Eq([]byte("bar"))).Return(nil)
+				hasher.EXPECT().Compare(gomock.Any(), gomock.Eq([]byte("foo")), gomock.Eq([]byte("bar"))).Return(nil)
 				handler.EXPECT().HandleTokenEndpointRequest(gomock.Any(), gomock.Any()).Return(ErrServerError)
 			},
 			handlers: TokenEndpointHandlers{handler},
@@ -155,7 +152,7 @@ func TestNewAccessRequest(t *testing.T) {
 				store.EXPECT().GetClient(gomock.Any(), gomock.Eq("foo")).Return(client, nil)
 				client.Public = false
 				client.Secret = []byte("foo")
-				hasher.EXPECT().Compare(ctx, gomock.Eq([]byte("foo")), gomock.Eq([]byte("bar"))).Return(nil)
+				hasher.EXPECT().Compare(gomock.Any(), gomock.Eq([]byte("foo")), gomock.Eq([]byte("bar"))).Return(nil)
 				handler.EXPECT().HandleTokenEndpointRequest(gomock.Any(), gomock.Any()).Return(nil)
 			},
 			handlers: TokenEndpointHandlers{handler},
@@ -355,8 +352,6 @@ func TestNewAccessRequestWithMixedClientAuth(t *testing.T) {
 	hasher := internal.NewMockHasher(ctrl)
 	defer ctrl.Finish()
 
-	ctx := gomock.AssignableToTypeOf(context.WithValue(context.TODO(), ContextKey("test"), nil))
-
 	client := &DefaultClient{}
 	config := &Config{ClientSecretsHasher: hasher, AudienceMatchingStrategy: DefaultAudienceMatchingStrategy}
 	fosite := &Fosite{Store: store, Config: config}
@@ -380,7 +375,7 @@ func TestNewAccessRequestWithMixedClientAuth(t *testing.T) {
 				store.EXPECT().GetClient(gomock.Any(), gomock.Eq("foo")).Return(client, nil)
 				client.Public = false
 				client.Secret = []byte("foo")
-				hasher.EXPECT().Compare(ctx, gomock.Eq([]byte("foo")), gomock.Eq([]byte("bar"))).Return(errors.New("hash err"))
+				hasher.EXPECT().Compare(gomock.Any(), gomock.Eq([]byte("foo")), gomock.Eq([]byte("bar"))).Return(errors.New("hash err"))
 				handlerWithoutClientAuth.EXPECT().HandleTokenEndpointRequest(gomock.Any(), gomock.Any()).Return(nil)
 			},
 			method:    "POST",
@@ -398,7 +393,7 @@ func TestNewAccessRequestWithMixedClientAuth(t *testing.T) {
 				store.EXPECT().GetClient(gomock.Any(), gomock.Eq("foo")).Return(client, nil)
 				client.Public = false
 				client.Secret = []byte("foo")
-				hasher.EXPECT().Compare(ctx, gomock.Eq([]byte("foo")), gomock.Eq([]byte("bar"))).Return(nil)
+				hasher.EXPECT().Compare(gomock.Any(), gomock.Eq([]byte("foo")), gomock.Eq([]byte("bar"))).Return(nil)
 				handlerWithoutClientAuth.EXPECT().HandleTokenEndpointRequest(gomock.Any(), gomock.Any()).Return(nil)
 				handlerWithClientAuth.EXPECT().HandleTokenEndpointRequest(gomock.Any(), gomock.Any()).Return(nil)
 			},

access_response_writer.go

@@ -7,12 +7,16 @@ import (
 	"context"
 
 	"github.com/ory/x/errorsx"
+	"github.com/ory/x/otelx"
+	"go.opentelemetry.io/otel/trace"
 
 	"github.com/pkg/errors"
 )
 
-func (f *Fosite) NewAccessResponse(ctx context.Context, requester AccessRequester) (AccessResponder, error) {
-	var err error
+func (f *Fosite) NewAccessResponse(ctx context.Context, requester AccessRequester) (_ AccessResponder, err error) {
+	ctx, span := trace.SpanFromContext(ctx).TracerProvider().Tracer("github.com/ory/fosite").Start(ctx, "Fosite.NewAccessResponse")
+	defer otelx.End(span, &err)
+
 	var tk TokenEndpointHandler
 
 	response := NewAccessResponse()

authorize_helper.go

@@ -141,11 +141,14 @@ func isMatchingAsLoopback(requested *url.URL, registeredURI string) bool {
 	return false
 }
 
+var (
+	regexLoopbackAddress = regexp.MustCompile(`^(127\.0\.0\.1|\[::1])(:\d+)?$`)
+)
+
 // Check if address is either an IPv4 loopback or an IPv6 loopback-
 // An optional port is ignored
 func isLoopbackAddress(address string) bool {
-	match, _ := regexp.MatchString("^(127.0.0.1|\\[::1\\])(:?)(\\d*)$", address)
-	return match
+	return regexLoopbackAddress.MatchString(address)
 }
 
 // IsValidRedirectURI validates a redirect_uri as specified in:

authorize_helper_test.go

@@ -6,7 +6,7 @@ package fosite_test
 import (
 	"bytes"
 	"context"
-	"io/ioutil"
+	"io"
 	"net/url"
 	"strings"
 	"testing"
@@ -284,7 +284,7 @@ func TestWriteAuthorizeFormPostResponse(t *testing.T) {
 		redirectURL := "https://localhost:8080/cb"
 		//parameters :=
 		fosite.WriteAuthorizeFormPostResponse(redirectURL, c.parameters, fosite.DefaultFormPostTemplate, &responseBuffer)
-		code, state, _, _, customParams, _, err := internal.ParseFormPostResponse(redirectURL, ioutil.NopCloser(bytes.NewReader(responseBuffer.Bytes())))
+		code, state, _, _, customParams, _, err := internal.ParseFormPostResponse(redirectURL, io.NopCloser(bytes.NewReader(responseBuffer.Bytes())))
 		assert.NoError(t, err, "case %d", d)
 		c.check(code, state, customParams, d)
 

authorize_helper_whitebox_test.go

@@ -0,0 +1,69 @@
+// Copyright © 2023 Ory Corp
+// SPDX-License-Identifier: Apache-2.0
+
+package fosite
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestIsLookbackAddress(t *testing.T) {
+	testCases := []struct {
+		name     string
+		have     string
+		expected bool
+	}{
+		{
+			"ShouldReturnTrueIPv4Loopback",
+			"127.0.0.1",
+			true,
+		},
+		{
+			"ShouldReturnTrueIPv4LoopbackWithPort",
+			"127.0.0.1:1230",
+			true,
+		},
+		{
+			"ShouldReturnTrueIPv6Loopback",
+			"[::1]",
+			true,
+		},
+		{
+			"ShouldReturnTrueIPv6LoopbackWithPort",
+			"[::1]:1230",
+			true,
+		}, {
+			"ShouldReturnFalse12700255",
+			"127.0.0.255",
+			false,
+		},
+		{
+			"ShouldReturnFalse12700255WithPort",
+			"127.0.0.255:1230",
+			false,
+		},
+		{
+			"ShouldReturnFalseInvalidFourthOctet",
+			"127.0.0.11230",
+			false,
+		},
+		{
+			"ShouldReturnFalseInvalidIPv4",
+			"127x0x0x11230",
+			false,
+		},
+		{
+			"ShouldReturnFalseInvalidIPv6",
+			"[::1]1230",
+			false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			assert.Equal(t, tc.expected, isLoopbackAddress(tc.have))
+		})
+	}
+}

authorize_request_handler.go

@@ -6,15 +6,17 @@ package fosite
 import (
 	"context"
 	"fmt"
-	"io/ioutil"
+	"io"
 	"net/http"
 	"strings"
 
 	"github.com/go-jose/go-jose/v3"
+	"go.opentelemetry.io/otel/trace"
 
 	"github.com/ory/fosite/i18n"
 	"github.com/ory/fosite/token/jwt"
 	"github.com/ory/x/errorsx"
+	"github.com/ory/x/otelx"
 
 	"github.com/pkg/errors"
 
@@ -74,7 +76,7 @@ func (f *Fosite) authorizeRequestParametersFromOpenIDConnectRequest(ctx context.
 			return errorsx.WithStack(ErrInvalidRequestURI.WithHintf("Unable to fetch OpenID Connect request parameters from 'request_uri' because status code '%d' was expected, but got '%d'.", http.StatusOK, response.StatusCode))
 		}
 
-		body, err := ioutil.ReadAll(response.Body)
+		body, err := io.ReadAll(response.Body)
 		if err != nil {
 			return errorsx.WithStack(ErrInvalidRequestURI.WithHintf("Unable to fetch OpenID Connect request parameters from 'request_uri' because body parsing failed with: %s.", err).WithWrap(err).WithDebug(err.Error()))
 		}
@@ -321,7 +323,10 @@ func (f *Fosite) authorizeRequestFromPAR(ctx context.Context, r *http.Request, r
 	return true, nil
 }
 
-func (f *Fosite) NewAuthorizeRequest(ctx context.Context, r *http.Request) (AuthorizeRequester, error) {
+func (f *Fosite) NewAuthorizeRequest(ctx context.Context, r *http.Request) (_ AuthorizeRequester, err error) {
+	ctx, span := trace.SpanFromContext(ctx).TracerProvider().Tracer("github.com/ory/fosite").Start(ctx, "Fosite.NewAuthorizeRequest")
+	defer otelx.End(span, &err)
+
 	return f.newAuthorizeRequest(ctx, r, false)
 }
 

authorize_response_writer.go

@@ -9,9 +9,14 @@ import (
 	"net/url"
 
 	"github.com/ory/x/errorsx"
+	"github.com/ory/x/otelx"
+	"go.opentelemetry.io/otel/trace"
 )
 
-func (f *Fosite) NewAuthorizeResponse(ctx context.Context, ar AuthorizeRequester, session Session) (AuthorizeResponder, error) {
+func (f *Fosite) NewAuthorizeResponse(ctx context.Context, ar AuthorizeRequester, session Session) (_ AuthorizeResponder, err error) {
+	ctx, span := trace.SpanFromContext(ctx).TracerProvider().Tracer("github.com/ory/fosite").Start(ctx, "Fosite.NewAuthorizeResponse")
+	defer otelx.End(span, &err)
+
 	var resp = &AuthorizeResponse{
 		Header:     http.Header{},
 		Parameters: url.Values{},

client_authentication_test.go

@@ -115,8 +115,7 @@ func TestAuthenticateClient(t *testing.T) {
 		},
 	}
 
-	var h http.HandlerFunc
-	h = func(w http.ResponseWriter, r *http.Request) {
+	var h http.HandlerFunc = func(w http.ResponseWriter, r *http.Request) {
 		require.NoError(t, json.NewEncoder(w).Encode(rsaJwks))
 	}
 	ts := httptest.NewServer(h)
@@ -586,12 +585,12 @@ func TestAuthenticateClientTwice(t *testing.T) {
 		"aud": "token-url",
 	}, key, "kid-foo")}, "client_assertion_type": []string{at}}
 
-	c, err := f.AuthenticateClient(nil, new(http.Request), formValues)
+	c, err := f.AuthenticateClient(context.Background(), new(http.Request), formValues)
 	require.NoError(t, err, "%#v", err)
 	assert.Equal(t, client, c)
 
 	// replay the request and expect it to fail
-	c, err = f.AuthenticateClient(nil, new(http.Request), formValues)
+	c, err = f.AuthenticateClient(context.Background(), new(http.Request), formValues)
 	require.Error(t, err)
 	assert.EqualError(t, err, ErrJTIKnown.Error())
 	assert.Nil(t, c)