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

User profile management APIs and UI #141258

Open
azasypkin opened this issue Sep 21, 2022 · 14 comments
Open

User profile management APIs and UI #141258

azasypkin opened this issue Sep 21, 2022 · 14 comments
Labels
Feature:Security/User Profile Meta Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! triage_needed

Comments

@azasypkin
Copy link
Member

azasypkin commented Sep 21, 2022

h2. Problem statement: 

Removing users does not currently deactivate their profile. As a result, an admin may remove an internal or external user but their profile will continue to be surfaced in search results. The profile will be assignable and the corresponding email will continue receiving email notifications but the (removed) user behind it will not have access to Kibana, nor they are any longer expected to act on assignments and notifications.

h2. Aim:

  • We want to create a UX that allows admins to easily deactivate profiles, in order to mitigate the problem described previously.
  • This UX should aim to iron out, aka make as transparent as possible, any friction caused by the profile/user duality. In the future (and in serverless) this duality should be eliminated.

h2. Design:

  • +UX+ [+Overview video+|https://drive.google.com/file/d/139uVxWLybH1VvTnf_WIcDY1gD2NDaT-2/view?usp=sharing]
  • [Whimsical wireframes|https://whimsical.com/user-profile-management-enhancements-6f2NYCEbAtf8QmVnmLaPjJ]
  • ([Discussion that led to the design|https://docs.google.com/document/d/1_1IFZ2fYreQoEVysST-hbEqp29wCsdqSR-Oe24Ya-Kw/edit?usp=sharing])
@azasypkin azasypkin added Meta Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! Feature:Security/User Profile labels Sep 21, 2022
@elasticmachine
Copy link
Contributor

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

@azasypkin
Copy link
Member Author

We should also check if it'd be feasible to automatically disable profiles for the Elasticsearch native users when they are deleted (either via Kibana UI, Kibana API or Elasticsearch API).

@ywangd do you think it'd be possible to implement something like this on the Elasticsearch side or there are reasons why we shouldn't do it?

@EricDavisX
Copy link
Contributor

This seems particularly impactful to usage where a user is 'assigned' to something in Kibana, as in the new 'assign users to a case' feature. It is natural for people to come and go from an organization, and Kibana users may not have great external communication to know not to select someone for assignment to a case if the expected user is the system and it was formerly their role to be assigned. Just providing context / reasoning to help when prioritizing.

@ywangd
Copy link
Member

ywangd commented Sep 26, 2022

When planning ES side work for user profiles, we talked about the possibility of updating profiles automatically for native users and decided to leave it out for the initial scope. I think the ask here, "disabling profile on native user deletion", is an indication that we should bring the discussion back. It is better if we could approach it more holistically (update, disable, enable, deletion) instead of just one API.

A few issues popped up in my mind are:

  • It makes native user APIs depend on profiles. If there is something wrong with profiles, the user APIs can fail as well. It is likely that the failure will be partially, e.g. the user is updated but failed at profile update, the entire process is reported as failure but it actually partially succeeded. It is not new though since we already have the same issue when updating users and clearing cache. But more separate steps tend to add more edge cases and complexity. It also means the user APIs will keep erroring until profile issues get fixed. Alternatively, we can have profile update to be optional for user APIs similar to how returning profile uid is optional for GetUser API. But it feels weird to me that update (mutation) can be optional (while optional "get" is much more normal).
  • If native realm is under the same security domain as an external realm (say SAML), whether to update the profile on user changes has more nuances. For example, I don't think it's safe to disable the profile on native user deletion since we do not know whether the same user is removed from the external realm. But should we update profile roles if roles of the native user change? If we update the roles, should the update follow the logic of activateProfile and update the recorded realm info as well? It also depends on security domain being configured consistently across the nodes. This is our recommendation so it might be OK to assume consistency (or at least no domain splitting).
  • Native users can be updated even when the native realm is disabled. Should profiles be updated in this case when there are user changes?

In summary, I think we need to answer two questions:

  1. Do we want to support automatically update profiles on native user changes?
  2. If so, where should we build the support, i.e. ES or Kibana. There are some tricky cases to handle as described above. ES has more knowledge about configurations to make decisions. But handling at Kibana level can have more flexibility (assuming ES exposing necessary information through APIs).

If the answers are "Yes" (1) and "ES" (2), I'll work on a document to lay out implementation options.

@ywangd
Copy link
Member

ywangd commented Sep 26, 2022

@bytebilly
Copy link
Contributor

Do we want to support automatically update profiles on native user changes?

I don't think that this automation should be tightly decoupled to the Elasticsearch Users API, since profiles are connected to users but they have been introduced to decouple the two concepts.

Probably the right solution is to work on an "offboarding" flow that allows admins to remove users from their system. This is more likely to be a nice UI rather than an automation of a single Elasticsearch API. This UI could ask questions and make decisions that could potentially disable the profile, but not necessarily.

@albertzaharovits
Copy link
Contributor

As Yang details above, I also think it would be onerous to keep user profiles and native users in sync at the ES level.
The delete/disable/update actions on the user can succeed but the related profile changes might independently fail.

They are also fundamentally different entities. Profiles, by design, encompass multiple users and are a "trace" of the last user that logged-in.

But I'm also sympathetic to the woes of admins that modify native users and then not being able to see that modification reflected when users are being referred to someplace else in the system.

Bottom line is that I think technically we could link native users and the profiles at the ES level, if the user experience requires it. But we might create the wrong expectation that changes to external users are similarly reflected to profiles.

@azasypkin
Copy link
Member Author

azasypkin commented Sep 30, 2022

Thanks for detailed context @ywangd and for the feedback @bytebilly and @albertzaharovits!

I agree, these are all valid concerns and there is a high chance to introduce a non-trivial amount of the complexity for the sake of limited value. With that in mind and if product agrees (ping @arisonl), it'd make sense to refrain from introducing this complexity until we get feedback that this functionality would be useful for our users. This functionality is also not blocking Kibana User Profile Management UI in any way.

Speaking of Kibana User Profile Management UI, it'd need some Elasticsearch API to retrieve all known profiles from Elasticsearch. I can think of the following requirements for such an API:

  • Unlike suggest API, it should be able to return both enabled and disabled user profiles (so that admins can perform both operations)
  • I guess it shouldn't require manage_security, and read_security should be enough?
  • It might need some sort of paging support. On the other hand, we don't have paging support for the users API and it feels like it never was a big problem, right? The slight difference here is that users, unlike user profiles, can be deleted.

Also if we assume that the user profile deactivation will be used for off-boarding and we don't plan to add Delete User Profile API, then we might need some way to either sort profiles by deactivation status or have a disabled/enabled filter to not clutter UI with the user profiles that were deactivated long time ago. If the number of the user profiles isn't huge, we can do this sorting\filtering on the Kibana side though.

What do you all think?

@ywangd
Copy link
Member

ywangd commented Oct 3, 2022

The issue with pagination, filtering and sorting is less about whether we can provide these functionalities, but more about what the API should look like. Previously, different ES teams have provided these features in different forms for different APIs. There is an ongoing proposal on the ES side to consolidate the approach. But I am not sure about it current status. IIUC, the proposal does not cater for more complex use cases, e.g. sophiscated filtering logic (it is mostly a flat list of AND), or page to page consistency (no duplicates or misses). Do you foresee any complex query logic that Kibana may want from this API? Or is it really just filtering based on "enabled"?

Depend on how urgent this requirement is for Kibana, we may be able to start with just "returning all profiles" (enabled or not) by augmenting the existing GetUserProfile API. It already supports returnning multiple profiles but requries profile uids to be explicitly specified. We can update it to return all profiles if no ID is specified. We can then iteratively add sorting, filtering and pagination (likely in this order) to it once we figure out how these parameters should look it.

@exalate-issue-sync exalate-issue-sync bot changed the title [Meta] User profile management APIs and UI User profile management APIs and UI Oct 14, 2022
@bytebilly
Copy link
Contributor

We can update it to return all profiles if no ID is specified.

My only concern in this case would be blocking the system (or at least the flow) when the number of user profiles is high. Do we feel this is a real risk for our scenario?

@ywangd
Copy link
Member

ywangd commented Nov 8, 2022

It's not ideal and can be an issue if the number are really high, e.g. over 10k. It also depends on how often the API is called. The issue is not really about the size because ES can handle responses of a couple dozen MBs. But creating and destroying 10K+ objects at a high rate can cause JVM GC threshing.

Ideally we should have pagination support (10K and beyond). But how to achieve this is still an open discussion on the ES side. In the meantime, depending on how urgent the need is here, we may just have to go with returning all profiles. It can optionally take size parameter to control the response size. But this size parameter also defeats the purpose of API if no pagination is possible.

@bytebilly
Copy link
Contributor

bytebilly commented Nov 8, 2022

@azasypkin could you please explain more in details how the "management" functionality is envisioned, and which is its priority/feasibility status?

If it's not a top priority, I'd consider implementing the full solution (with pagination) at the proper time rather than having a temporary approach that is potentially subject to problems.

@azasypkin
Copy link
Member Author

@azasypkin could you please explain more in details how the "management" functionality is envisioned, and which is its priority/feasibility status?

Sorry for the radio silence here @ywangd and @bytebilly, still catching up on the emails after PTO. Let me figure out the current state of the things on this one and get back to you. I see we've already initiated discussion with the UX team, but we don't have designs yet to start working on this.

If it's not a top priority, I'd consider implementing the full solution (with pagination) at the proper time rather than having a temporary approach that is potentially subject to problems.

That'd be my preference too, but let me first double check if it's not a top priority ask for the solutions.

@azasypkin
Copy link
Member Author

UPDATE: we all met a few days ago to discuss our next steps, here is the summary:

  • We don't have defined timelines and UX/designs for this functionality yet, and it's not actionable on the engineering side. Once we have more clarity, we'll reach out to the Elasticsearch team.
  • We agreed that since there is no urgency here, we'll try to work together on a more future proof implementation that assumes pagination, filtering, and sorting on the Elasticsearch API side. Unless the priorities change, we're not planning to have any temporary workarounds for this functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Security/User Profile Meta Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! triage_needed
Projects
None yet
Development

No branches or pull requests

9 participants