From d01a5c8f9111f4c68306f309bd26055dddd23e07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20M=C3=B6hlmann?= Date: Fri, 9 Jun 2023 16:31:44 +0200 Subject: [PATCH] fix: don't error on invalid i18n tags in discovery (#407) * reproduce #406 * fix: don't error on invalid i18n tags in discovery This changes the use of `[]language.Tag` to `oidc.Locales` in `DiscoveryConfig`. This should be compatible with callers that use the `[]language.Tag` . Locales now implements the `json.Unmarshaler` interface. With support for json arrays or space seperated strings. The latter because `UnmarshalText` might have been implicetely called by the json library before we added UnmarshalJSON. Fixes: #406 --- pkg/client/client_test.go | 54 ++++++++++++++++++++++++++ pkg/oidc/discovery.go | 8 +--- pkg/oidc/types.go | 51 ++++++++++++++++++++++-- pkg/oidc/types_test.go | 81 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 185 insertions(+), 9 deletions(-) create mode 100644 pkg/client/client_test.go diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go new file mode 100644 index 0000000..02c408b --- /dev/null +++ b/pkg/client/client_test.go @@ -0,0 +1,54 @@ +package client + +import ( + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDiscover(t *testing.T) { + type wantFields struct { + UILocalesSupported bool + } + + type args struct { + issuer string + wellKnownUrl []string + } + tests := []struct { + name string + args args + wantFields *wantFields + wantErr bool + }{ + { + name: "spotify", // https://github.com/zitadel/oidc/issues/406 + args: args{ + issuer: "https://accounts.spotify.com", + }, + wantFields: &wantFields{ + UILocalesSupported: true, + }, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := Discover(tt.args.issuer, http.DefaultClient, tt.args.wellKnownUrl...) + if tt.wantErr { + assert.Error(t, err) + return + } + require.NoError(t, err) + if tt.wantFields == nil { + return + } + assert.Equal(t, tt.args.issuer, got.Issuer) + if tt.wantFields.UILocalesSupported { + assert.NotEmpty(t, got.UILocalesSupported) + } + }) + } +} diff --git a/pkg/oidc/discovery.go b/pkg/oidc/discovery.go index 3574101..14fce5e 100644 --- a/pkg/oidc/discovery.go +++ b/pkg/oidc/discovery.go @@ -1,9 +1,5 @@ package oidc -import ( - "golang.org/x/text/language" -) - const ( DiscoveryEndpoint = "/.well-known/openid-configuration" ) @@ -130,10 +126,10 @@ type DiscoveryConfiguration struct { ServiceDocumentation string `json:"service_documentation,omitempty"` // ClaimsLocalesSupported contains a list of BCP47 language tag values that the OP supports for values of Claims returned. - ClaimsLocalesSupported []language.Tag `json:"claims_locales_supported,omitempty"` + ClaimsLocalesSupported Locales `json:"claims_locales_supported,omitempty"` // UILocalesSupported contains a list of BCP47 language tag values that the OP supports for the user interface. - UILocalesSupported []language.Tag `json:"ui_locales_supported,omitempty"` + UILocalesSupported Locales `json:"ui_locales_supported,omitempty"` // RequestParameterSupported specifies whether the OP supports use of the `request` parameter. If omitted, the default value is false. RequestParameterSupported bool `json:"request_parameter_supported,omitempty"` diff --git a/pkg/oidc/types.go b/pkg/oidc/types.go index 167f8b7..23367ef 100644 --- a/pkg/oidc/types.go +++ b/pkg/oidc/types.go @@ -9,6 +9,7 @@ import ( "time" "github.com/gorilla/schema" + "github.com/muhlemmer/gu" "golang.org/x/text/language" "gopkg.in/square/go-jose.v2" ) @@ -81,14 +82,58 @@ func (l *Locale) UnmarshalJSON(data []byte) error { type Locales []language.Tag -func (l *Locales) UnmarshalText(text []byte) error { - locales := strings.Split(string(text), " ") +// ParseLocales parses a slice of strings into Locales. +// If an entry causes a parse error or is undefined, +// it is ignored and not set to Locales. +func ParseLocales(locales []string) Locales { + out := make(Locales, 0, len(locales)) for _, locale := range locales { tag, err := language.Parse(locale) if err == nil && !tag.IsRoot() { - *l = append(*l, tag) + out = append(out, tag) } } + return out +} + +// UnmarshalText implements the [encoding.TextUnmarshaler] interface. +// It decodes an unquoted space seperated string into Locales. +// Undefined language tags in the input are ignored and ommited from +// the resulting Locales. +func (l *Locales) UnmarshalText(text []byte) error { + *l = ParseLocales( + strings.Split(string(text), " "), + ) + return nil +} + +// UnmarshalJSON implements the [json.Unmarshaler] interface. +// It decodes a json array or a space seperated string into Locales. +// Undefined language tags in the input are ignored and ommited from +// the resulting Locales. +func (l *Locales) UnmarshalJSON(data []byte) error { + var dst any + if err := json.Unmarshal(data, &dst); err != nil { + return fmt.Errorf("oidc locales: %w", err) + } + + // We catch the posibility of a space seperated string here, + // because UnmarshalText might have been implicetely called + // by the json library before we added UnmarshalJSON. + switch v := dst.(type) { + case nil: + *l = nil + case string: + *l = ParseLocales(strings.Split(v, " ")) + case []any: + locales, err := gu.AssertInterfaces[string](v) + if err != nil { + return fmt.Errorf("oidc locales: %w", err) + } + *l = ParseLocales(locales) + default: + return fmt.Errorf("oidc locales: unsupported type: %T", v) + } return nil } diff --git a/pkg/oidc/types_test.go b/pkg/oidc/types_test.go index 64f07f1..69540c2 100644 --- a/pkg/oidc/types_test.go +++ b/pkg/oidc/types_test.go @@ -224,6 +224,13 @@ func TestLocale_UnmarshalJSON(t *testing.T) { assert.Equal(t, want, got) } +func TestParseLocales(t *testing.T) { + in := []string{language.Afrikaans.String(), language.Danish.String(), "foobar", language.Und.String()} + want := Locales{language.Afrikaans, language.Danish} + got := ParseLocales(in) + assert.ElementsMatch(t, want, got) +} + func TestLocales_UnmarshalText(t *testing.T) { type args struct { text []byte @@ -281,6 +288,80 @@ func TestLocales_UnmarshalText(t *testing.T) { } } +func TestLocales_UnmarshalJSON(t *testing.T) { + in := []string{language.Afrikaans.String(), language.Danish.String(), "foobar", language.Und.String()} + spaceSepStr := strconv.Quote(strings.Join(in, " ")) + jsonArray, err := json.Marshal(in) + require.NoError(t, err) + + out := Locales{language.Afrikaans, language.Danish} + + type args struct { + data []byte + } + tests := []struct { + name string + args args + want Locales + wantErr bool + }{ + { + name: "invalid JSON", + args: args{ + data: []byte("~~~"), + }, + wantErr: true, + }, + { + name: "null", + args: args{ + data: []byte("null"), + }, + want: nil, + }, + { + name: "space seperated string", + args: args{ + data: []byte(spaceSepStr), + }, + want: out, + }, + { + name: "json string array", + args: args{ + data: jsonArray, + }, + want: out, + }, + { + name: "json invalid array", + args: args{ + data: []byte(`[1,2,3]`), + }, + wantErr: true, + }, + { + name: "invalid type (float64)", + args: args{ + data: []byte("22"), + }, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var got Locales + err := got.UnmarshalJSON([]byte(tt.args.data)) + if tt.wantErr { + assert.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} + func TestScopes_UnmarshalText(t *testing.T) { type args struct { text []byte