Repository: zitadel/oidc
Pull request #673 (2024-10-31 21:09:47)

fix: ignore all unmarshal errors from locale
This changes unmarshal of Locale so that rather than ignoring just `language.ValueError` errors, it ignores all errors. Generally speaking, a bad locale value should not prevent users from authenticating.

Resolves  https://github.com/zitadel/oidc/issues/672


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/zitadel/oidc/pull/673?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zitadel) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 60.94%. Comparing base [(`0992c5f`)](https://app.codecov.io/gh/zitadel/oidc/commit/0992c5f3ce793c265c82993cf17fcb7c5a7e6cdf?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zitadel) to head [(`d822f27`)](https://app.codecov.io/gh/zitadel/oidc/commit/d822f27b3642c38873cba207538e51afcfd2fbbf?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zitadel).
> Report is 100 commits behind head on main.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main     #673      +/-   ##
==========================================
+ Coverage   60.06%   60.94%   +0.88%     
==========================================
  Files          80       81       +1     
  Lines        6998     7454     +456     
==========================================
+ Hits         4203     4543     +340     
- Misses       2498     2601     +103     
- Partials      297      310      +13     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/zitadel/oidc/pull/673?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zitadel).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zitadel).

muhlemmer: I'm approving this fix as I understand the rationale that locale is not so important as failed authentications. However, I have a dislike of ignoring errors in general, as it may hamper debug efforts. If this ever backfires in complaints where people couldn't figure out why the Locale is missing (silent error) we might rollback this change.

I do recommend pressuring Okta to do their job and fix this on their side for the sake of long-term stability.

Patch:
diff --git a/pkg/oidc/types.go b/pkg/oidc/types.go
index e7292e6..7063426 100644
--- a/pkg/oidc/types.go
+++ b/pkg/oidc/types.go
@@ -3,7 +3,6 @@ package oidc
 import (
 	"database/sql/driver"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"reflect"
 	"strings"
@@ -78,22 +77,16 @@ func (l *Locale) MarshalJSON() ([]byte, error) {
 }
 
 // UnmarshalJSON implements json.Unmarshaler.
-// When [language.ValueError] is encountered, the containing tag will be set
+// All unmarshal errors for are ignored.
+// When an error is encountered, the containing tag will be set
 // to an empty value (language "und") and no error will be returned.
 // This state can be checked with the `l.Tag().IsRoot()` method.
 func (l *Locale) UnmarshalJSON(data []byte) error {
 	err := json.Unmarshal(data, &l.tag)
-	if err == nil {
-		return nil
-	}
-
-	// catch "well-formed but unknown" errors
-	var target language.ValueError
-	if errors.As(err, &target) {
+	if err != nil {
 		l.tag = language.Tag{}
-		return nil
 	}
-	return err
+	return nil
 }
 
 type Locales []language.Tag


Test patch:
diff --git a/pkg/oidc/types_test.go b/pkg/oidc/types_test.go
index df93a73..c7ce0ee 100644
--- a/pkg/oidc/types_test.go
+++ b/pkg/oidc/types_test.go
@@ -232,9 +232,11 @@ func TestLocale_UnmarshalJSON(t *testing.T) {
 			},
 		},
 		{
-			name:    "bad form, error",
-			input:   `{"locale": "g!!!!!"}`,
-			wantErr: true,
+			name:  "bad form, error",
+			input: `{"locale": "g!!!!!"}`,
+			want: dst{
+				Locale: &Locale{},
+			},
 		},
 	}