From 4dca29f1f9635a3879f400c8b5d18db44ee12565 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tim=20M=C3=B6hlmann?= <tim+github@zitadel.com>
Date: Thu, 2 Mar 2023 15:24:44 +0200
Subject: [PATCH] fix: use the same schema encoder everywhere (#299)

properly register SpaceDelimitedArray for all instances
of schema.Encoder inside the oidc framework.

Closes #295
---
 pkg/client/client.go   | 10 +---------
 pkg/oidc/types.go      | 12 ++++++++++++
 pkg/oidc/types_test.go | 19 +++++++++++++++++++
 pkg/op/device_test.go  |  3 ---
 pkg/op/op.go           |  2 +-
 5 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/pkg/client/client.go b/pkg/client/client.go
index b9ae008..ebe1442 100644
--- a/pkg/client/client.go
+++ b/pkg/client/client.go
@@ -8,11 +8,9 @@ import (
 	"io"
 	"net/http"
 	"net/url"
-	"reflect"
 	"strings"
 	"time"
 
-	"github.com/gorilla/schema"
 	"golang.org/x/oauth2"
 	"gopkg.in/square/go-jose.v2"
 
@@ -21,13 +19,7 @@ import (
 	"github.com/zitadel/oidc/v2/pkg/oidc"
 )
 
-var Encoder = func() httphelper.Encoder {
-	e := schema.NewEncoder()
-	e.RegisterEncoder(oidc.SpaceDelimitedArray{}, func(value reflect.Value) string {
-		return value.Interface().(oidc.SpaceDelimitedArray).Encode()
-	})
-	return e
-}()
+var Encoder = httphelper.Encoder(oidc.NewEncoder())
 
 // Discover calls the discovery endpoint of the provided issuer and returns its configuration
 // It accepts an optional argument "wellknownUrl" which can be used to overide the dicovery endpoint url
diff --git a/pkg/oidc/types.go b/pkg/oidc/types.go
index 1260798..21b6fba 100644
--- a/pkg/oidc/types.go
+++ b/pkg/oidc/types.go
@@ -4,9 +4,11 @@ import (
 	"database/sql/driver"
 	"encoding/json"
 	"fmt"
+	"reflect"
 	"strings"
 	"time"
 
+	"github.com/gorilla/schema"
 	"golang.org/x/text/language"
 	"gopkg.in/square/go-jose.v2"
 )
@@ -125,6 +127,16 @@ func (s SpaceDelimitedArray) Value() (driver.Value, error) {
 	return strings.Join(s, " "), nil
 }
 
+// NewEncoder returns a schema Encoder with
+// a registered encoder for SpaceDelimitedArray.
+func NewEncoder() *schema.Encoder {
+	e := schema.NewEncoder()
+	e.RegisterEncoder(SpaceDelimitedArray{}, func(value reflect.Value) string {
+		return value.Interface().(SpaceDelimitedArray).Encode()
+	})
+	return e
+}
+
 type Time time.Time
 
 func (t *Time) UnmarshalJSON(data []byte) error {
diff --git a/pkg/oidc/types_test.go b/pkg/oidc/types_test.go
index 6c62c40..74323da 100644
--- a/pkg/oidc/types_test.go
+++ b/pkg/oidc/types_test.go
@@ -3,10 +3,12 @@ package oidc
 import (
 	"bytes"
 	"encoding/json"
+	"net/url"
 	"strconv"
 	"strings"
 	"testing"
 
+	"github.com/gorilla/schema"
 	"github.com/stretchr/testify/assert"
 	"golang.org/x/text/language"
 )
@@ -335,3 +337,20 @@ func TestSpaceDelimitatedArray_ValuerNil(t *testing.T) {
 		assert.Equal(t, SpaceDelimitedArray(nil), reversed, "scan nil")
 	}
 }
+
+func TestNewEncoder(t *testing.T) {
+	type request struct {
+		Scopes SpaceDelimitedArray `schema:"scope"`
+	}
+	a := request{
+		Scopes: SpaceDelimitedArray{"foo", "bar"},
+	}
+
+	values := make(url.Values)
+	NewEncoder().Encode(a, values)
+	assert.Equal(t, url.Values{"scope": []string{"foo bar"}}, values)
+
+	var b request
+	schema.NewDecoder().Decode(&b, values)
+	assert.Equal(t, a, b)
+}
diff --git a/pkg/op/device_test.go b/pkg/op/device_test.go
index ca68759..b3ac89d 100644
--- a/pkg/op/device_test.go
+++ b/pkg/op/device_test.go
@@ -98,8 +98,6 @@ func TestParseDeviceCodeRequest(t *testing.T) {
 			name:    "empty request",
 			wantErr: true,
 		},
-		/* decoding a SpaceDelimitedArray is broken
-		https://github.com/zitadel/oidc/issues/295
 		{
 			name: "success",
 			req: &oidc.DeviceAuthorizationRequest{
@@ -107,7 +105,6 @@ func TestParseDeviceCodeRequest(t *testing.T) {
 				ClientID: "web",
 			},
 		},
-		*/
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
diff --git a/pkg/op/op.go b/pkg/op/op.go
index 2859722..bb45425 100644
--- a/pkg/op/op.go
+++ b/pkg/op/op.go
@@ -189,7 +189,7 @@ func newProvider(ctx context.Context, config *Config, storage Storage, issuer fu
 	o.decoder = schema.NewDecoder()
 	o.decoder.IgnoreUnknownKeys(true)
 
-	o.encoder = schema.NewEncoder()
+	o.encoder = oidc.NewEncoder()
 
 	o.crypto = NewAESCrypto(config.CryptoKey)