Functions for API key rotation now support fully-qualified IDs

CHANGELOG.md

@@ -21,8 +21,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ### Changed
 - Resource IDs can now be partially-qualified, adhering to the form
-  [<account>:]<kind>:<identifier>.
+  `[<account>:]<kind>:<identifier>`.
   [cyberark/conjur-api-go#153](https://github.com/cyberark/conjur-api-go/pull/153)
+- User and Host IDs passed to their respective API key rotation functions can
+  now be fully-qualified, adhering to the form `[[<account>:]<kind>:]<identifier>`.
+  [cyberark/conjur-api-go#166](https://github.com/cyberark/conjur-api-go/pull/166)
 - The Hostfactory id is no longer required to be a fully qualified id.
   [cyberark/conjur-api-go#164](https://github.com/cyberark/conjur-api-go/pull/164)
 

conjurapi/authn.go

@@ -269,7 +269,8 @@ func (c *Client) ListOidcProviders() ([]OidcProvider, error) {
 }
 
 // RotateAPIKey replaces the API key of a role on the server with a new
-// random secret.
+// random secret. Given that a fully-qualified resource id resembles
+// '<account>:<kind>:<identifier>', argument roleID must be at least partially-qualified.
 //
 // The authenticated user must have update privilege on the role.
 func (c *Client) RotateAPIKey(roleID string) ([]byte, error) {
@@ -296,23 +297,34 @@ func (c *Client) RotateCurrentUserAPIKey() ([]byte, error) {
 }
 
 // RotateUserAPIKey constructs a role ID from a given user ID then replaces the
-// API key of the role with a new random secret.
+// API key of the role with a new random secret. Given that a fully-qualified
+// resource ID resembles '<account>:<kind>:<identifier>', argument userID will
+// be accepted as either fully- or partially-qualified, but the provided role
+// must be a user.
 //
 // The authenticated user must have update privilege on the role.
 func (c *Client) RotateUserAPIKey(userID string) ([]byte, error) {
-	config := c.GetConfig()
-	roleID := fmt.Sprintf("%s:user:%s", config.Account, userID)
-	return c.RotateAPIKey(roleID)
+	return c.rotateApiKeyAndEnforceKind(userID, "user")
 }
 
 // RotateHostAPIKey constructs a role ID from a given host ID then replaces the
-// API key of the role with a new random secret.
+// API key of the role with a new random secret. Given that a fully-qualified
+// resource ID resembles '<account>:<kind>:<identifier>', argument hostID will
+// be accepted as either fully- or partially-qualified, but the provided role
+// must be a host.
 //
 // The authenticated user must have update privilege on the role.
 func (c *Client) RotateHostAPIKey(hostID string) ([]byte, error) {
-	config := c.GetConfig()
-	roleID := fmt.Sprintf("%s:host:%s", config.Account, hostID)
+	return c.rotateApiKeyAndEnforceKind(hostID, "host")
+}
+
+func (c *Client) rotateApiKeyAndEnforceKind(roleID, kind string) ([]byte, error) {
+	account, kind, identifier, err := c.parseIDandEnforceKind(roleID, kind)
+	if err != nil {
+		return nil, err
+	}
 
+	roleID = fmt.Sprintf("%s:%s:%s", account, kind, identifier)
 	return c.RotateAPIKey(roleID)
 }
 

conjurapi/authn_test.go

@@ -44,6 +44,12 @@ func TestClient_RotateAPIKey(t *testing.T) {
 			login:            "alice",
 			readResponseBody: true,
 		},
+		{
+			name:             "Rotate the API key of a partially-qualified role and read the data stream",
+			roleId:           "user:alice",
+			login:            "alice",
+			readResponseBody: true,
+		},
 	}
 
 	for _, tc := range testCases {
@@ -109,17 +115,40 @@ func TestClient_RotateCurrentUserAPIKey(t *testing.T) {
 }
 
 type rotateHostAPIKeyTestCase struct {
-	name   string
-	hostID string
-	login  string
+	name       string
+	hostID     string
+	login      string
+	assertions func(t *testing.T, tc rotateHostAPIKeyTestCase, conjur *Client)
 }
 
 func TestClient_RotateHostAPIKey(t *testing.T) {
 	testCases := []rotateHostAPIKeyTestCase{
 		{
-			name:   "Rotate the API key of a foreign host",
-			hostID: "bob",
-			login:  "host/bob",
+			name:       "Rotate the API key of a foreign host: ID only",
+			hostID:     "bob",
+			login:      "host/bob",
+			assertions: runRotateHostAPIKeyAssertions,
+		},
+		{
+			name:       "Rotate the API key of a foreign host: partially qualified",
+			hostID:     "host:bob",
+			login:      "host/bob",
+			assertions: runRotateHostAPIKeyAssertions,
+		},
+		{
+			name:       "Rotate the API key of a foreign host: fully qualified",
+			hostID:     "cucumber:host:bob",
+			login:      "host/bob",
+			assertions: runRotateHostAPIKeyAssertions,
+		},
+		{
+			name:   "Rotate the API key of a foreign host: wrong role kind",
+			hostID: "user:alice",
+			assertions: func(t *testing.T, tc rotateHostAPIKeyTestCase, conjur *Client) {
+				_, err := conjur.RotateHostAPIKey(tc.hostID)
+				assert.Error(t, err)
+				assert.Contains(t, err.Error(), "must represent a host")
+			},
 		},
 	}
 
@@ -130,7 +159,7 @@ func TestClient_RotateHostAPIKey(t *testing.T) {
 			assert.NoError(t, err)
 
 			// EXERCISE
-			runRotateHostAPIKeyAssertions(t, tc, conjur)
+			tc.assertions(t, tc, conjur)
 		})
 	}
 }
@@ -150,17 +179,40 @@ func runRotateHostAPIKeyAssertions(t *testing.T, tc rotateHostAPIKeyTestCase, co
 // This is probably redundant with the above test case. Just going to keep them
 // separate for expediency for now.
 type rotateUserAPIKeyTestCase struct {
-	name   string
-	userID string
-	login  string
+	name       string
+	userID     string
+	login      string
+	assertions func(t *testing.T, tc rotateUserAPIKeyTestCase, conjur *Client)
 }
 
 func TestClient_RotateUserAPIKey(t *testing.T) {
 	testCases := []rotateUserAPIKeyTestCase{
 		{
-			name:   "Rotate the API key of a user",
-			userID: "alice",
-			login:  "alice",
+			name:       "Rotate the API key of a user: ID only",
+			userID:     "alice",
+			login:      "alice",
+			assertions: runRotateUserAPIKeyAssertions,
+		},
+		{
+			name:       "Rotate the API key of a user: partially qualified",
+			userID:     "user:alice",
+			login:      "alice",
+			assertions: runRotateUserAPIKeyAssertions,
+		},
+		{
+			name:       "Rotate the API key of a user: fully qualified",
+			userID:     "cucumber:user:alice",
+			login:      "alice",
+			assertions: runRotateUserAPIKeyAssertions,
+		},
+		{
+			name:   "Rotate the API key of a user: wrong role kind",
+			userID: "host:bob",
+			assertions: func(t *testing.T, tc rotateUserAPIKeyTestCase, conjur *Client) {
+				_, err := conjur.RotateUserAPIKey(tc.userID)
+				assert.Error(t, err)
+				assert.Contains(t, err.Error(), "must represent a user")
+			},
 		},
 	}
 
@@ -171,7 +223,7 @@ func TestClient_RotateUserAPIKey(t *testing.T) {
 			assert.NoError(t, err)
 
 			// EXERCISE
-			runRotateUserAPIKeyAssertions(t, tc, conjur)
+			tc.assertions(t, tc, conjur)
 		})
 	}
 }

conjurapi/client.go

@@ -308,11 +308,14 @@ func (c *Client) OidcAuthenticateRequest(code, nonce, code_verifier string) (*ht
 	return req, nil
 }
 
+// RotateAPIKeyRequest requires roleID argument to be at least partially-qualified
+// ID of from [<account>:]<kind>:<identifier>.
 func (c *Client) RotateAPIKeyRequest(roleID string) (*http.Request, error) {
-	_, _, _, err := c.parseID(roleID)
+	account, kind, identifier, err := c.parseID(roleID)
 	if err != nil {
 		return nil, err
 	}
+	roleID = fmt.Sprintf("%s:%s:%s", account, kind, identifier)
 
 	rotateURL := makeRouterURL(c.authnURL(), "api_key").withFormattedQuery("role=%s", roleID).String()
 
@@ -751,14 +754,51 @@ func makeFullId(account, kind, id string) string {
 // c.parseID("prod:user:alice") => "prod", "user", "alice", nil
 // c.parseID("malformed")       =>     "",     "",      "". error
 func (c *Client) parseID(id string) (account, kind, identifier string, err error) {
-	tokens := strings.SplitN(id, ":", 3)
-	if len(tokens) == 3 {
-		return tokens[0], tokens[1], tokens[2], nil
-	} else if len(tokens) == 2 {
-		return c.config.Account, tokens[0], tokens[1], nil
-	} else {
+	account, kind, identifier = c.unopinionatedParseID(id)
+	if identifier == "" || kind == "" {
 		return "", "", "", fmt.Errorf("Malformed ID '%s': must be fully- or partially-qualified, of form [<account>:]<kind>:<identifier>", id)
 	}
+	if account == "" {
+		account = c.config.Account
+	}
+	return account, kind, identifier, nil
+}
+
+// parseIDandEnforceKind accepts as argument a resource ID and a kind, and returns
+// the components - account, resource kind, and identifier - only if the provided
+// resource matches the expected kind. If the ID is only partially-qualified, the
+// configured account will be returned, and if the ID consists only of the
+// identifier, the expected kind will be returned.
+//
+// Examples:
+// c.parseID("dev:user:alice", "user")  =>  "dev", "user", "alice", nil
+// c.parseID("user:alice", "user")      =>  "dev", "user", "alice", nil
+// c.parseID("alice", "user")           =>  "dev", "user", "alice", nil
+// c.parseID("prod:user:alice", "user") => "prod", "user", "alice", nil
+// c.parseID("host:alice", "user")      =>     "",     "",      "", error
+func (c *Client) parseIDandEnforceKind(id, enforcedKind string) (account, kind, identifier string, err error) {
+	account, kind, identifier = c.unopinionatedParseID(id)
+	if (identifier == "") || (kind != "" && kind != enforcedKind) {
+		return "", "", "", fmt.Errorf("Malformed ID '%s', must represent a %s, of form [[<account>:]%s:]<identifier>", id, enforcedKind, enforcedKind)
+	}
+	if kind == "" {
+		kind = enforcedKind
+	}
+	if account == "" {
+		account = c.config.Account
+	}
+	return account, kind, identifier, nil
+}
+
+// unopinionatedParseID returns the components of the provided ID - account,
+// resource kind, and identifier - without expectation on resource kind or
+// account inclusion.
+func (c *Client) unopinionatedParseID(id string) (account, kind, identifier string) {
+	tokens := strings.SplitN(id, ":", 3)
+	for len(tokens) < 3 {
+		tokens = append([]string{""}, tokens...)
+	}
+	return tokens[0], tokens[1], tokens[2]
 }
 
 func NewClient(config Config) (*Client, error) {