delete assigned identities when deleting policy

This commit changes the implementation of the
`/v1/policy/delete` API.

Deleting a policy used to just remove the policy with the
specified name from the policy set. When there were identities
assigned to this particular policy then identities would become
"stale"/"dangling".

If the KES server would encounter a request from an identity that
would be "assigned" to a deleted policy it would deny any access,
as it would if the identity would not be assigned to a policy.

This commit changes the behavior of `/v1/policy/delete` such that
deleting a policy also removes all assigned identities from the
KES server. This behavior is save because:
 - An identity can be assigned to (at most) one policy at one point
   in time.
 - An assigned identity is just a mapping from (external) identity
   to a policy. It does not have any state whatsoever.

It's worth noting that assigning an identity to a policy and
re-assigning an identity from one (potentially deleted) policy
to another policy are the exact same operations.

Therefore, no state gets lost when removing the assigned identities
once a policy gets deleted - except for which identities "belong"
together based on being assigned to the same policy. However, this
information would be lost anyway on e.g. KES server restart and cannot
be assumed to be persistent.

Also, it is anyway not possible to assign an identity to a non-existing
policy via the `/v1/identity/assign` API.

***

From a user perspective this change has the following implications:
 1. Now, an identity is always assigned to exactly one (existing) policy.
 2. Setting a policy and deleting a policy are not inverse operations
    anymore.

client.go

@@ -23,10 +23,10 @@ import (
 // functions.
 //
 // In general, a client just requires:
-// •  a KES server endpoint
-// •  a X.509 certificate for authentication.
+//   • a KES server endpoint
+//   • a X.509 certificate for authentication
 //
-// However, custom transport protcols, timeouts,
+// However, custom transport protocols, timeouts,
 // connection pooling, etc. can be specified via
 // a custom http.RoundTripper. For example:
 //   client := &Client{
@@ -416,7 +416,18 @@ func (c *Client) Decrypt(key string, ciphertext, context []byte) ([]byte, error)
 	return response.Plaintext, nil
 }
 
-func (c *Client) WritePolicy(name string, policy *Policy) error {
+// SetPolicy adds the given policy to the set of policies.
+// There can be just one policy with one particular name at
+// one point in time.
+//
+// If there is already a policy with the given name then SetPolicy
+// overwrites the existing policy with the given one.
+//
+// If there are identities assigned to an existing policy then
+// SetPolicy will not remove those identities before overwriting
+// the policy. Instead, it will just updated the policy entry such
+// that the given policy automatically applies to those identities.
+func (c *Client) SetPolicy(name string, policy *Policy) error {
 	content, err := json.Marshal(policy)
 	if err != nil {
 		return err
@@ -433,7 +444,9 @@ func (c *Client) WritePolicy(name string, policy *Policy) error {
 	return nil
 }
 
-func (c *Client) ReadPolicy(name string) (*Policy, error) {
+// GetPolicy returns the policy with the given name. If no such
+// policy exists then GetPolicy returns ErrPolicyNotFound.
+func (c *Client) GetPolicy(name string) (*Policy, error) {
 	client := retry(c.HTTPClient)
 	resp, err := client.Get(fmt.Sprintf("%s/v1/policy/read/%s", c.Endpoint, name))
 	if err != nil {
@@ -454,7 +467,17 @@ func (c *Client) ReadPolicy(name string) (*Policy, error) {
 	return &policy, nil
 }
 
+// ListPolicies returns a list of policies with names that
+// match the given glob pattern. For example
+//   policies, err := client.ListPolicies("*") // '*' matches any
+// returns the names of all existing policies.
+//
+// If no / an empty pattern is provided then ListPolicies uses
+// the pattern '*' as default.
 func (c *Client) ListPolicies(pattern string) ([]string, error) {
+	if pattern == "" { // The empty pattern never matches anything
+		pattern = "*" // => default to: list "all" policies
+	}
 	client := retry(c.HTTPClient)
 	resp, err := client.Get(fmt.Sprintf("%s/v1/policy/list/%s", c.Endpoint, url.PathEscape(pattern)))
 	if err != nil {
@@ -473,6 +496,17 @@ func (c *Client) ListPolicies(pattern string) ([]string, error) {
 	return policies, nil
 }
 
+// DeletePolicy removes the policy with the given name. It will not
+// return an error if no policy exists.
+//
+// If there are identities assigned to the deleted policies then these
+// identities will be removed as well.
+//
+// Therefore, setting an empty policy and deleting a policy have
+// slightly different implications. The former will revoke any
+// access permission for all identities assigned to the policy.
+// The later will remove the policy as well as all identities
+// assigned to it.
 func (c *Client) DeletePolicy(name string) error {
 	url := fmt.Sprintf("%s/v1/policy/delete/%s", c.Endpoint, name)
 	req, err := http.NewRequest(http.MethodDelete, url, retryBody(nil))

cmd/kes/policy.go

@@ -93,7 +93,7 @@ func addPolicy(args []string) error {
 	if err = policy.UnmarshalJSON(data); err != nil {
 		return fmt.Errorf("Policy file is invalid JSON: %v", err)
 	}
-	if err = client.WritePolicy(args[0], &policy); err != nil {
+	if err = client.SetPolicy(args[0], &policy); err != nil {
 		return fmt.Errorf("Failed to add policy '%s': %v", args[0], err)
 	}
 	return nil
@@ -140,7 +140,7 @@ func showPolicy(args []string) error {
 	if err != nil {
 		return err
 	}
-	policy, err := client.ReadPolicy(name)
+	policy, err := client.GetPolicy(name)
 	if err != nil {
 		return fmt.Errorf("Failed to fetch policy '%s': %v", args[0], err)
 	}
@@ -181,7 +181,7 @@ func listPolicies(args []string) error {
 		cli.Usage()
 		os.Exit(2)
 	}
-	policy := "*"
+	var policy string
 	if len(args) == 1 {
 		policy = args[0]
 	}

error.go

@@ -12,10 +12,22 @@ import (
 )
 
 var (
+	// ErrNotAllowed represents a KES server response returned when the
+	// client has not sufficient policy permissions to perform a particular
+	// operation.
+	ErrNotAllowed Error = NewError(http.StatusForbidden, "prohibited by policy")
+
+	// ErrKeyNotFound represents a KES server response returned when a client
+	// tries to access or use a cryptographic key which does not exist.
 	ErrKeyNotFound Error = NewError(http.StatusNotFound, "key does not exist")
-	ErrKeyExists   Error = NewError(http.StatusBadRequest, "key does already exist")
-	ErrKeySealed   Error = NewError(http.StatusForbidden, "key is sealed")
-	ErrNotAllowed  Error = NewError(http.StatusForbidden, "prohibited by policy")
+
+	// ErrKeyExists represents a KES server response returned when a client tries
+	// to create a cryptographic key which already exists.
+	ErrKeyExists Error = NewError(http.StatusBadRequest, "key does already exist")
+
+	// ErrPolicyNotFound represents a KES server response returned when a client
+	// tries to access a policy which does not exist.
+	ErrPolicyNotFound Error = NewError(http.StatusNotFound, "policy does not exist")
 )
 
 // Error is the type of client-server API errors.

internal/auth/policy.go

@@ -81,8 +81,16 @@ func (r *Roles) Get(name string) (*kes.Policy, bool) {
 
 func (r *Roles) Delete(name string) {
 	r.lock.Lock()
+	defer r.lock.Unlock()
+
 	delete(r.roles, name)
-	r.lock.Unlock()
+	if r.effectiveRoles != nil { // Remove all assigned identities
+		for id, policy := range r.effectiveRoles {
+			if name == policy {
+				delete(r.effectiveRoles, id)
+			}
+		}
+	}
 }
 
 func (r *Roles) Policies() (names []string) {
@@ -109,7 +117,7 @@ func (r *Roles) Assign(name string, id kes.Identity) error {
 	}
 	_, ok := r.roles[name]
 	if !ok {
-		return errors.New("key: policy does not exists")
+		return kes.ErrPolicyNotFound
 	}
 	if r.effectiveRoles == nil {
 		r.effectiveRoles = map[kes.Identity]string{}

internal/http/handler.go

@@ -415,7 +415,6 @@ func HandleWritePolicy(roles *auth.Roles) http.HandlerFunc {
 func HandleReadPolicy(roles *auth.Roles) http.HandlerFunc {
 	var (
 		ErrInvalidPolicyName = kes.NewError(http.StatusBadRequest, "invalid policy name")
-		ErrPolicyNotFound    = kes.NewError(http.StatusBadRequest, "policy does not exist")
 	)
 	return func(w http.ResponseWriter, r *http.Request) {
 		name := pathBase(r.URL.Path)
@@ -426,7 +425,7 @@ func HandleReadPolicy(roles *auth.Roles) http.HandlerFunc {
 
 		policy, ok := roles.Get(name)
 		if !ok {
-			Error(w, ErrPolicyNotFound)
+			Error(w, kes.ErrPolicyNotFound)
 			return
 		}
 		json.NewEncoder(w).Encode(policy)
@@ -465,7 +464,6 @@ func HandleAssignIdentity(roles *auth.Roles) http.HandlerFunc {
 		ErrIdentityUnknown = kes.NewError(http.StatusBadRequest, "identity is unknown")
 		ErrIdentityRoot    = kes.NewError(http.StatusBadRequest, "identity is root")
 		ErrSelfAssign      = kes.NewError(http.StatusForbidden, "identity cannot assign policy to itself")
-		ErrPolicyNotFound  = kes.NewError(http.StatusBadRequest, "policy does not exist")
 	)
 	return func(w http.ResponseWriter, r *http.Request) {
 		identity := kes.Identity(pathBase(r.URL.Path))
@@ -484,7 +482,7 @@ func HandleAssignIdentity(roles *auth.Roles) http.HandlerFunc {
 
 		policy := pathBase(strings.TrimSuffix(r.URL.Path, identity.String()))
 		if err := roles.Assign(policy, identity); err != nil {
-			Error(w, ErrPolicyNotFound)
+			Error(w, kes.ErrPolicyNotFound)
 			return
 		}
 		w.WriteHeader(http.StatusOK)