Group-membership inconsistencies when using identity/group to remove member entities by group name

[indjb]

Describe the bug
Group-membership inconsistencies arise when using identity/group by group name only to remove member entities:

• The group reflects the new membership, with the entity missing.
• The entity shows that it is still a member of the group.

The result is that tokens for the entity are still granted the policies for the group.

To Reproduce

This script reproduces the behavior:

#!/bin/bash                                                                                                                                                                                                                                                                                                                  

readonly group_name=group-name
readonly token=devRootToken

function kill_vault {
    pkill -P $$
}
trap kill_vault EXIT

vault server -dev -dev-root-token-id="$token" >/dev/null 2>&1 &
sleep 1

function vault_api {
    local -r path="$1" ; shift
    curl -s -H "X-Vault-Token: $token" "$@" "http://localhost:8200/v1/$path"
}

function print_membership_info {
    echo -ne "\tgroup $group_id says members are:  "
    vault_api "identity/group/id/$group_id" | jq -c .data.member_entity_ids
    echo -ne "\tentity $entity_id says groups are:  "
    vault_api "identity/entity/id/$entity_id" | jq -c .data.group_ids
}

readonly entity_id="$(vault_api identity/entity -d'{}' | jq -r .data.id)"
readonly group_id="$(vault_api identity/group -d"{\"name\": \"$group_name\",\"member_entity_ids\":[\"$entity_id\"]}" | jq -r .data.id)"
echo "Initially:"
print_membership_info

vault_api identity/group -d"{\"name\": \"$group_name\", \"member_entity_ids\":[]}" >/dev/null
echo "After updating group $group_id to remove $entity_id using identity/group"
print_membership_info

The output will be something like:

Initially:
        group 3f174f6a-2f9f-933b-8352-a997ad1e01c7 says members are:  ["86a30c8d-54c6-f1aa-0e6d-c77e38379ab9"]
        entity 86a30c8d-54c6-f1aa-0e6d-c77e38379ab9 says groups are:  ["3f174f6a-2f9f-933b-8352-a997ad1e01c7"]
After updating group 3f174f6a-2f9f-933b-8352-a997ad1e01c7 to remove 86a30c8d-54c6-f1aa-0e6d-c77e38379ab9 using identity/group
        group 3f174f6a-2f9f-933b-8352-a997ad1e01c7 says members are:  []
        entity 86a30c8d-54c6-f1aa-0e6d-c77e38379ab9 says groups are:  ["3f174f6a-2f9f-933b-8352-a997ad1e01c7"]

Expected behavior

I expect the entity's list of groups to always be in sync with the group's list of members.

Environment:

• Vault Server Version (retrieve with vault status):

$ vault status
Key             Value
---             -----
Seal Type       shamir
Initialized     true
Sealed          false
Total Shares    1
Threshold       1
Version         1.5.4
Cluster Name    vault-cluster-cac3084b
Cluster ID      d0383e61-ddb4-65d7-96a0-6256b52decf9
HA Enabled      false

• Vault CLI Version (retrieve with vault version):

$ vault version
Vault v1.5.5 (f5d1ddb3750e7c28e25036e1ef26a4c02379fc01)

• Server Operating System/Architecture:

$ cat /etc/os-release | grep VERSION=
VERSION="16.04.4 LTS (Xenial Xerus)"

• Vault server configuration file(s):

Default configs only. Dev mode.

Additional context

This inconsistency does not arise when using identity/group/name/$group_name or identity/group/id/$group_id to remove the entity from the group, so the "fix" in our case is just to use a different endpoint, but I think it's important to clarify (and possibly fix) the behavior of the endpoint in question when used in this way. It's true that the documentation for identity/group only promises to update an existing group if the ID is given, but if the ID is not given -- and the name matches the name of an existing group -- the behavior is not specified in the documentation. The most obvious possibilities seem to be:

• Vault returns an error (4xx): Vault could complain that that endpoint can only update existing groups by ID and that updates by name for existing groups should be done by identity/group/name/$group_name. It's not clear that that's desirable behavior, and anyway it's not what Vault is actually doing.
• Vault updates the group by name: Vault could choose to update the group by name, which may be what the caller wants. Vault's current behavior seems to be to do so, but only partially, since it's updating the group itself but not cascading the change down to the removed entity. If Vault is changed to fully perform the update by name for existing groups via this endpoint, the documentation should be updated to clarify the behavior.

In any case, it seems that Vault should never behave in a way that results in the mismatch between the entity and the group. The result in our case was that the entity was still getting policies for the group, despite the group thinking the entity was no longer a member, leading to entities maintaining access to secrets for which we thought access had been revoked.

[indjb]

Seemingly related issues:

• Cannot remove a Member Group from an Identity Group #5464
  • Similar-sounding issue with removing a group from a group.
• Creating a group with the same name as an existing group is interpreted as a modification to the original group, even by a different user #9002
  • A question about how identity/group should act when referencing an existing group by name only.
• Identity groups not properly cleaned/merged with identity/entity/merge request #10084
  • Similar-sounding issue encountered when merging groups.

[raskchanky]

Thank you @indjb for an incredibly thorough bug report, complete with reproduction via shell script. 👏

[hdeberranger]

Looks like fix has not been merged. Do you have updates about this?

[mgritter]

This affects policies as well. The common code does check for an existing group of the same name, but after policies are assigned:

mgritter@ubuntu:~/vault-enterprise$ vault write identity/group name=test metadata="test=1" policies="policy-1"
Key     Value
---     -----
id      506afdcf-bd47-40f0-c83b-9be0c65b7929
name    test
mgritter@ubuntu:~/vault-enterprise$ vault write identity/group name=test metadata="test=2" policies="policy-2"
Key     Value
---     -----
id      506afdcf-bd47-40f0-c83b-9be0c65b7929
name    test
mgritter@ubuntu:~/vault-enterprise$ vault read identity/group/name/test
Key                  Value
---                  -----
alias                map[]
creation_time        2021-01-28T22:28:24.444948885Z
id                   506afdcf-bd47-40f0-c83b-9be0c65b7929
last_update_time     2021-01-28T22:28:30.006352725Z
member_entity_ids    []
member_group_ids     <nil>
metadata             map[test:2]       <======= metadata changed
modify_index         2
name                 test
namespace_id         root
parent_group_ids     <nil>
policies             [policy-1]           <======== policy not changes
type                 internal

[samuelmasuy]

Hi @mgritter,
The fix applied here "broke" the API, would you like me to open another issue?

vault write identity/group name="xxx" type="external" -format=json used to return the ID of the group as part of its output, even it the group existed. It does not anymore.

❯ vault version
Vault v1.6.1 (6d2db3f033e02e70202bef9ec896360062b88b03)
❯ vault server -dev
❯ vault write identity/group name="xxxx" type="external" metadata=team="xxxx" -format=json
{
  "request_id": "25660d8e-07f1-0bf2-e11f-1d67c0151fbe",
  "lease_id": "",
  "lease_duration": 0,
  "renewable": false,
  "data": {
    "id": "d37121cb-6238-206d-81ce-bd018e78b600",
    "name": "xxxx"
  },
  "warnings": null
}
❯ vault write identity/group name="xxxx" type="external" metadata=team="xxxx" -format=json
{
  "request_id": "2cd7f05e-f33d-09f9-1804-9332667e6d4f",
  "lease_id": "",
  "lease_duration": 0,
  "renewable": false,
  "data": {
    "id": "d37121cb-6238-206d-81ce-bd018e78b600",
    "name": "xxxx"
  },
  "warnings": null
}
❯ vault version
Vault v1.6.3 (b540be4b7ec48d0dd7512c8d8df9399d6bf84d76)
❯ vault server -dev
❯ vault write identity/group name="xxxx" type="external" metadata=team="xxxx" -format=json
{
  "request_id": "3006cf8b-e8b7-4d14-16b1-df9d7eaaa450",
  "lease_id": "",
  "lease_duration": 0,
  "renewable": false,
  "data": {
    "id": "91e90a2b-dda3-ebaa-2bc5-d6e7301b3aa2",
    "name": "xxxx"
  },
  "warnings": null
}
❯ vault write identity/group name="xxxx" type="external" metadata=team="xxxx" -format=json | wc
       0       0       0

[mgritter]

Hey @samuelmasuy that's an interesting side effect. I think probably the team will not change it (but feel free to open an issue.) Today's my last day at HashiCorp so I won't be fixing it, anyway.

The issue is that the second call goes to the "modify group" API endpoint, which doesn't return a response:

mgritter@ubuntu:~/vault$ vault write identity/group name="mygroup" metadata=team=mine
Key     Value
---     -----
id      d5da3e1f-0cef-020c-c797-c704c508f1d4
name    mygroup

mgritter@ubuntu:~/vault$ vault write identity/group/name/mygroup metadata=time=yours
Success! Data written to: identity/group/name/mygroup

That is, we only return the group on a create, not an update operation. So I think this is consistent that the second call to identity/group does not return a response, even if it differs from the previous behavior (which was a result of going through the "new group" code path each time, and half-heartedly swapping in the existing group too late in the process.)

[creslinux]

I have run into this also, my issue logged here: #11334

The tutorials for OIDC Auth, using groups fail as the GROUP_ID is not in the variable for canonical_id

ie, this fails:

GROUP_ID=$(vault write -field=id identity/group name="manager" type="external" \
         policies="manager" \
         metadata=responsibility="Manage K/V Secrets"
         
OIDC_AUTH_ACCESSOR=$(vault auth list -format=json  | jq -r '."oidc/".accessor')
      
vault write identity/group-alias name="kv-mgr" \
         mount_accessor="$OIDC_AUTH_ACCESSOR" \
         canonical_id="$GROUP_ID"

[creslinux]

Is there another way to discover the group_id after a group has been created, from the group name?
I would prefer not to have to persist this field, it rubs against my IDP holding the only fields I need to know about, the rest my flow done in logic.