Skip to content

Commit

Permalink
Support JSON lists for Okta user groups+policies. (#3801)
Browse files Browse the repository at this point in the history
* Support JSON lists for Okta user groups+policies.

Migrate the manually-parsed comma-separated string field types for user
groups and user policies to TypeCommaStringSlice. This means user
endpoints now accept proper lists as input for these fields in addition
to comma-separated string values. The value for reads remains a list.

Update the Okta API documentation for users and groups to reflect that
both user group and user/group policy fields are list-valued.

Update the Okta acceptance tests to cover passing a list value for the
user policy field, and require the OKTA_API_TOKEN env var to be set
(required for the "everyone" policy tests to pass).

* Fix typo, add comma-separated docs.
  • Loading branch information
jgiles authored and jefferai committed Jan 16, 2018
1 parent f320f00 commit 2b719ae
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 34 deletions.
21 changes: 13 additions & 8 deletions builtin/credential/okta/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,16 +53,16 @@ func TestBackend_Config(t *testing.T) {
testConfigCreate(t, configData),
testLoginWrite(t, username, "wrong", "E0000004", 0, nil),
testLoginWrite(t, username, password, "user is not a member of any authorized policy", 0, nil),
testAccUserGroups(t, username, "local_grouP,lOcal_group2"),
testAccUserGroups(t, username, "local_grouP,lOcal_group2", []string{"user_policy"}),
testAccGroups(t, "local_groUp", "loCal_group_policy"),
testLoginWrite(t, username, password, "", defaultLeaseTTLVal, []string{"local_group_policy"}),
testLoginWrite(t, username, password, "", defaultLeaseTTLVal, []string{"local_group_policy", "user_policy"}),
testAccGroups(t, "everyoNe", "everyone_grouP_policy,eveRy_group_policy2"),
testLoginWrite(t, username, password, "", defaultLeaseTTLVal, []string{"local_group_policy"}),
testLoginWrite(t, username, password, "", defaultLeaseTTLVal, []string{"local_group_policy", "user_policy"}),
testConfigUpdate(t, configDataToken),
testConfigRead(t, token, configData),
testLoginWrite(t, username, password, "", updatedDuration, []string{"everyone_group_policy", "every_group_policy2", "local_group_policy"}),
testLoginWrite(t, username, password, "", updatedDuration, []string{"everyone_group_policy", "every_group_policy2", "local_group_policy", "user_policy"}),
testAccGroups(t, "locAl_group2", "testgroup_group_policy"),
testLoginWrite(t, username, password, "", updatedDuration, []string{"everyone_group_policy", "every_group_policy2", "local_group_policy", "testgroup_group_policy"}),
testLoginWrite(t, username, password, "", updatedDuration, []string{"everyone_group_policy", "every_group_policy2", "local_group_policy", "testgroup_group_policy", "user_policy"}),
},
})
}
Expand Down Expand Up @@ -154,19 +154,24 @@ func testAccPreCheck(t *testing.T) {
if v := os.Getenv("OKTA_ORG"); v == "" {
t.Fatal("OKTA_ORG must be set for acceptance tests")
}

if v := os.Getenv("OKTA_API_TOKEN"); v == "" {
t.Fatal("OKTA_API_TOKEN must be set for acceptance tests")
}
}

func testAccUserGroups(t *testing.T, user string, groups string) logicaltest.TestStep {
func testAccUserGroups(t *testing.T, user string, groups interface{}, policies interface{}) logicaltest.TestStep {
return logicaltest.TestStep{
Operation: logical.UpdateOperation,
Path: "users/" + user,
Data: map[string]interface{}{
"groups": groups,
"groups": groups,
"policies": policies,
},
}
}

func testAccGroups(t *testing.T, group string, policies string) logicaltest.TestStep {
func testAccGroups(t *testing.T, group string, policies interface{}) logicaltest.TestStep {
t.Logf("[testAccGroups] - Registering group %s, policy %s", group, policies)
return logicaltest.TestStep{
Operation: logical.UpdateOperation,
Expand Down
20 changes: 6 additions & 14 deletions builtin/credential/okta/path_users.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package okta

import (
"context"
"strings"

"github.com/hashicorp/vault/logical"
"github.com/hashicorp/vault/logical/framework"
Expand Down Expand Up @@ -31,13 +30,13 @@ func pathUsers(b *backend) *framework.Path {
},

"groups": &framework.FieldSchema{
Type: framework.TypeString,
Description: "Comma-separated list of groups associated with the user.",
Type: framework.TypeCommaStringSlice,
Description: "List of groups associated with the user.",
},

"policies": &framework.FieldSchema{
Type: framework.TypeString,
Description: "Comma-separated list of policies associated with the user.",
Type: framework.TypeCommaStringSlice,
Description: "List of policies associated with the user.",
},
},

Expand Down Expand Up @@ -111,15 +110,8 @@ func (b *backend) pathUserWrite(ctx context.Context, req *logical.Request, d *fr
return logical.ErrorResponse("Error empty name"), nil
}

groups := strings.Split(d.Get("groups").(string), ",")
for i, g := range groups {
groups[i] = strings.TrimSpace(g)
}

policies := strings.Split(d.Get("policies").(string), ",")
for i, p := range policies {
policies[i] = strings.TrimSpace(p)
}
groups := d.Get("groups").([]string)
policies := d.Get("policies").([]string)

// Store it
entry, err := logical.StorageEntryJSON("user/"+name, &UserEntry{
Expand Down
33 changes: 21 additions & 12 deletions website/source/api/auth/okta/index.html.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,14 +139,15 @@ Registers a new user and maps a set of policies to it.
### Parameters

- `username` `(string: <required>)` - Name of the user.
- `groups` `(string: "")` - Comma-separated list of groups associated with the
user.
- `policies` `(string: "")` - Comma-separated list of policies associated with
the user.
- `groups` `(array: [])` - List or comma-separated string of groups associated with the user.
- `policies` `(array: [])` - List or comma-separated string of policies associated with the user.

```json
{
"policies": "dev,prod",
"policies": [
"dev",
"prod"
]
}
```

Expand Down Expand Up @@ -189,8 +190,11 @@ $ curl \
"lease_duration": 0,
"renewable": false,
"data": {
"policies": "default,dev",
"groups": ""
"policies": [
"default",
"dev",
],
"groups": []
},
"warnings": null
}
Expand Down Expand Up @@ -244,7 +248,7 @@ $ curl \
"data": {
"keys": [
"admins",
"dev-users"
"dev-users"
]
},
"lease_duration": 0,
Expand All @@ -264,12 +268,14 @@ Registers a new group and maps a set of policies to it.
### Parameters

- `name` `(string: <required>)` - The name of the group.
- `policies` `(string: "")` - Comma-separated list of policies associated with
the group.
- `policies` `(array: [])` - The list or comma-separated string of policies associated with the group.

```json
{
"policies": "dev,prod",
"policies": [
"dev",
"prod"
]
}
```

Expand Down Expand Up @@ -312,7 +318,10 @@ $ curl \
"lease_duration": 0,
"renewable": false,
"data": {
"policies": "default,admin"
"policies": [
"default",
"admin"
]
},
"warnings": null
}
Expand Down

0 comments on commit 2b719ae

Please sign in to comment.