From 6e43576bf4d606ec7074f2aa8e44acf3da366c3f Mon Sep 17 00:00:00 2001 From: Josh Giles Date: Tue, 16 Jan 2018 08:03:46 -0500 Subject: [PATCH 1/2] 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). --- builtin/credential/okta/backend_test.go | 21 ++++++++------ builtin/credential/okta/path_users.go | 20 ++++--------- website/source/api/auth/okta/index.html.md | 33 ++++++++++++++-------- 3 files changed, 40 insertions(+), 34 deletions(-) diff --git a/builtin/credential/okta/backend_test.go b/builtin/credential/okta/backend_test.go index 9c2503d87fdb..cb476f2d9183 100644 --- a/builtin/credential/okta/backend_test.go +++ b/builtin/credential/okta/backend_test.go @@ -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"}), }, }) } @@ -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, diff --git a/builtin/credential/okta/path_users.go b/builtin/credential/okta/path_users.go index 9036a40214eb..17e9f7a8d05f 100644 --- a/builtin/credential/okta/path_users.go +++ b/builtin/credential/okta/path_users.go @@ -2,7 +2,6 @@ package okta import ( "context" - "strings" "github.com/hashicorp/vault/logical" "github.com/hashicorp/vault/logical/framework" @@ -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.", }, }, @@ -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{ diff --git a/website/source/api/auth/okta/index.html.md b/website/source/api/auth/okta/index.html.md index a66f4177fa0e..5fef0a381f2c 100644 --- a/website/source/api/auth/okta/index.html.md +++ b/website/source/api/auth/okta/index.html.md @@ -139,14 +139,15 @@ Registers a new user and maps a set of policies to it. ### Parameters - `username` `(string: )` - 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 of groups associated with the user. +- `policies` `(array: [])` - List of policies associated with the user. ```json { - "policies": "dev,prod", + "policies": [ + "dev", + "prod" + ] } ``` @@ -189,8 +190,11 @@ $ curl \ "lease_duration": 0, "renewable": false, "data": { - "policies": "default,dev", - "groups": "" + "policies": [ + "default", + "dev", + ], + "groups": [] }, "warnings": null } @@ -244,7 +248,7 @@ $ curl \ "data": { "keys": [ "admins", - "dev-users" + "dev-users" ] }, "lease_duration": 0, @@ -264,12 +268,14 @@ Registers a new group and maps a set of policies to it. ### Parameters - `name` `(string: )` - The name of the group. -- `policies` `(string: "")` - Comma-separated list of policies associated with - the group. +- `policies` `(policies: [])` - The list of policies associated with the group. ```json { - "policies": "dev,prod", + "policies": [ + "dev", + "prod" + ] } ``` @@ -312,7 +318,10 @@ $ curl \ "lease_duration": 0, "renewable": false, "data": { - "policies": "default,admin" + "policies": [ + "default", + "admin" + ] }, "warnings": null } From b2b1baaf4c3da7978d85d5512a10e8f25219130d Mon Sep 17 00:00:00 2001 From: Josh Giles Date: Tue, 16 Jan 2018 18:18:45 -0500 Subject: [PATCH 2/2] Fix typo, add comma-separated docs. --- website/source/api/auth/okta/index.html.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/website/source/api/auth/okta/index.html.md b/website/source/api/auth/okta/index.html.md index 5fef0a381f2c..6d3dfd5d00d9 100644 --- a/website/source/api/auth/okta/index.html.md +++ b/website/source/api/auth/okta/index.html.md @@ -139,8 +139,8 @@ Registers a new user and maps a set of policies to it. ### Parameters - `username` `(string: )` - Name of the user. -- `groups` `(array: [])` - List of groups associated with the user. -- `policies` `(array: [])` - 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 { @@ -268,7 +268,7 @@ Registers a new group and maps a set of policies to it. ### Parameters - `name` `(string: )` - The name of the group. -- `policies` `(policies: [])` - The list of policies associated with the group. +- `policies` `(array: [])` - The list or comma-separated string of policies associated with the group. ```json {