fix: response handling for session store (bearer + cookie)

middleware/grpc_middleware_test.go

@@ -9,11 +9,6 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"io"
-	"net"
-	"net/http"
-	"net/http/httptest"
-	"os"
 	"testing"
 	"time"
 
@@ -22,10 +17,8 @@ import (
 	"github.com/stretchr/testify/require"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/credentials/insecure"
 	"google.golang.org/grpc/status"
 	"google.golang.org/grpc/test/bufconn"
-	"google.golang.org/grpc/test/grpc_testing"
 	grpcTesting "google.golang.org/grpc/test/grpc_testing"
 
 	"github.com/ory/oathkeeper/driver"
@@ -34,59 +27,6 @@ import (
 	"github.com/ory/oathkeeper/rule"
 )
 
-func testClient(t *testing.T, l *bufconn.Listener, dialOpts ...grpc.DialOption) grpcTesting.TestServiceClient {
-	conn, err := grpc.Dial("bufnet",
-		append(dialOpts,
-			grpc.WithTransportCredentials(insecure.NewCredentials()),
-			grpc.WithAuthority("myproject.apis.ory.sh"),
-			grpc.WithContextDialer(func(context.Context, string) (net.Conn, error) { return l.Dial() }),
-		)...,
-	)
-	require.NoError(t, err)
-	t.Cleanup(func() { conn.Close() })
-
-	return grpcTesting.NewTestServiceClient(conn)
-}
-
-func testTokenCheckServer(t *testing.T) *httptest.Server {
-	s := httptest.NewServer(http.HandlerFunc(
-		func(w http.ResponseWriter, r *http.Request) {
-			if r.Header.Get("authorization") != "Bearer correct token" {
-				t.Logf("denied request %+v", r)
-				w.WriteHeader(http.StatusForbidden)
-				return
-			}
-			t.Logf("allowed request %+v", r)
-			io.WriteString(w, "{}")
-		}))
-	t.Cleanup(s.Close)
-	return s
-}
-
-func writeTestConfig(t *testing.T, pattern string, content string) string {
-	f, err := os.CreateTemp(t.TempDir(), pattern)
-	if err != nil {
-		t.Error(err)
-		return ""
-	}
-	defer f.Close()
-	io.WriteString(f, content)
-
-	return f.Name()
-}
-
-type testToken string
-
-func (t testToken) GetRequestMetadata(context.Context, ...string) (map[string]string, error) {
-	return map[string]string{"authorization": "Bearer " + string(t)}, nil
-}
-func (t testToken) RequireTransportSecurity() bool { return false }
-
-type upstream struct {
-	*MockTestServiceServer
-	grpc_testing.UnsafeTestServiceServer
-}
-
 func TestMiddleware(t *testing.T) {
 	ctx := context.Background()
 
@@ -104,6 +44,7 @@ authenticators:
   bearer_token:
     enabled: true
     config:
+      subject_from: identity.id
       check_session_url: %s
 authorizers:
   allow:

middleware/middleware_test.go

@@ -0,0 +1,105 @@
+// Copyright © 2023 Ory Corp
+// SPDX-License-Identifier: Apache-2.0
+
+package middleware_test
+
+import (
+	"context"
+	"io"
+	"net"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"google.golang.org/grpc"
+	"google.golang.org/grpc/credentials/insecure"
+	"google.golang.org/grpc/test/bufconn"
+	grpcTesting "google.golang.org/grpc/test/grpc_testing"
+)
+
+func testClient(t *testing.T, l *bufconn.Listener, dialOpts ...grpc.DialOption) grpcTesting.TestServiceClient {
+	conn, err := grpc.Dial("bufnet",
+		append(dialOpts,
+			grpc.WithTransportCredentials(insecure.NewCredentials()),
+			grpc.WithAuthority("myproject.apis.ory.sh"),
+			grpc.WithContextDialer(func(context.Context, string) (net.Conn, error) { return l.Dial() }),
+		)...,
+	)
+	require.NoError(t, err)
+	t.Cleanup(func() { conn.Close() })
+
+	return grpcTesting.NewTestServiceClient(conn)
+}
+
+// testTokenCheckServer is a kratos-style mock server which responds to requests to authenticate the request
+// for a successful response the token has to be `Beaerer correct token`.
+func testTokenCheckServer(t *testing.T) *httptest.Server {
+	s := httptest.NewServer(http.HandlerFunc(
+		func(w http.ResponseWriter, r *http.Request) {
+			if r.Header.Get("authorization") != "Bearer correct token" {
+				t.Logf("denied request %+v", r)
+				w.WriteHeader(http.StatusForbidden)
+				return
+			}
+			t.Logf("allowed request %+v", r)
+			io.WriteString(w, `{
+				"id": "user-id",
+				"active": true,
+				"expires_at": "2022-12-31T13:50:30.427292Z",
+				"authenticated_at": "2022-12-01T13:50:30.825516Z",
+				"authenticator_assurance_level": "aal1",
+				"authentication_methods": [
+				  {
+					"method": "password",
+					"aal": "aal1",
+					"completed_at": "2022-12-01T13:50:30.427375604Z"
+				  }
+				],
+				"issued_at": "2022-12-01T13:50:30.427292Z",
+				"identity": {
+				  "id": "user-id",
+				  "schema_id": "schema-id",
+				  "state": "active",
+				  "state_changed_at": "2022-12-01T13:50:30.331786Z",
+				  "traits": {
+					"email": "[email protected]",
+					"name": "User"
+				  },
+				  "verifiable_addresses": [],
+				  "recovery_addresses": [],
+				  "metadata_public": null,
+				  "created_at": "2022-12-01T13:50:30.340643Z",
+				  "updated_at": "2022-12-01T13:50:30.340643Z"
+				},
+				"devices": []
+			  }`)
+		}))
+	t.Cleanup(s.Close)
+	return s
+}
+
+func writeTestConfig(t *testing.T, pattern string, content string) string {
+	f, err := os.CreateTemp(t.TempDir(), pattern)
+	if err != nil {
+		t.Error(err)
+		return ""
+	}
+	defer f.Close()
+	io.WriteString(f, content)
+
+	return f.Name()
+}
+
+type testToken string
+
+func (t testToken) GetRequestMetadata(context.Context, ...string) (map[string]string, error) {
+	return map[string]string{"authorization": "Bearer " + string(t)}, nil
+}
+func (t testToken) RequireTransportSecurity() bool { return false }
+
+type upstream struct {
+	*MockTestServiceServer
+	grpcTesting.UnsafeTestServiceServer
+}

pipeline/authn/authenticator_bearer_token.go

@@ -154,6 +154,10 @@ func (a *AuthenticatorBearerToken) Authenticate(r *http.Request, session *Authen
 		return helper.ErrForbidden.WithReasonf("The configured extra_from GJSON path returned an error on JSON output: %s", err.Error()).WithDebugf("GJSON path: %s\nBody: %s\nResult: %s", cf.ExtraFrom, body, extraRaw).WithTrace(err)
 	}
 
+	if len(subject) == 0 {
+		return errors.WithStack(helper.ErrForbidden.WithReasonf("Subject field from remote endpoint is empty."))
+	}
+
 	session.Subject = subject
 	session.Extra = extra
 	return nil

pipeline/authn/authenticator_bearer_token_test.go

@@ -203,6 +203,24 @@ func TestAuthenticatorBearerToken(t *testing.T) {
 					Subject: "123",
 				},
 			},
+			{
+				d: "should NOT pass Accept-Encoding",
+				r: &http.Request{Host: "some-host", Header: http.Header{"Authorization": {"bearer zyx"}, "Accept-Encoding": {"gzip"}}, URL: &url.URL{Path: "/users/123", RawQuery: "query=string"}, Method: "PUT"},
+				router: func(w http.ResponseWriter, r *http.Request) {
+					assert.Equal(t, r.Method, "PUT")
+					assert.Equal(t, "some-host", r.Header.Get("X-Forwarded-Host"))
+					assert.Equal(t, "bar", r.Header.Get("X-Foo"))
+					assert.Equal(t, r.Header.Get("Authorization"), "bearer zyx")
+					assert.Equal(t, "", r.Header.Get("Accept-Encoding"))
+					w.WriteHeader(200)
+					w.Write([]byte(`{"sub": "123"}`))
+				},
+				config:    []byte(`{"preserve_host": true, "additional_headers": {"X-Foo": "bar","X-Forwarded-For": "not-some-host"}}`),
+				expectErr: false,
+				expectSess: &AuthenticationSession{
+					Subject: "123",
+				},
+			},
 			{
 				d: "does not pass request body through to auth server",
 				r: &http.Request{
@@ -258,6 +276,19 @@ func TestAuthenticatorBearerToken(t *testing.T) {
 					Extra:   map[string]interface{}{"session": map[string]interface{}{"foo": "bar"}, "identity": map[string]interface{}{"id": "123"}},
 				},
 			},
+			{
+				d: "it should error when the subject is empty",
+				r: &http.Request{Header: http.Header{"Authorization": {"bearer token"}}, URL: &url.URL{Path: ""}},
+				setup: func(t *testing.T, m *httprouter.Router) {
+					m.GET("/", func(w http.ResponseWriter, _ *http.Request, _ httprouter.Params) {
+						w.WriteHeader(200)
+						w.Write([]byte(`{"identity": {"id": ""}, "session": {"foo": "bar"}}`))
+					})
+				},
+				config:     []byte(`{"subject_from": "identity.id", "extra_from": "@this"}`),
+				expectErr:  true,
+				expectSess: &AuthenticationSession{},
+			},
 			{
 				d: "should work with custom header forwarded",
 				r: &http.Request{Header: http.Header{"Authorization": {"bearer token"}, "X-User": {"123"}}, URL: &url.URL{Path: ""}},

pipeline/authn/authenticator_cookie_session.go

@@ -197,9 +197,9 @@ func forwardRequestToSessionStore(client *http.Client, r *http.Request, cf Authe
 			return json.RawMessage{}, errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Unable to fetch cookie session context from remote: %+v", err))
 		}
 		return body, nil
-	} else {
-		return json.RawMessage{}, errors.WithStack(helper.ErrUnauthorized)
 	}
+
+	return json.RawMessage{}, errors.WithStack(helper.ErrUnauthorized)
 }
 
 func PrepareRequest(r *http.Request, cf AuthenticatorForwardConfig) (http.Request, error) {