fix: use Query.Get when fetching QueryParameter (#1106)

.github/workflows/ci.yml

@@ -63,7 +63,7 @@ jobs:
     name: Run tests
     strategy:
       matrix:
-        name: ["reload", "e2e", "forwarded-header"]
+        name: ["reload", "e2e", "forwarded-header", "bearer-token"]
     needs:
       - sdk-generate
     steps:

helper/bearer.go

@@ -26,7 +26,10 @@ func BearerTokenFromRequest(r *http.Request, tokenLocation *BearerTokenLocation)
 			}
 			return r.Header.Get(*tokenLocation.Header)
 		} else if tokenLocation.QueryParameter != nil {
-			return r.FormValue(*tokenLocation.QueryParameter)
+			if r.URL == nil {
+				return ""
+			}
+			return r.URL.Query().Get(*tokenLocation.QueryParameter)
 		} else if tokenLocation.Cookie != nil {
 			cookie, err := r.Cookie(*tokenLocation.Cookie)
 			if err != nil {

helper/bearer_test.go

@@ -4,7 +4,10 @@
 package helper_test
 
 import (
+	"io"
 	"net/http"
+	"net/url"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -54,9 +57,7 @@ func TestBearerTokenFromRequest(t *testing.T) {
 		expectedToken := "token"
 		customQueryParameterName := "Custom-Auth"
 		request := &http.Request{
-			Form: map[string][]string{
-				customQueryParameterName: {expectedToken},
-			},
+			URL: &url.URL{RawQuery: customQueryParameterName + "=" + expectedToken},
 		}
 		tokenLocation := helper.BearerTokenLocation{QueryParameter: &customQueryParameterName}
 		token := helper.BearerTokenFromRequest(request, &tokenLocation)
@@ -77,4 +78,28 @@ func TestBearerTokenFromRequest(t *testing.T) {
 		token := helper.BearerTokenFromRequest(request, &tokenLocation)
 		assert.Equal(t, expectedToken, token)
 	})
+
+	t.Run("case=Should not consume body when token from query parameter and Content-type is 'application/x-www-form-urlencoded' ", func(t *testing.T) {
+		expectedToken := "token"
+		customQueryParameterName := "Custom-Auth"
+
+		request := &http.Request{
+			Method: http.MethodPost,
+			URL: &url.URL{
+				RawQuery: customQueryParameterName + "=" + expectedToken,
+			},
+			Body: io.NopCloser(strings.NewReader("body")),
+			Header: http.Header{
+				"Content-Type": {"application/x-www-form-urlencoded"},
+			},
+		}
+
+		tokenLocation := helper.BearerTokenLocation{QueryParameter: &customQueryParameterName}
+		token := helper.BearerTokenFromRequest(request, &tokenLocation)
+		assert.Equal(t, expectedToken, token)
+
+		b, err := io.ReadAll(request.Body)
+		assert.NoError(t, err)
+		assert.Equal(t, "body", string(b))
+	})
 }

pipeline/authn/authenticator_jwt_test.go

@@ -8,6 +8,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+	"net/url"
 	"testing"
 	"time"
 
@@ -137,13 +138,11 @@ func TestAuthenticatorJWT(t *testing.T) {
 			{
 				d: "should pass because the valid JWT token was provided in a proper location (custom query parameter)",
 				r: &http.Request{
-					Form: map[string][]string{
-						"token": {
-							gen(keys[1], jwt.MapClaims{
-								"sub": "sub",
-								"exp": now.Add(time.Hour).Unix(),
-							}),
-						},
+					URL: &url.URL{
+						RawQuery: "token=" + gen(keys[1], jwt.MapClaims{
+							"sub": "sub",
+							"exp": now.Add(time.Hour).Unix(),
+						}),
 					},
 				},
 				config:    `{"token_from": {"query_parameter": "token"}}`,

pipeline/authn/authenticator_oauth2_introspection_test.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"net/http"
 	"net/http/httptest"
+	"net/url"
 	"testing"
 	"time"
 
@@ -113,8 +114,8 @@ func TestAuthenticatorOAuth2Introspection(t *testing.T) {
 			{
 				d: "should pass because the valid token was provided in a proper location (custom query parameter)",
 				r: &http.Request{
-					Form: map[string][]string{
-						"token": {"token"},
+					URL: &url.URL{
+						RawQuery: "token=" + "token",
 					},
 				},
 				config:    []byte(`{"token_from": {"query_parameter": "token"}}`),

test/bearer-token/.gitignore

@@ -0,0 +1 @@
+*.log

test/bearer-token/config.yaml

@@ -0,0 +1,27 @@
+serve:
+  api:
+    port: 6061
+  proxy:
+    port: 6060
+
+access_rules:
+  repositories:
+    - file://./rules.1.json
+
+authenticators:
+  noop:
+    enabled: true
+  bearer_token:
+    enabled: true
+    config:
+      check_session_url: http://localhost:6662/session
+
+authorizers:
+  allow:
+    enabled: true
+  deny:
+    enabled: true
+
+mutators:
+  noop:
+    enabled: true

test/bearer-token/okapi/main.go

@@ -0,0 +1,52 @@
+// Copyright © 2023 Ory Corp
+// SPDX-License-Identifier: Apache-2.0
+
+package main
+
+import (
+	"fmt"
+	"io"
+	"net/http"
+	"os"
+
+	"github.com/julienschmidt/httprouter"
+)
+
+func main() {
+	router := httprouter.New()
+
+	router.POST("/test", test)
+	router.GET("/session", session)
+
+	port := os.Getenv("PORT")
+	if port == "" {
+		port = "6662"
+	}
+
+	server := http.Server{
+		Addr:    fmt.Sprintf(":%s", port),
+		Handler: router,
+	}
+
+	if err := server.ListenAndServe(); err != nil {
+		panic(err)
+	}
+}
+
+func test(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
+	b, err := io.ReadAll(r.Body)
+	if err != nil {
+		w.WriteHeader(http.StatusInternalServerError)
+	}
+
+	if len(b) == 0 {
+		w.WriteHeader(http.StatusBadRequest)
+		return
+	}
+
+	w.WriteHeader(http.StatusOK)
+}
+
+func session(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
+	w.WriteHeader(http.StatusOK)
+}

test/bearer-token/rules.1.json

@@ -0,0 +1,34 @@
+[
+  {
+    "id": "test",
+    "upstream": {
+      "url": "http://127.0.0.1:6662"
+    },
+    "match": {
+      "url": "http://127.0.0.1:6060/test",
+      "methods": ["POST"]
+    },
+    "authenticators": [
+      {
+        "handler": "bearer_token",
+        "config": {
+          "check_session_url": "http://127.0.0.1:6662/session",
+          "preserve_path": true,
+          "preserve_query": false,
+          "force_method": "GET",
+          "token_from": {
+            "query_parameter": "token"
+          }
+        }
+      }
+    ],
+    "authorizer": {
+      "handler": "allow"
+    },
+    "mutators": [
+      {
+        "handler": "noop"
+      }
+    ]
+  }
+]

test/bearer-token/run.sh

@@ -0,0 +1,77 @@
+#!/bin/bash
+
+set -euo pipefail
+
+waitport() {
+  i=0
+  while ! nc -z localhost "$1" ; do
+    sleep 1
+    if [ $i -gt 10 ]; then
+      cat ./config.yaml
+      cat ./oathkeeper.log
+      exit 1
+    fi
+    i=$((i+1))
+  done
+}
+
+cd "$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
+
+
+run_oathkekeper() {
+  killall oathkeeper || true
+
+  export OATHKEEPER_PROXY=http://127.0.0.1:6060
+  export OATHKEEPER_API=http://127.0.0.1:6061
+
+  go build -o . ../..
+  LOG_LEVEL=debug ./oathkeeper --config ./config.yaml serve > ./oathkeeper.log 2>&1 &
+
+  waitport 6060
+  waitport 6061
+}
+
+run_api(){
+  killall okapi || true
+
+  PORT=6662 go run ./okapi > ./api.log 2>&1 &
+
+  waitport 6662
+}
+
+SUCCESS_TEST=()
+FAILED_TEST=()
+
+run_test() {
+  label=$1
+  shift 1
+
+  result="0"
+  "$@" || result="1"
+
+  if [[ "$result" -eq "0" ]]; then
+    SUCCESS_TEST+=("$label")
+  else
+    FAILED_TEST+=("$label")
+  fi
+}
+
+function finish {
+  echo ::group::Config
+  cat ./config.yaml
+  cat ./rules.1.json
+  echo ::endgroup::
+  echo ::group::Log
+  cat ./oathkeeper.log
+  echo ::endgroup::
+}
+trap finish EXIT
+
+run_oathkekeper
+run_api
+
+curl -X POST -f http://127.0.0.1:6060/test?token=token -F fk=fv -H "Content-Type: application/x-www-form-urlencoded" -i
+
+kill %1 || true
+
+trap - EXIT