Skip to content

Commit

Permalink
Vault: path changes & rotation fix (#103)
Browse files Browse the repository at this point in the history
Vault: path changes & rotation fix

Path has been changed for:

GET|POST|PUT|DELETE  /openstack/cloud/:name
GET|POST|PUT|DELETE  /openstack/role/:name
GET|POST|PUT|DELETE  /openstack/static-role/:name

Closes: #102
User password change has been fixed on rotation
Acceptance tests

Running acceptance tests...
=== RUN   TestPlugin
=== RUN   TestPlugin/TestCloudLifecycle
=== RUN   TestPlugin/TestCloudLifecycle/WriteCloud
=== RUN   TestPlugin/TestCloudLifecycle/ReadCloud
=== RUN   TestPlugin/TestCloudLifecycle/ListClouds
=== RUN   TestPlugin/TestCloudLifecycle/ListClouds/method-LIST
=== PAUSE TestPlugin/TestCloudLifecycle/ListClouds/method-LIST
=== RUN   TestPlugin/TestCloudLifecycle/ListClouds/method-GET
=== PAUSE TestPlugin/TestCloudLifecycle/ListClouds/method-GET
=== CONT  TestPlugin/TestCloudLifecycle/ListClouds/method-LIST
=== CONT  TestPlugin/TestCloudLifecycle/ListClouds/method-GET
=== RUN   TestPlugin/TestCloudLifecycle/DeleteCloud
=== RUN   TestPlugin/TestCredsLifecycle
=== RUN   TestPlugin/TestCredsLifecycle/user_token
=== RUN   TestPlugin/TestCredsLifecycle/user_password
=== RUN   TestPlugin/TestCredsLifecycle/root_token
=== RUN   TestPlugin/TestInfo
info_test.go:42:
Error Trace:    info_test.go:42
Error:          Should NOT be empty, but was &{    }
Test:           TestPlugin/TestInfo
=== RUN   TestPlugin/TestRoleLifecycle
roles_test.go:53: Cloud with name 0agdgjaxfo was created
=== RUN   TestPlugin/TestRoleLifecycle/WriteRole
=== RUN   TestPlugin/TestRoleLifecycle/ReadRole
=== RUN   TestPlugin/TestRoleLifecycle/ListRoles
=== RUN   TestPlugin/TestRoleLifecycle/ListRoles/method-LIST
=== PAUSE TestPlugin/TestRoleLifecycle/ListRoles/method-LIST
=== RUN   TestPlugin/TestRoleLifecycle/ListRoles/method-GET
=== PAUSE TestPlugin/TestRoleLifecycle/ListRoles/method-GET
=== CONT  TestPlugin/TestRoleLifecycle/ListRoles/method-LIST
=== CONT  TestPlugin/TestRoleLifecycle/ListRoles/method-GET
=== RUN   TestPlugin/TestRoleLifecycle/DeleteRole
=== CONT  TestPlugin/TestRoleLifecycle
plugin_test.go:337: Cloud with name 0agdgjaxfo has been removed
=== RUN   TestPlugin/TestRootRotate
rotate_test.go:65: Cloud with name default1 was created
rotate_test.go:68: Cloud with name xspk was created
plugin_test.go:337: Cloud with name xspk has been removed
plugin_test.go:337: Cloud with name default1 has been removed
=== RUN   TestPlugin/TestStaticCredsLifecycle
=== RUN   TestPlugin/TestStaticCredsLifecycle/user_password
=== RUN   TestPlugin/TestStaticCredsLifecycle/user_token_project_id
=== RUN   TestPlugin/TestStaticCredsLifecycle/user_token_project_name
=== RUN   TestPlugin/TestStaticRoleLifecycle
=== RUN   TestPlugin/TestStaticRoleLifecycle/WriteRole
=== RUN   TestPlugin/TestStaticRoleLifecycle/ReadRole
=== RUN   TestPlugin/TestStaticRoleLifecycle/ListRoles
=== RUN   TestPlugin/TestStaticRoleLifecycle/ListRoles/method-LIST
=== PAUSE TestPlugin/TestStaticRoleLifecycle/ListRoles/method-LIST
=== RUN   TestPlugin/TestStaticRoleLifecycle/ListRoles/method-GET
=== PAUSE TestPlugin/TestStaticRoleLifecycle/ListRoles/method-GET
=== CONT  TestPlugin/TestStaticRoleLifecycle/ListRoles/method-LIST
=== CONT  TestPlugin/TestStaticRoleLifecycle/ListRoles/method-GET
=== RUN   TestPlugin/TestStaticRoleLifecycle/DeleteRole
--- FAIL: TestPlugin (30.37s)
--- PASS: TestPlugin/TestCloudLifecycle (0.09s)
--- PASS: TestPlugin/TestCloudLifecycle/WriteCloud (0.08s)
--- PASS: TestPlugin/TestCloudLifecycle/ReadCloud (0.00s)
--- PASS: TestPlugin/TestCloudLifecycle/ListClouds (0.00s)
--- PASS: TestPlugin/TestCloudLifecycle/ListClouds/method-LIST (0.00s)
--- PASS: TestPlugin/TestCloudLifecycle/ListClouds/method-GET (0.00s)
--- PASS: TestPlugin/TestCloudLifecycle/DeleteCloud (0.00s)
--- PASS: TestPlugin/TestCredsLifecycle (6.41s)
--- PASS: TestPlugin/TestCredsLifecycle/user_token (3.39s)
--- PASS: TestPlugin/TestCredsLifecycle/user_password (1.07s)
--- PASS: TestPlugin/TestCredsLifecycle/root_token (0.93s)
--- FAIL: TestPlugin/TestInfo (0.00s)
--- PASS: TestPlugin/TestRoleLifecycle (0.01s)
--- PASS: TestPlugin/TestRoleLifecycle/WriteRole (0.00s)
--- PASS: TestPlugin/TestRoleLifecycle/ReadRole (0.00s)
--- PASS: TestPlugin/TestRoleLifecycle/ListRoles (0.00s)
--- PASS: TestPlugin/TestRoleLifecycle/ListRoles/method-LIST (0.00s)
--- PASS: TestPlugin/TestRoleLifecycle/ListRoles/method-GET (0.00s)
--- PASS: TestPlugin/TestRoleLifecycle/DeleteRole (0.00s)
--- PASS: TestPlugin/TestRootRotate (5.36s)
--- PASS: TestPlugin/TestStaticCredsLifecycle (15.22s)
--- PASS: TestPlugin/TestStaticCredsLifecycle/user_password (5.69s)
--- PASS: TestPlugin/TestStaticCredsLifecycle/user_token_project_id (4.20s)
--- PASS: TestPlugin/TestStaticCredsLifecycle/user_token_project_name (4.16s)
--- PASS: TestPlugin/TestStaticRoleLifecycle (3.13s)
--- PASS: TestPlugin/TestStaticRoleLifecycle/WriteRole (1.13s)
--- PASS: TestPlugin/TestStaticRoleLifecycle/ReadRole (0.00s)
--- PASS: TestPlugin/TestStaticRoleLifecycle/ListRoles (0.00s)
--- PASS: TestPlugin/TestStaticRoleLifecycle/ListRoles/method-LIST (0.00s)
--- PASS: TestPlugin/TestStaticRoleLifecycle/ListRoles/method-GET (0.00s)
--- PASS: TestPlugin/TestStaticRoleLifecycle/DeleteRole (0.00s)
FAIL
FAIL    github.com/opentelekomcloud/vault-plugin-secrets-openstack/acceptance   31.055s
FAIL
make: *** [functional] Error 1

Reviewed-by: Aloento <None>
Reviewed-by: Anton Sidelnikov <None>
  • Loading branch information
artem-lifshits authored Aug 23, 2022
1 parent 069a6af commit 9f567c6
Show file tree
Hide file tree
Showing 14 changed files with 60 additions and 70 deletions.
2 changes: 1 addition & 1 deletion acceptance/cloud_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,5 +101,5 @@ func (p *PluginTest) TestCloudLifecycle() {
var cloudsListURL = "/v1/openstack/clouds"

func cloudURL(name string) string {
return fmt.Sprintf("/v1/openstack/cloud/%s", name)
return fmt.Sprintf("/v1/openstack/clouds/%s", name)
}
2 changes: 1 addition & 1 deletion acceptance/plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ var (
pluginMountEndpoint = fmt.Sprintf("/v1/sys/mounts/%s", pluginAlias)
pluginPwdPolicyEndpoint = fmt.Sprintf("/v1/sys/policies/password/%s", policyAlias)

cloudBaseEndpoint = fmt.Sprintf("/v1/%s/cloud", pluginAlias)
cloudBaseEndpoint = fmt.Sprintf("/v1/%s/clouds", pluginAlias)
)

type vaultCfg struct {
Expand Down
2 changes: 1 addition & 1 deletion acceptance/roles_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func (p *PluginTest) TestRoleLifecycle() {
}

func roleURL(name string) string {
return fmt.Sprintf("/v1/openstack/role/%s", name)
return fmt.Sprintf("/v1/openstack/roles/%s", name)
}

func expectedRoleData(cloudName string) map[string]interface{} {
Expand Down
2 changes: 1 addition & 1 deletion acceptance/static_roles_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func (p *PluginTest) TestStaticRoleLifecycle() {
}

func staticRoleURL(name string) string {
return fmt.Sprintf("/v1/openstack/static-role/%s", name)
return fmt.Sprintf("/v1/openstack/static-roles/%s", name)
}

func expectedStaticRoleData(cloudName string, aux *AuxiliaryData) map[string]interface{} {
Expand Down
46 changes: 23 additions & 23 deletions doc/source/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@
This endpoint configures the root credentials to communicate with OpenStack instance. If credentials already exist, this
will overwrite them.

| Method | Path |
|:-------|:--------------------------|
| `POST` | `/openstack/cloud/:cloud` |
| `PUT` | `/openstack/cloud/:cloud` |
| Method | Path |
|:-------|:---------------------------|
| `POST` | `/openstack/clouds/:cloud` |
| `PUT` | `/openstack/clouds/:cloud` |

### Parameters

Expand Down Expand Up @@ -50,24 +50,24 @@ $ curl \
--header "X-Vault-Token: ..." \
--request POST \
--data @payload.json \
http://127.0.0.1:8200/v1/openstack/cloud/example-cloud
http://127.0.0.1:8200/v1/openstack/clouds/example-cloud
```

## Read Root Configuration

This endpoint allows you to read non-secure values that have been set in the `cloud/:cloud` endpoint.
In particular, the `password` parameter is never returned.

| Method | Path |
|:-------|:--------------------------|
| `GET` | `/openstack/cloud/:cloud` |
| Method | Path |
|:-------|:---------------------------|
| `GET` | `/openstack/clouds/:cloud` |

### Sample Request

```shell
$ curl \
--header "X-Vault-Token: ..." \
http://127.0.0.1:8200/v1/openstack/cloud/example-cloud
http://127.0.0.1:8200/v1/openstack/clouds/example-cloud
```

### Sample Response
Expand Down Expand Up @@ -138,10 +138,10 @@ $ curl \
This endpoint creates or updates the role with the given `name`. If a role with the name does not exist, it will be
created. If the role exists, it will be updated with the new attributes.

| Method | Path |
|:-------|:------------------------|
| `POST` | `/openstack/role/:name` |
| `PUT` | `/openstack/role/:name` |
| Method | Path |
|:-------|:-------------------------|
| `POST` | `/openstack/roles/:name` |
| `PUT` | `/openstack/roles/:name` |

### Parameters

Expand Down Expand Up @@ -194,7 +194,7 @@ $ curl \
--header "X-Vault-Token: ..." \
--request POST \
--data @payload.json \
http://127.0.0.1:8200/v1/openstack/role/example-role
http://127.0.0.1:8200/v1/openstack/roles/example-role
```

### Sample Payload
Expand Down Expand Up @@ -274,9 +274,9 @@ or

This endpoint queries an existing role by the given name. If the role does not exist, a 404 is returned.

| Method | Path |
|:---------|:------------------------|
| `GET` | `/openstack/role/:name` |
| Method | Path |
|:---------|:-------------------------|
| `GET` | `/openstack/roles/:name` |

### Parameters

Expand All @@ -287,7 +287,7 @@ This endpoint queries an existing role by the given name. If the role does not e
```shell
$ curl \
--header "X-Vault-Token: ..." \
http://127.0.0.1:8200/v1/openstack/role/example-role
http://127.0.0.1:8200/v1/openstack/roles/example-role
```

### Sample Response
Expand Down Expand Up @@ -427,10 +427,10 @@ $ curl \
This endpoint creates or updates the static role with the given `name`. If a role with the name does not exist, it will be
created. If the role exists, it will be updated with the new attributes.

| Method | Path |
|:-------|:-------------------------------|
| `POST` | `/openstack/static-role/:name` |
| `PUT` | `/openstack/static-role/:name` |
| Method | Path |
|:-------|:--------------------------------|
| `POST` | `/openstack/static-roles/:name` |
| `PUT` | `/openstack/static-roles/:name` |

### Parameters

Expand Down Expand Up @@ -473,7 +473,7 @@ $ curl \
--header "X-Vault-Token: ..." \
--request POST \
--data @payload.json \
http://127.0.0.1:8200/v1/openstack/static-role/example-role
http://127.0.0.1:8200/v1/openstack/static-roles/example-role
```

### Sample Payload
Expand Down
43 changes: 20 additions & 23 deletions openstack/fixtures/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -332,28 +332,25 @@ func SetupKeystoneMock(t *testing.T, userID, projectName string, enabled Enabled
})
}

if enabled.UserGet {
th.Mux.HandleFunc(fmt.Sprintf("/v3/users/%s", userID), func(w http.ResponseWriter, r *http.Request) {
th.TestMethod(t, r, "GET")

handleGetUser(t, w, r, userID)
})
}

if enabled.UserPatch {
th.Mux.HandleFunc(fmt.Sprintf("/v3/users/%s", userID), func(w http.ResponseWriter, r *http.Request) {
th.TestMethod(t, r, "PATCH")

handleUpdateUser(t, w, r, userID)
})
}

if enabled.UserDelete {
th.Mux.HandleFunc(fmt.Sprintf("/v3/users/%s", userID), func(w http.ResponseWriter, r *http.Request) {
th.TestHeader(t, r, "Accept", "application/json")
th.TestMethod(t, r, "DELETE")
th.Mux.HandleFunc(fmt.Sprintf("/v3/users/%s", userID), func(w http.ResponseWriter, r *http.Request) {
switch r.Method {
case "PATCH":
if enabled.UserPatch {
handleUpdateUser(t, w, r, userID)
}
case "GET":
if enabled.UserGet {
handleGetUser(t, w, r, userID)
}
case "DELETE":
if enabled.UserDelete {
th.TestHeader(t, r, "Accept", "application/json")
th.TestMethod(t, r, "DELETE")

w.WriteHeader(http.StatusNoContent)
})
}
w.WriteHeader(http.StatusNoContent)
}
default:
w.WriteHeader(404)
}
})
}
2 changes: 1 addition & 1 deletion openstack/path_cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
)

const (
pathCloud = "cloud"
pathCloud = "clouds"
pathClouds = "clouds/?"
cloudsStoragePath = "clouds"

Expand Down
2 changes: 1 addition & 1 deletion openstack/path_role.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ which are used to control permissions to OpenStack resources.
)

var (
pathRole = fmt.Sprintf("role/%s", framework.GenericNameRegex("name"))
pathRole = fmt.Sprintf("roles/%s", framework.GenericNameRegex("name"))

errRoleGet = errors.New("error searching for the role")
)
Expand Down
2 changes: 1 addition & 1 deletion openstack/path_role_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
)

func rolePath(name string) string {
return fmt.Sprintf("%s/%s", "role", name)
return fmt.Sprintf("%s/%s", "roles", name)
}

func TestRoleStoragePath(t *testing.T) {
Expand Down
2 changes: 0 additions & 2 deletions openstack/path_rotate_root_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@ func TestRotateRootCredentials_ok(t *testing.T) {
}

func TestRotateRootCredentials_error(t *testing.T) {
t.Parallel()

t.Run("read-fail", func(t *testing.T) {
userID, _ := uuid.GenerateUUID()
projectName := tools.RandomString("p", 5)
Expand Down
7 changes: 2 additions & 5 deletions openstack/path_static_creds.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,12 +170,9 @@ func (b *backend) rotateStaticCreds(ctx context.Context, r *logical.Request, d *
return nil, err
}

err = users.ChangePassword(client, role.UserID, users.ChangePasswordOpts{
Password: newPassword,
OriginalPassword: role.Secret,
}).ExtractErr()
_, err = users.Update(client, role.UserID, users.UpdateOpts{Password: newPassword}).Extract()
if err != nil {
return nil, err
return nil, fmt.Errorf("error rotating user password for user `%s`: %s", role.Username, err)
}

role.Secret = newPassword
Expand Down
14 changes: 6 additions & 8 deletions openstack/path_static_creds_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,9 @@ func TestRotateStaticCredentials_ok(t *testing.T) {
projectName := tools.RandomString("p", 5)

fixtures.SetupKeystoneMock(t, userID, projectName, fixtures.EnabledMocks{
TokenPost: true,
TokenGet: true,
PasswordChange: true,
UserGet: true,
TokenPost: true,
TokenGet: true,
UserPatch: true,
})

testClient := thClient.ServiceClient()
Expand Down Expand Up @@ -194,13 +193,12 @@ func TestRotateStaticCredentials_ok(t *testing.T) {

roleName := createSaveRandomStaticRole(t, s, projectName, "password", secret, userID)

res, err := b.HandleRequest(context.Background(), &logical.Request{
Operation: logical.ReadOperation,
Path: credsStaticPath(roleName),
_, err := b.HandleRequest(context.Background(), &logical.Request{
Operation: logical.CreateOperation,
Path: rotateStaticCreds(roleName),
Storage: s,
})
require.NoError(t, err)
require.NotEmpty(t, res.Data)
})
}

Expand Down
2 changes: 1 addition & 1 deletion openstack/path_static_role.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ which are used to control permissions to OpenStack resources.
)

var (
staticPathRole = fmt.Sprintf("static-role/%s", framework.GenericNameRegex("name"))
staticPathRole = fmt.Sprintf("static-roles/%s", framework.GenericNameRegex("name"))
)

func (b *backend) pathStaticRoles() *framework.Path {
Expand Down
2 changes: 1 addition & 1 deletion openstack/path_static_role_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
)

func staticRolePath(name string) string {
return fmt.Sprintf("%s/%s", "static-role", name)
return fmt.Sprintf("%s/%s", "static-roles", name)
}

func TestStaticRoleStoragePath(t *testing.T) {
Expand Down

0 comments on commit 9f567c6

Please sign in to comment.