Repository: zitadel/oidc
Pull request #653 (2024-09-16 13:05:05)

feat(oidc): return defined error when discovery failed
This PR adds a specific error to handle discovery failures. 
It allows proper error handling when the issuer URL is invalid or the configuration is missing.
Thank you.

### Definition of Ready
- [x] I am happy with the code
- [x] Short description of the feature/issue is added in the pr description
- [ ] 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
- [ ] Critical parts are tested automatically
- [ ] Where possible E2E tests are implemented
- [ ] Documentation/examples are up-to-date
- [ ] All non-functional requirements are met
- [ ] Functionality of the acceptance criteria is checked manually on the dev system.



codecov[bot]: ## [Codecov](https://app.codecov.io/gh/zitadel/oidc/pull/653?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 62.91%. 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 [(`aadfd30`)](https://app.codecov.io/gh/zitadel/oidc/commit/aadfd3017f0043487ceda345cc6654343f1cbb6a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zitadel).
> Report is 92 commits behind head on main.

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


```diff
@@            Coverage Diff             @@
##             main     #653      +/-   ##
==========================================
+ Coverage   60.06%   62.91%   +2.85%     
==========================================
  Files          80       77       -3     
  Lines        6998     6137     -861     
==========================================
- Hits         4203     3861     -342     
+ Misses       2498     1966     -532     
- Partials      297      310      +13     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/zitadel/oidc/pull/653?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).

ay4toh5i: @muhlemmer 
Thanks for taking the time to review my PR and for the feedback! 
I’ve addressed the points you mentioned. Could you take another look when you get a chance?

Patch:
diff --git a/pkg/client/client.go b/pkg/client/client.go
index 990da9b..56417b5 100644
--- a/pkg/client/client.go
+++ b/pkg/client/client.go
@@ -42,7 +42,7 @@ func Discover(ctx context.Context, issuer string, httpClient *http.Client, wellK
 	discoveryConfig := new(oidc.DiscoveryConfiguration)
 	err = httphelper.HttpRequest(httpClient, req, &discoveryConfig)
 	if err != nil {
-		return nil, err
+		return nil, errors.Join(oidc.ErrDiscoveryFailed, err)
 	}
 	if logger, ok := logging.FromContext(ctx); ok {
 		logger.Debug("discover", "config", discoveryConfig)
diff --git a/pkg/oidc/verifier.go b/pkg/oidc/verifier.go
index cb66676..f580da6 100644
--- a/pkg/oidc/verifier.go
+++ b/pkg/oidc/verifier.go
@@ -41,6 +41,7 @@ type IDClaims interface {
 var (
 	ErrParse                   = errors.New("parsing of request failed")
 	ErrIssuerInvalid           = errors.New("issuer does not match")
+	ErrDiscoveryFailed         = errors.New("OpenID Provider Configuration Discovery has failed")
 	ErrSubjectMissing          = errors.New("subject missing")
 	ErrAudience                = errors.New("audience is not valid")
 	ErrAzpMissing              = errors.New("authorized party is not set. If Token is valid for multiple audiences, azp must not be empty")


Test patch:
diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go
index e06c825..1046941 100644
--- a/pkg/client/client_test.go
+++ b/pkg/client/client_test.go
@@ -7,6 +7,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"github.com/zitadel/oidc/v3/pkg/oidc"
 )
 
 func TestDiscover(t *testing.T) {
@@ -22,7 +23,7 @@ func TestDiscover(t *testing.T) {
 		name       string
 		args       args
 		wantFields *wantFields
-		wantErr    bool
+		wantErr    error
 	}{
 		{
 			name: "spotify", // https://github.com/zitadel/oidc/issues/406
@@ -32,17 +33,20 @@ func TestDiscover(t *testing.T) {
 			wantFields: &wantFields{
 				UILocalesSupported: true,
 			},
-			wantErr: false,
+			wantErr: nil,
+		},
+		{
+			name: "discovery failed",
+			args: args{
+				issuer: "https://example.com",
+			},
+			wantErr: oidc.ErrDiscoveryFailed,
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			got, err := Discover(context.Background(), tt.args.issuer, http.DefaultClient, tt.args.wellKnownUrl...)
-			if tt.wantErr {
-				assert.Error(t, err)
-				return
-			}
-			require.NoError(t, err)
+			require.ErrorIs(t, err, tt.wantErr)
 			if tt.wantFields == nil {
 				return
 			}