From aadfd3017f0043487ceda345cc6654343f1cbb6a Mon Sep 17 00:00:00 2001 From: Ayato Date: Mon, 16 Sep 2024 21:53:45 +0900 Subject: [PATCH 1/6] feat(oidc): return defined error when discovery failed --- pkg/client/client.go | 2 +- pkg/client/client_test.go | 20 ++++++++++++++++---- pkg/oidc/verifier.go | 1 + 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index 990da9b5..58f1511f 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, fmt.Errorf("%w: %w", oidc.ErrDiscoveryFailed, err) } if logger, ok := logging.FromContext(ctx); ok { logger.Debug("discover", "config", discoveryConfig) diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index e06c8252..bef23768 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,16 +33,27 @@ func TestDiscover(t *testing.T) { wantFields: &wantFields{ UILocalesSupported: true, }, - wantErr: false, + wantErr: nil, + }, + { + name: "discovery failed", + args: args{ + issuer: "https://example.com", + }, + wantFields: &wantFields{ + UILocalesSupported: true, + }, + 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) + if tt.wantErr != nil { + assert.ErrorIs(t, err, tt.wantErr) return } + require.NoError(t, err) if tt.wantFields == nil { return diff --git a/pkg/oidc/verifier.go b/pkg/oidc/verifier.go index cb666762..e4ea8131 100644 --- a/pkg/oidc/verifier.go +++ b/pkg/oidc/verifier.go @@ -40,6 +40,7 @@ type IDClaims interface { var ( ErrParse = errors.New("parsing of request failed") + ErrDiscoveryFailed = errors.New("OpenID Provider Configuration Discovery is failed") ErrIssuerInvalid = errors.New("issuer does not match") ErrSubjectMissing = errors.New("subject missing") ErrAudience = errors.New("audience is not valid") From 973611612e8a32e2cddf00e0b6d68b1c32ec7f42 Mon Sep 17 00:00:00 2001 From: Ayato Date: Tue, 17 Sep 2024 23:10:07 +0900 Subject: [PATCH 2/6] Use errors.Join() to join errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Tim Möhlmann --- pkg/client/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index 58f1511f..551ac920 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, fmt.Errorf("%w: %w", oidc.ErrDiscoveryFailed, err) + return nil, errors.Join(err, oidc.ErrDiscoveryFailed) } if logger, ok := logging.FromContext(ctx); ok { logger.Debug("discover", "config", discoveryConfig) From 6021ef3fcde84a9755345dff2cb123a086cc5426 Mon Sep 17 00:00:00 2001 From: Ayato Date: Tue, 17 Sep 2024 23:12:45 +0900 Subject: [PATCH 3/6] Remove unnecessary field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Tim Möhlmann --- pkg/client/client_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index bef23768..e527236f 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -40,9 +40,6 @@ func TestDiscover(t *testing.T) { args: args{ issuer: "https://example.com", }, - wantFields: &wantFields{ - UILocalesSupported: true, - }, wantErr: oidc.ErrDiscoveryFailed, }, } From ffd0bb9f33ee1f29951eff4b4235546e574df578 Mon Sep 17 00:00:00 2001 From: Ayato Date: Tue, 17 Sep 2024 23:15:39 +0900 Subject: [PATCH 4/6] Fix order and message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Tim Möhlmann --- pkg/oidc/verifier.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/oidc/verifier.go b/pkg/oidc/verifier.go index e4ea8131..f580da66 100644 --- a/pkg/oidc/verifier.go +++ b/pkg/oidc/verifier.go @@ -40,8 +40,8 @@ type IDClaims interface { var ( ErrParse = errors.New("parsing of request failed") - ErrDiscoveryFailed = errors.New("OpenID Provider Configuration Discovery is 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") From 81c89c3949ea9fc2589fa081402f53ab2d0115ce Mon Sep 17 00:00:00 2001 From: Ayato Date: Tue, 17 Sep 2024 23:41:55 +0900 Subject: [PATCH 5/6] Fix error order --- pkg/client/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index 551ac920..56417b5f 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, errors.Join(err, oidc.ErrDiscoveryFailed) + return nil, errors.Join(oidc.ErrDiscoveryFailed, err) } if logger, ok := logging.FromContext(ctx); ok { logger.Debug("discover", "config", discoveryConfig) From 9af7d5e75e0b1916592aa7fae89a3b1776ed10a1 Mon Sep 17 00:00:00 2001 From: Ayato Date: Thu, 19 Sep 2024 00:10:19 +0900 Subject: [PATCH 6/6] Simplify error assertion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Tim Möhlmann --- pkg/client/client_test.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index e527236f..10469417 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -46,12 +46,7 @@ func TestDiscover(t *testing.T) { 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 != nil { - assert.ErrorIs(t, err, tt.wantErr) - return - } - - require.NoError(t, err) + require.ErrorIs(t, err, tt.wantErr) if tt.wantFields == nil { return }