From ba33aebf0a98d130230b87fb360cf7f13dd087d0 Mon Sep 17 00:00:00 2001 From: Noah Fontes Date: Tue, 19 Jan 2021 13:48:52 -0800 Subject: [PATCH] Make auth code URL optional --- README.md | 6 ++-- pkg/backend/path_config.go | 5 +++- pkg/backend/path_config_test.go | 49 +++++++++++++++++++++++++++++++++ pkg/provider/basic.go | 12 ++++---- pkg/provider/basic_test.go | 7 +++-- pkg/provider/provider.go | 2 +- pkg/testutil/mock.go | 4 +-- 7 files changed, 71 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index d2c698a..e0eae6d 100644 --- a/README.md +++ b/README.md @@ -141,7 +141,9 @@ tokens. #### `PUT` (`write`) -Retrieve an authorization code URL for the given state. +Retrieve an authorization code URL for the given state. Some providers may not +provide the plugin with information about this URL, in which case accessing this +endpoint will return an error. | Name | Description | Type | Default | Required | |------|-------------|------|---------|----------| @@ -284,6 +286,6 @@ arbitrary OAuth 2 authorization code grant flow. | Name | Description | Default | Required | |------|-------------|---------|----------| -| `auth_code_url` | The URL to submit the initial authorization code request to. | None | Yes | +| `auth_code_url` | The URL to submit the initial authorization code request to. | None | No | | `token_url` | The URL to use for exchanging temporary codes and refreshing access tokens. | None | Yes | | `auth_style` | How to authenticate to the token URL. If specified, must be one of `in_header` or `in_params`. | Automatically detect | No | diff --git a/pkg/backend/path_config.go b/pkg/backend/path_config.go index bd8c51b..f97392b 100644 --- a/pkg/backend/path_config.go +++ b/pkg/backend/path_config.go @@ -104,13 +104,16 @@ func (b *backend) configAuthCodeURLUpdateOperation(ctx context.Context, req *log return logical.ErrorResponse("missing state"), nil } - url := c.Provider.Public(c.Config.ClientID).AuthCodeURL( + url, ok := c.Provider.Public(c.Config.ClientID).AuthCodeURL( state.(string), provider.WithRedirectURL(data.Get("redirect_url").(string)), provider.WithScopes(data.Get("scopes").([]string)), provider.WithURLParams(data.Get("auth_url_params").(map[string]string)), provider.WithURLParams(c.Config.AuthURLParams), ) + if !ok { + return logical.ErrorResponse("authorization code URL not available"), nil + } resp := &logical.Response{ Data: map[string]interface{}{ diff --git a/pkg/backend/path_config_test.go b/pkg/backend/path_config_test.go index 2be4211..589ca82 100644 --- a/pkg/backend/path_config_test.go +++ b/pkg/backend/path_config_test.go @@ -128,3 +128,52 @@ func TestConfigAuthCodeURL(t *testing.T) { assert.Equal(t, "geoff", qs.Get("foo")) // Configuration takes precedence! assert.Equal(t, "quux", qs.Get("baz")) } + +func TestConfigClientCredentials(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + storage := &logical.InmemStorage{} + + b := New(Options{}) + require.NoError(t, b.Setup(ctx, &logical.BackendConfig{})) + + // Write configuration. + req := &logical.Request{ + Operation: logical.UpdateOperation, + Path: configPath, + Storage: storage, + Data: map[string]interface{}{ + "client_id": "abc", + "client_secret": "def", + "provider": "custom", + "provider_options": map[string]interface{}{ + "token_url": "http://localhost/auth/token", + }, + }, + } + + resp, err := b.HandleRequest(ctx, req) + require.NoError(t, err) + require.False(t, resp != nil && resp.IsError(), "response has error: %+v", resp.Error()) + require.Nil(t, resp) + + // Retrieve an auth code URL. + req = &logical.Request{ + Operation: logical.UpdateOperation, + Path: configAuthCodeURLPath, + Storage: storage, + Data: map[string]interface{}{ + "state": "qwerty", + "scopes": []string{"read", "write"}, + "redirect_url": "http://example.com/redirect", + "auth_url_params": map[string]string{"foo": "bar", "baz": "quux"}, + }, + } + + resp, err = b.HandleRequest(ctx, req) + require.NoError(t, err) + require.NotNil(t, resp) + require.True(t, resp.IsError()) + require.EqualError(t, resp.Error(), "authorization code URL not available") +} diff --git a/pkg/provider/basic.go b/pkg/provider/basic.go index 79b0bf5..b1f2a47 100644 --- a/pkg/provider/basic.go +++ b/pkg/provider/basic.go @@ -29,7 +29,11 @@ type basicOperations struct { base *oauth2.Config } -func (bo *basicOperations) AuthCodeURL(state string, opts ...AuthCodeURLOption) string { +func (bo *basicOperations) AuthCodeURL(state string, opts ...AuthCodeURLOption) (string, bool) { + if bo.base.Endpoint.AuthURL == "" { + return "", false + } + o := &AuthCodeURLOptions{} o.ApplyOptions(opts) @@ -38,7 +42,7 @@ func (bo *basicOperations) AuthCodeURL(state string, opts ...AuthCodeURLOption) cfg.Scopes = o.Scopes cfg.RedirectURL = o.RedirectURL - return cfg.AuthCodeURL(state, o.AuthCodeOptions...) + return cfg.AuthCodeURL(state, o.AuthCodeOptions...), true } func (bo *basicOperations) AuthCodeExchange(ctx context.Context, code string, opts ...AuthCodeExchangeOption) (*Token, error) { @@ -175,10 +179,6 @@ func CustomFactory(ctx context.Context, vsn int, opts map[string]string) (Provid return nil, ErrNoProviderWithVersion } - if opts["auth_code_url"] == "" { - return nil, &OptionError{Option: "auth_code_url", Message: "authorization code URL is required"} - } - if opts["token_url"] == "" { return nil, &OptionError{Option: "token_url", Message: "token URL is required"} } diff --git a/pkg/provider/basic_test.go b/pkg/provider/basic_test.go index a988b24..63ca59d 100644 --- a/pkg/provider/basic_test.go +++ b/pkg/provider/basic_test.go @@ -32,12 +32,15 @@ func TestBasicPublic(t *testing.T) { ops := basicTest.Public("foo") - u, err := url.Parse(ops.AuthCodeURL( + authCodeURL, ok := ops.AuthCodeURL( "state", provider.WithRedirectURL("http://example.com/redirect"), provider.WithScopes{"a", "b", "c"}, provider.WithURLParams{"baz": "quux"}, - )) + ) + require.True(t, ok) + + u, err := url.Parse(authCodeURL) require.NoError(t, err) assert.Equal(t, "http", u.Scheme) diff --git a/pkg/provider/provider.go b/pkg/provider/provider.go index 2d67f37..4418c9c 100644 --- a/pkg/provider/provider.go +++ b/pkg/provider/provider.go @@ -36,7 +36,7 @@ func (o *AuthCodeURLOptions) ApplyOptions(opts []AuthCodeURLOption) { // PublicOperations defines the operations for a client that only require // knowledge of the client ID. type PublicOperations interface { - AuthCodeURL(state string, opts ...AuthCodeURLOption) string + AuthCodeURL(state string, opts ...AuthCodeURLOption) (string, bool) } // AuthCodeExchangeOptions are options for the AuthCodeExchange operation. diff --git a/pkg/testutil/mock.go b/pkg/testutil/mock.go index 651849b..f48af8d 100644 --- a/pkg/testutil/mock.go +++ b/pkg/testutil/mock.go @@ -169,7 +169,7 @@ type mockOperations struct { clientCredentialsFn MockClientCredentialsFunc } -func (mo *mockOperations) AuthCodeURL(state string, opts ...provider.AuthCodeURLOption) string { +func (mo *mockOperations) AuthCodeURL(state string, opts ...provider.AuthCodeURLOption) (string, bool) { o := &provider.AuthCodeURLOptions{} o.ApplyOptions(opts) @@ -178,7 +178,7 @@ func (mo *mockOperations) AuthCodeURL(state string, opts ...provider.AuthCodeURL Endpoint: MockEndpoint, Scopes: o.Scopes, RedirectURL: o.RedirectURL, - }).AuthCodeURL(state, o.AuthCodeOptions...) + }).AuthCodeURL(state, o.AuthCodeOptions...), true } func (mo *mockOperations) AuthCodeExchange(ctx context.Context, code string, opts ...provider.AuthCodeExchangeOption) (*provider.Token, error) {