Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Normalize plugin_name option for mount and enable-auth #3202

Merged
merged 4 commits into from
Aug 31, 2017
Merged

Normalize plugin_name option for mount and enable-auth #3202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6935b52
Add top-level and config-level plugin_name option for mount and enabl…
calvn Aug 17, 2017
441ed3c
Fix config structs tag
calvn Aug 18, 2017
f7bb2ff
Update api docs
calvn Aug 18, 2017
f71b8e6
Merge branch 'master' into plugin-config
jefferai Aug 31, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions api/sys_auth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,15 @@ func (c *Sys) DisableAuth(path string) error {
// documentation. Please refer to that documentation for more details.

type EnableAuthOptions struct {
Type string `json:"type" structs:"type"`
Description string `json:"description" structs:"description"`
Local bool `json:"local" structs:"local"`
PluginName string `json:"plugin_name,omitempty" structs:"plugin_name,omitempty" mapstructure:"plugin_name"`
Type string `json:"type" structs:"type"`
Description string `json:"description" structs:"description"`
Config AuthConfigInput `json:"config" structs:"config"`
Local bool `json:"local" structs:"local"`
PluginName string `json:"plugin_name,omitempty" structs:"plugin_name,omitempty"`
}

type AuthConfigInput struct {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are arguments against it (mostly being, maybe what we need will diverge in the future) but it seems like simply reusing MountConfigInput could be good here. This allows the same structure to be used for tuning both (ttl, maxttl, updated plugin name, etc.).

Copy link
Contributor Author

@calvn calvn Aug 31, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is ForceNoCache a property of auth mounts as well? Can the same be said for MountConfigOutput and AuthConfigOutput?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is. Most mounts do nothing with it, but it is technically able to be set. So -- yes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about the second question, should we use MountConfigOutput for both cases then since the config params should be the same?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that this would be an API change. But -- then we're inconsistent because you get an AuthConfigOutput with a MountConfigInput whereas for secrets you get MountConfigInput/MountConfigOutput.

Part of me says "this is a silly situation to be in already and let's just break it and be done with it" but...we probably shouldn't. So I guess, leave AuthConfigInput after all.

PluginName string `json:"plugin_name,omitempty" structs:"plugin_name,omitempty" mapstructure:"plugin_name"`
}

type AuthMount struct {
Expand Down
1 change: 1 addition & 0 deletions api/sys_mounts.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ type MountInput struct {
Description string `json:"description" structs:"description"`
Config MountConfigInput `json:"config" structs:"config"`
Local bool `json:"local" structs:"local"`
PluginName string `json:"plugin_name,omitempty" structs:"plugin_name"`
}

type MountConfigInput struct {
Expand Down
6 changes: 4 additions & 2 deletions command/auth_enable.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,10 @@ func (c *AuthEnableCommand) Run(args []string) int {
if err := client.Sys().EnableAuthWithOptions(path, &api.EnableAuthOptions{
Type: authType,
Description: description,
PluginName: pluginName,
Local: local,
Config: api.AuthConfigInput{
PluginName: pluginName,
},
Local: local,
}); err != nil {
c.Ui.Error(fmt.Sprintf(
"Error: %s", err))
Expand Down
67 changes: 58 additions & 9 deletions vault/logical_system.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,10 @@ func NewSystemBackend(core *Core) *SystemBackend {
Default: false,
Description: strings.TrimSpace(sysHelp["mount_local"][0]),
},
"plugin_name": &framework.FieldSchema{
Type: framework.TypeString,
Description: strings.TrimSpace(sysHelp["mount_plugin_name"][0]),
},
},

Callbacks: map[logical.Operation]framework.OperationFunc{
Expand Down Expand Up @@ -493,15 +497,19 @@ func NewSystemBackend(core *Core) *SystemBackend {
Type: framework.TypeString,
Description: strings.TrimSpace(sysHelp["auth_desc"][0]),
},
"plugin_name": &framework.FieldSchema{
Type: framework.TypeString,
Description: strings.TrimSpace(sysHelp["auth_plugin"][0]),
"config": &framework.FieldSchema{
Type: framework.TypeMap,
Description: strings.TrimSpace(sysHelp["auth_config"][0]),
},
"local": &framework.FieldSchema{
Type: framework.TypeBool,
Default: false,
Description: strings.TrimSpace(sysHelp["mount_local"][0]),
},
"plugin_name": &framework.FieldSchema{
Type: framework.TypeString,
Description: strings.TrimSpace(sysHelp["auth_plugin"][0]),
},
},

Callbacks: map[logical.Operation]framework.OperationFunc{
Expand Down Expand Up @@ -1256,6 +1264,7 @@ func (b *SystemBackend) handleMount(
path := data.Get("path").(string)
logicalType := data.Get("type").(string)
description := data.Get("description").(string)
pluginName := data.Get("plugin_name").(string)

path = sanitizeMountPath(path)

Expand Down Expand Up @@ -1310,9 +1319,19 @@ func (b *SystemBackend) handleMount(
logical.ErrInvalidRequest
}

// Only set plugin-name if mount is of type plugin
if logicalType == "plugin" && apiConfig.PluginName != "" {
config.PluginName = apiConfig.PluginName
// Only set plugin-name if mount is of type plugin, with apiConfig.PluginName
// option taking precedence.
if logicalType == "plugin" {
switch {
case apiConfig.PluginName != "":
config.PluginName = apiConfig.PluginName
case pluginName != "":
config.PluginName = pluginName
default:
return logical.ErrorResponse(
"plugin_name must be provided for plugin backend"),
logical.ErrInvalidRequest
}
}

// Copy over the force no cache if set
Expand Down Expand Up @@ -1754,10 +1773,31 @@ func (b *SystemBackend) handleEnableAuth(
pluginName := data.Get("plugin_name").(string)

var config MountConfig
var apiConfig APIMountConfig

// Only set plugin name if mount is of type plugin
if logicalType == "plugin" && pluginName != "" {
config.PluginName = pluginName
configMap := data.Get("config").(map[string]interface{})
if configMap != nil && len(configMap) != 0 {
err := mapstructure.Decode(configMap, &apiConfig)
if err != nil {
return logical.ErrorResponse(
"unable to convert given auth config information"),
logical.ErrInvalidRequest
}
}

// Only set plugin name if mount is of type plugin, with apiConfig.PluginName
// option taking precedence.
if logicalType == "plugin" {
switch {
case apiConfig.PluginName != "":
config.PluginName = apiConfig.PluginName
case pluginName != "":
config.PluginName = pluginName
default:
return logical.ErrorResponse(
"plugin_name must be provided for plugin backend"),
logical.ErrInvalidRequest
}
}

if logicalType == "" {
Expand Down Expand Up @@ -2541,6 +2581,11 @@ and max_lease_ttl.`,
and is unaffected by replication.`,
},

"mount_plugin_name": {
`Name of the plugin to mount based from the name registered
in the plugin catalog.`,
},

"tune_default_lease_ttl": {
`The default lease TTL for this mount.`,
},
Expand Down Expand Up @@ -2677,6 +2722,10 @@ Example: you might have an OAuth backend for GitHub, and one for Google Apps.
"",
},

"auth_config": {
`Configuration for this mount, such as plugin_name.`,
},

"auth_plugin": {
`Name of the auth plugin to use based from the name in the plugin catalog.`,
"",
Expand Down
34 changes: 24 additions & 10 deletions vault/logical_system_integ_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@ import (
)

func TestSystemBackend_Plugin_secret(t *testing.T) {
cluster := testSystemBackendMock(t, 1, logical.TypeLogical)
cluster := testSystemBackendMock(t, 2, logical.TypeLogical)
defer cluster.Cleanup()
}

func TestSystemBackend_Plugin_auth(t *testing.T) {
cluster := testSystemBackendMock(t, 1, logical.TypeCredential)
cluster := testSystemBackendMock(t, 2, logical.TypeCredential)
defer cluster.Cleanup()
}

Expand Down Expand Up @@ -148,12 +148,18 @@ func testSystemBackendMock(t *testing.T, numMounts int, backendType logical.Back
case logical.TypeLogical:
vault.TestAddTestPlugin(t, core.Core, "mock-plugin", "TestBackend_PluginMainLogical")
for i := 0; i < numMounts; i++ {
resp, err := client.Logical().Write(fmt.Sprintf("sys/mounts/mock-%d", i), map[string]interface{}{
// Alternate input styles for plugin_name on every other mount
options := map[string]interface{}{
"type": "plugin",
"config": map[string]interface{}{
}
if (i+1)%2 == 0 {
options["config"] = map[string]interface{}{
"plugin_name": "mock-plugin",
},
})
}
} else {
options["plugin_name"] = "mock-plugin"
}
resp, err := client.Logical().Write(fmt.Sprintf("sys/mounts/mock-%d", i), options)
if err != nil {
t.Fatalf("err: %v", err)
}
Expand All @@ -164,10 +170,18 @@ func testSystemBackendMock(t *testing.T, numMounts int, backendType logical.Back
case logical.TypeCredential:
vault.TestAddTestPlugin(t, core.Core, "mock-plugin", "TestBackend_PluginMainCredentials")
for i := 0; i < numMounts; i++ {
resp, err := client.Logical().Write(fmt.Sprintf("sys/auth/mock-%d", i), map[string]interface{}{
"type": "plugin",
"plugin_name": "mock-plugin",
})
// Alternate input styles for plugin_name on every other mount
options := map[string]interface{}{
"type": "plugin",
}
if (i+1)%2 == 0 {
options["config"] = map[string]interface{}{
"plugin_name": "mock-plugin",
}
} else {
options["plugin_name"] = "mock-plugin"
}
resp, err := client.Logical().Write(fmt.Sprintf("sys/auth/mock-%d", i), options)
if err != nil {
t.Fatalf("err: %v", err)
}
Expand Down
15 changes: 12 additions & 3 deletions website/source/api/system/auth.html.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,16 +74,25 @@ For example, mounting the "foo" auth backend will make it accessible at
- `type` `(string: <required>)` – Specifies the name of the authentication
backend type, such as "github" or "token".

- `config` `(map<string|string>: nil)` – Specifies configuration options for
this mount. These are the possible values:

- `plugin_name`

The plugin_name can be provided in the config map or as a top-level option,
with the former taking precedence.

- `plugin_name` `(string: "")` – Specifies the name of the auth plugin to
use based from the name in the plugin catalog. Applies only to plugin
backends.

Additionally, the following options are allowed in Vault open-source, but
relevant functionality is only supported in Vault Enterprise:

- `local` `(bool: false)` – Specifies if the auth backend is a local mount
only. Local mounts are not replicated nor (if a secondary) removed by
replication.

- `plugin_name` `(string: "")` – Specifies the name of the auth plugin to
use based from the name in the plugin catalog.

### Sample Payload

```json
Expand Down
14 changes: 10 additions & 4 deletions website/source/api/system/mounts.html.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,16 +74,22 @@ This endpoint mounts a new secret backend at the given path.
mount.

- `config` `(map<string|string>: nil)` – Specifies configuration options for
this mount. This is an object with three possible values:
this mount. This is an object with four possible values:

- `default_lease_ttl`
- `max_lease_ttl`
- `force_no_cache`
- `plugin_name`

These control the default and maximum lease time-to-live, and force
disabling backend caching respectively. If set on a specific mount, this
overrides the global defaults.
These control the default and maximum lease time-to-live, force
disabling backend caching, and option plugin name for plugin backends
respectively. The first three options override the global defaults if
set on a specific mount. The plugin_name can be provided in the config
map or as a top-level option, with the former taking precedence.

- `plugin_name` `(string: "")` – Specifies the name of the plugin to
use based from the name in the plugin catalog. Applies only to plugin
backends.

Additionally, the following options are allowed in Vault open-source, but
relevant functionality is only supported in Vault Enterprise:
Expand Down