Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Empty entity metadata for multiple auth methods #14494

Closed
MochaCaffe opened this issue Mar 15, 2022 · 10 comments
Closed

Empty entity metadata for multiple auth methods #14494

MochaCaffe opened this issue Mar 15, 2022 · 10 comments
Labels
auth/cert Authentication - certificates auth/ldap bug Used to indicate a potential bug core/auth

Comments

@MochaCaffe
Copy link

MochaCaffe commented Mar 15, 2022

Describe the bug
It looks like Vault doesn't register the identity.entity.metadata property when logging in. This was detected in my case when using LDAP or cert authentication. Yet to be confirmed on other auth methods

Update on 29/06

This appears to be by design because multiple auth methods can map to the same identity. To avoid reconciling conflicts as the aliases merge, the metadata is set to nil:

// Metadata represents the explicit metadata which is set by the
// clients. This is useful to tie any information pertaining to the
// aliases. This is a non-unique field of entity, meaning multiple
// entities can have the same metadata set. Entities will be indexed based
// on this explicit metadata. This enables virtual groupings of entities
// based on its metadata.
// @inject_tag: sentinel:"-"
Metadata map[string]string `protobuf:"bytes,4,rep,name=metadata,proto3" json:"metadata,omitempty" protobuf_key:"bytes,1,opt,name=key,proto3" protobuf_val:"bytes,2,opt,name=value,proto3" sentinel:"-"`

The metadata is available on the token in the meta field.

Expected behavior
For example, when using ldap authentication:

We expect to register the following entity, according to builtin/credential/ldap/path_login.go :

	auth := &logical.Auth{
		Metadata: map[string]string{
			"username": username,
		},
		InternalData: map[string]interface{}{
			"password": password,
		},
		DisplayName: username,
		Alias: &logical.Alias{
			Name: effectiveUsername,
			Metadata: map[string]string{
				"name": username,
			},
		},
	}

To Reproduce
Log in using LDAP auth and find the current identity id.
We should expect identity.entity.metadata.username to be set. However, what I find is an empty metadata:

vault read identity/entity/id/3ce5e46d-cadc-9aa4-99b2-1e1866f91900 
Key                    Value
---                    -----
metadata               <nil>

But, it's set properly in the alias (same value used):

vault read identity/entity-alias/id/16eec1e8-c975-ed5f-c9b0-b93a90b79846
Key                    Value
---                    -----
metadata              map[name:maxime.genet]

Environment:

  • Vault Server Version: v1.10.0-rc1
  • Vault CLI Version: v1.9.4
  • Server Operating System/Architecture: Kubernetes statefulset, based on Docker image vault:1.10.0-rc1

Vault server configuration file:

{
        "api_addr" = "http://vault:8200"
        "cluster_addr" = "http://$${.Env.POD_NAME}:8201"
        "listener" = [{
            "tcp" = {
            "address" = "0.0.0.0:8300", # Ingress endpoint
            "tls_cert_file" = "/tls/tls.crt",
            "tls_key_file" = "/tls/tls.key"
            }
        },{
          "tcp" = {
            "address" = "0.0.0.0:8200", # Local endpoint
            "tls_cert_file" = "/vault/tls/server.crt",
            "tls_key_file" = "/vault/tls/server.key"
          }

            }]
        "storage" = {
          "raft" = {
            "path" = "/vault/raft"
          }
        }
        "telemetry" = {
          "statsd_address" = "localhost:9125"
        }
        "ui" = true
      }

Additional context
For the TLS cert authentication method, I suggested to move metadata information into the entity.alias: #14418

@swayne275 swayne275 added bug Used to indicate a potential bug auth/cert Authentication - certificates auth/ldap core/auth labels Mar 16, 2022
@ryowright
Copy link
Contributor

I'd like to take a look at this.

@ncabatoff
Copy link
Collaborator

Hi @MochaCaffe,

Why do you think entity metadata should be automatically populated? Think about when there are multiple auth methods in use - should the entity metadata flip-flop back and forth each time one of them is used? To avoid that inconsistent behaviour, we only populate the alias metadata based on login. The entity metadata can be set explicitly if desired.

@MochaCaffe
Copy link
Author

Hi @ncabatoff, if so, I don't understand why the metadata property is specified in this case with LDAP auth. Is this property used somewhere else than the entity ?

@ncabatoff
Copy link
Collaborator

As far as I know logical.Auth.Metadata is only used in audit records, but I could well be missing something.

@ncabatoff
Copy link
Collaborator

Ah, found something I missed: it's used in renew requests, to know how to re-authenticate.

@calvn
Copy link
Member

calvn commented Mar 21, 2022

The additional metadata value identity.entity.aliases.<mount accessor>.metadata.name was a recent changed added in #13669.

Due to changes introduced int #11000, the value on identity.entity.aliases.<mount accessor>.name (note the absence of .metadata here) may not necessarily be the login username, so we added this in the alias metadata to allow policy templates to reference the username by value.

@calvn
Copy link
Member

calvn commented Mar 21, 2022

I see that we do set in in both metadata maps though, so it's surprising that it comes up empty on the entity lookup case.

auth := &logical.Auth{
Metadata: map[string]string{
"username": username,
},
InternalData: map[string]interface{}{
"password": password,
},
DisplayName: username,
Alias: &logical.Alias{
Name: effectiveUsername,
Metadata: map[string]string{
"name": username,
},
},
}

@MochaCaffe
Copy link
Author

MochaCaffe commented Mar 22, 2022

When creating an entity manually, metadata are properly registered though:

vault write -format=json identity/entity name="bob-smith" policies="base"      metadata=organization="ACME Inc."      metadata=team="QA" 
{
  "request_id": "aa5efc37-80de-9ae7-3712-4baf24b0f183",
  "lease_id": "",
  "lease_duration": 0,
  "renewable": false,
  "data": {
    "aliases": null,
    "id": "f75e4f88-ea01-b9ad-1ba0-96f21f965352",
    "name": "bob-smith"
  },
  "warnings": null
}
vault read identity/entity/id/f75e4f88-ea01-b9ad-1ba0-96f21f965352
Key                    Value
---                    -----
aliases                []
creation_time          2022-03-22T07:55:05.317329804Z
direct_group_ids       []
disabled               false
group_ids              []
id                     f75e4f88-ea01-b9ad-1ba0-96f21f965352
inherited_group_ids    []
last_update_time       2022-03-22T07:55:05.317329804Z
merged_entity_ids      <nil>
metadata               map[organization:ACME Inc. team:QA]
name                   bob-smith
namespace_id           root
policies               [base]

@jasonodonnell
Copy link
Contributor

jasonodonnell commented Apr 5, 2022

This appears to be by design because multiple auth methods can map to the same identity. To avoid reconciling conflicts as the aliases merge, the metadata is set to nil:

// Metadata represents the explicit metadata which is set by the
// clients. This is useful to tie any information pertaining to the
// aliases. This is a non-unique field of entity, meaning multiple
// entities can have the same metadata set. Entities will be indexed based
// on this explicit metadata. This enables virtual groupings of entities
// based on its metadata.
// @inject_tag: sentinel:"-"
Metadata map[string]string `protobuf:"bytes,4,rep,name=metadata,proto3" json:"metadata,omitempty" protobuf_key:"bytes,1,opt,name=key,proto3" protobuf_val:"bytes,2,opt,name=value,proto3" sentinel:"-"`

The metadata is available on the token in the meta field.

@ncabatoff
Copy link
Collaborator

Can we close this bug? It was premised on an incorrect understanding of auth metadata and how that relates to identity. Alternatively, can the summary/description be updated to reflect the actual issue? I'm not sure what this is now or I'd do so myself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auth/cert Authentication - certificates auth/ldap bug Used to indicate a potential bug core/auth
Projects
None yet
Development

No branches or pull requests

7 participants