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

[Fleet] Replace usage of username by user profile uid #138703

Open
juliaElastic opened this issue Aug 12, 2022 · 11 comments
Open

[Fleet] Replace usage of username by user profile uid #138703

juliaElastic opened this issue Aug 12, 2022 · 11 comments
Assignees
Labels
discuss Team:Fleet Team label for Observability Data Collection Fleet team

Comments

@juliaElastic
Copy link
Contributor

Fleet uses username from security plugin to set updatedBy field in agent policy and package policy.
security.authc.getCurrentUser(req).username
https://github.com/elastic/kibana/blob/main/x-pack/plugins/fleet/server/services/agent_policy.ts#L206

created_by: options?.user?.username ?? 'system',
updated_at: isoDate,
updated_by: options?.user?.username ?? 'system',

It was raised that users of this API should migrate to use profile ID instead as username is not guaranteed to be unique.
((await security.userProfiles.getCurrent(req)).uid)
https://www.elastic.co/guide/en/elasticsearch/reference/master/user-profile.html

Can we get some guidance whether user profile ID should be used in our use case?

Originally discussed here: https://elastic.slack.com/archives/C9097ABGC/p1660232160243589?thread_ts=1660148287.877479&cid=C9097ABGC

cc @joshdover

@juliaElastic juliaElastic added discuss Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! Team:Fleet Team label for Observability Data Collection Fleet team labels Aug 12, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-security (Team:Security)

@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@azasypkin
Copy link
Member

Can we get some guidance whether user profile ID should be used in our use case?

What this updatedBy field is used for exactly? As a general rule, starting from 8.4, we're supposed to always use profile ID whenever we need to refer to the stack user. But unlike username profile ID might not be available in certain scenarios, and we need to assess if it's acceptable or not in every specific case:

  • Profile ID isn't available for the anonymous users that use a single anonymous "service account" (in quotes because it's just any native Elasticsearch user, not a real Elasticsearch service account) under the hood.
  • Profile ID isn't available for users authentication via HTTP authentication (e.g. when someone queries Kibana via cURL with Authorization: Basic username:password or Authorization: ApiKey xxxxx HTTP headers).

@legrego
Copy link
Member

legrego commented Aug 17, 2022

But unlike username profile ID might not be available in certain scenarios, and we need to assess if it's acceptable or not

I wonder if we (as a platform) should offer an opinionated, graceful fallback. If the consumer can do without a true identity for their use case, then perhaps we could offer to store a display name (and/or other fields) in lieu of a profile id. This could allow for a ux that more gracefully degrades.

For example (hypothetical!): If updatedBy is only used to populate a column within a table, then storing the profile id would allow us to offer the "full" experience (correct avatar, link to public profile, etc.), but falling back to a simple display name may offer a better experience than showing nothing at all.

@azasypkin
Copy link
Member

I wonder if we (as a platform) should offer an opinionated, graceful fallback.

++, we'll definitely provide/document guidance on this one that might eventually result into one-stop set of programmatic APIs (e.g. "get me the the most unique identifier for the current user you can get" and "resolve this user identifier to the most human readable label you can") for all types of users. I'd like to learn more about the existing uses cases before we offer anything though.

@juliaElastic
Copy link
Contributor Author

What this updatedBy field is used for exactly?

It is stored as a field in agent and integration policies, I found one place where the Updated by field is displayed on Integrations UI:

image

@paul-tavares Can you confirm what is security plugin using this field for?

@paul-tavares
Copy link
Contributor

Hi @juliaElastic ,
Yes, we need to have created/updated by field from a security standpoint in order to ensure we know who changed policies. I think that is a critical item to have on all documents so that individuals can be identified if necessary.

We have not yet opened an issue on our side to follow up, but note that I think many more discussions will be needed on this due to the fact that the Security team is suggesting we store a User Profile UUID. That value is not useful from a UX standpoint and thus we will need to discuss the implication of making such change and how we can continue to support showing a username on the UI.

cc/ @kevinlog , @ashokaditya

@azasypkin
Copy link
Member

azasypkin commented Aug 22, 2022

in order to ensure we know who changed policies.

How can these policies be updated? Are they always updated interactively by the logged in user or you also provide HTTP APIs to update these policies with something like curl -H "Authorization: ApiKey/Basic/Bearer xxx"?

That value is not useful from a UX standpoint

Kibana Security plugin exposes APIs to resolve profile UID to the user profile that always includes username and might include full_name and email if specified by the user. The code might become more complex, but there should be everything to maintain the same or better level of UX.

@legrego
Copy link
Member

legrego commented Aug 22, 2022

Yes, we need to have created/updated by field from a security standpoint in order to ensure we know who changed policies. I think that is a critical item to have on all documents so that individuals can be identified if necessary.

It's worth noting that storing/displaying a username is not always sufficient for determining identity. It's possible that two different people can share a username, by authenticating via different realms. For example, you can define an elastic user within a SAML realm, and that would not necessarily be the same individual as the person using the built-in elastic user.

User Profiles are a much stronger form of identity within the platform, and were intentionally designed to support these nuanced scenarios.

@juliaElastic
Copy link
Contributor Author

juliaElastic commented Aug 22, 2022

in order to ensure we know who changed policies.

How can these policies be updated? Are they always updated interactively by the logged in user or you also provide HTTP APIs to update these policies with something like curl -H "Authorization: ApiKey/Basic/Bearer xxx"?

The policies can be updated through UI and API (the UI uses the same API that we expose to users). The API supports ApiKey, Bearer auth, with those we just set "updatedBy: system".

How urgent is this work to replace username?

@azasypkin
Copy link
Member

The policies can be updated through UI and API (the UI uses the same API that we expose to users). The API supports ApiKey, Bearer auth, with those we just set "updatedBy: system".

Thanks for confirming, let me discuss this case with the team.

How urgent is this work to replace username?

There is no urgency from our end, but the sooner you migrate the sooner you might leverage all user profile related functionality and potentially less code paths to migrate later.

@legrego legrego removed the Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! label Jan 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Team:Fleet Team label for Observability Data Collection Fleet team
Projects
None yet
Development

No branches or pull requests

5 participants