fix(rp): don't ignore JWKS parsing errors (#771)

This safely ignores unknown key type errors on JWKS while returning all
other errors. Returned errors are wrap to easily identify which key in
the set is problematic if any.

Jose v4.0.3 was handling this correctly according to spec, but it was
reverted in v4.0.4 as the implementation was a breaking change due to
the custom UnmarshalJSON on the key set. For details see:
- https://github.com/go-jose/go-jose/issues/136
- https://github.com/go-jose/go-jose/pull/137

Jose v4.0.4 also provided a handy static error to check for unknown web
key types. Sadly this was removed: a prefix match on the error message
is the best option until Jose improves it's error handling.

Hopefully, Jose will not change the error message in a patch or minor
version release. But just in case, test cases have been added to detect
it.

Closes #541

### Definition of Ready

- [x] I am happy with the code
- [x] Short description of the feature/issue is added in the pr
description
- [x] PR is linked to the corresponding user story
- [ ] Acceptance criteria are met
- [ ] All open todos and follow ups are defined in a new ticket and
justified
- [ ] Deviations from the acceptance criteria and design are agreed with
the PO and documented.
- [x] No debug or dead code
- [x] My code has no repetitions
- [x] Critical parts are tested automatically
- [x] Where possible E2E tests are implemented
- [x] Documentation/examples are up-to-date
- [x] All non-functional requirements are met
- [ ] Functionality of the acceptance criteria is checked manually on
the dev system.

Co-authored-by: Wim Van Laer <wim07101993@users.noreply.github.com>

pkg/client/rp/jwks.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+	"strings"
 	"sync"
 
 	jose "github.com/go-jose/go-jose/v4"
@@ -14,6 +15,8 @@ import (
 	"github.com/zitadel/oidc/v3/pkg/oidc"
 )
 
+const joseUnknownKeyTypeErrMsg = "go-jose/go-jose: unknown json web key type '"
+
 func NewRemoteKeySet(client *http.Client, jwksURL string, opts ...func(*remoteKeySet)) oidc.KeySet {
 	keyset := &remoteKeySet{httpClient: client, jwksURL: jwksURL}
 	for _, opt := range opts {
@@ -238,14 +241,19 @@ func (k *jsonWebKeySet) UnmarshalJSON(data []byte) (err error) {
 	var raw rawJSONWebKeySet
 	err = json.Unmarshal(data, &raw)
 	if err != nil {
-		return err
+		return fmt.Errorf("oidc: failed to unmarshall key set: %w", err)
 	}
-	for _, key := range raw.Keys {
+	for i, key := range raw.Keys {
 		webKey := new(jose.JSONWebKey)
-		err = webKey.UnmarshalJSON(key)
-		if err == nil {
-			k.Keys = append(k.Keys, *webKey)
+		if err = webKey.UnmarshalJSON(key); err != nil {
+			if strings.HasPrefix(err.Error(), joseUnknownKeyTypeErrMsg) {
+				continue
+			}
+
+			return fmt.Errorf("oidc: failed to unmarshal key %d from set: %w", i, err)
 		}
+
+		k.Keys = append(k.Keys, *webKey)
 	}
 	return nil
 }

pkg/client/rp/jwks_test.go

@@ -0,0 +1,74 @@
+package rp
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestJsonWebKeySet_UnmarshalJSON(t *testing.T) {
+	tests := []struct {
+		name       string
+		jsonData   string
+		wantKeyLen int
+		wantErr    bool
+		errPrefix  string
+	}{
+		{
+			name:       "valid key set",
+			jsonData:   `{"keys":[{"kty":"RSA","use":"sig","kid":"key1","alg":"RS256","n":"n-value","e":"e-value"}]}`,
+			wantKeyLen: 1,
+			wantErr:    false,
+		},
+		{
+			name:       "empty key set",
+			jsonData:   `{"keys":[]}`,
+			wantKeyLen: 0,
+			wantErr:    false,
+		},
+		{
+			name:       "unknown key type",
+			jsonData:   `{"keys":[{"kty":"UNKNOWN","use":"sig","kid":"key1"}]}`,
+			wantKeyLen: 0,
+			wantErr:    false,
+		},
+		{
+			name:       "mixed valid and unknown key types",
+			jsonData:   `{"keys":[{"kty":"RSA","use":"sig","kid":"key1","alg":"RS256","n":"n-value","e":"e-value"},{"kty":"UNKNOWN","use":"sig","kid":"key2"}]}`,
+			wantKeyLen: 1,
+			wantErr:    false,
+		},
+		{
+			name:       "invalid json",
+			jsonData:   `{"keys":[{]`,
+			wantKeyLen: 0,
+			wantErr:    true,
+			errPrefix:  "oidc: failed to unmarshall key set: ",
+		},
+		{
+			name:       "other error during key unmarshal",
+			jsonData:   `{"keys":[{"kty":"RSA","use":"sig","kid":"key1","alg":"RS256"}]}`,
+			wantKeyLen: 0,
+			wantErr:    true,
+			errPrefix:  "oidc: failed to unmarshal key 0 from set: ",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var keySet jsonWebKeySet
+			err := keySet.UnmarshalJSON([]byte(tt.jsonData))
+
+			if tt.wantErr {
+				assert.Error(t, err)
+				assert.NotContains(t, err.Error(), joseUnknownKeyTypeErrMsg)
+				assert.True(t, strings.HasPrefix(err.Error(), tt.errPrefix))
+
+			} else {
+				assert.NoError(t, err)
+				assert.Len(t, keySet.Keys, tt.wantKeyLen)
+			}
+		})
+	}
+}