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

Question: Should "internal users" be renamed to "internal accounts" #2842

Closed
stephen-crawford opened this issue Jun 8, 2023 · 10 comments
Closed
Labels
breaking This issue is or proposes a breaking change enhancement New feature or request

Comments

@stephen-crawford
Copy link
Contributor

stephen-crawford commented Jun 8, 2023

As part of the extension support work, service accounts were added to the Security Plugin. These accounts live inside the internal users configuration and make use of the same APIs.

However, when speaking with Ka Ming who has helped designed many of the UX/UI elements for the Security Plugin, they brought up the point that perhaps it would be more appropriate to rename these objects to "internal accounts."

If this seems like a good idea, we will need to look at the usage of the internal users throughout the plugin and determine the best place to swap things over. It may be most appropriate to simply swap the dashboards text and introduce a secondary API for the new naming. We won't be able to swap to a new naming scheme entirely until 3.0.0 because this would cause breaks in existing automatic calls systems.

@stephen-crawford stephen-crawford added enhancement New feature or request untriaged Require the attention of the repository maintainers and may need to be prioritized labels Jun 8, 2023
@stephen-crawford stephen-crawford changed the title Question: Should "internal users Question: Should "internal users" be renamed to "internal accounts" Jun 8, 2023
@peternied
Copy link
Member

Good idea to get other perspectives on the naming of these features.

While we are looking at a shift in the name, is 'internal' meaningful to consumers of OpenSearch? What about System Accounts, or Service Accounts?

We won't be able to swap to a new naming scheme entirely until 3.0.0 because this would cause breaks in existing automatic calls systems.

Could you provide a github query that would show how this would be a breaking change?

@peternied peternied added breaking This issue is or proposes a breaking change and removed untriaged Require the attention of the repository maintainers and may need to be prioritized labels Jun 8, 2023
@stephen-crawford
Copy link
Contributor Author

Hi @peternied,

Taking a broad look, I see we have 70+ usages in the plugin.

Narrowing this down to changes that would be breaking, the the only exposed file is the InternalUsersApiAction. The current API system expects internalusers to be in the route for the API calls.

If we swap to something like internalaccounts we would potentially break automated workflows that relied on these calls. A straightforward workaround for this would be to keep the internalusers routes and simply mark is as on a deprecation path for 3.0.0. We could then introduce a new API path with internalaccounts and simply resolve both routes with the same handler.

The caveat with this approach would be that we would be introducing additional deprecation notices etc. which may not be something we want to do. I recall there being confusion around the deprecation of the securityadmin tool etc. So if we do mark things as deprecation path we should be mindful in it.

For other changes that could be breaking, we can see that the ConfigurationRepository, Security Admin, and surrounding logic expect an internal_users.yml file. In total, there are 9 Java files which reference internal_users.yml. In this group, the only break I could see was if someone had an automated configuration process for their cluster where they had scripted restarts to their cluster in order to add new internal users manually. This would be quite inefficient, so does not seem likely, but in this case, you could encounter a problem if a surrounding script continued to create internal_users.yml and we swapped to expected internal_accounts.yml.

@peternied
Copy link
Member

Oh! I misread the issue, I was thinking this only applied to the new service accounts features we were building.

At an API level, no we should not make any changes, this is a breaking change and will impact the requests and schema of standard conventions of OpenSearch. I would also argue that the 'smallness' in difference between these two names would be eye rolling for plugins that need to migration from 2.X -> 3.X release.

I think we can look at changing how the UX in dashboards treats this concept, where it won't create confusion. Maybe we could look at those on a case-by-case basis.

@stephen-crawford
Copy link
Contributor Author

No worries Peter!

Yeah, I agree it is a relatively small change that is probably best handled on the frontend side only. I wanted to make sure I at least surfaced the question however to give Ka Ming as much info as possible when looking at how we want to present interaction with service accounts.

I still am waiting for follow up on his GitHub username but he will be able to provide more information about what he is thinking shortly.

Originally he asked:
"""
You may consider thinking through what an end to end journey of how users interact with service accounts. This will help determine the frontend scope. Seems like there are areas we may need to address in addition to filtering the list of users.

Have we considered some of these flows?

  1. Edit flow: Can users edit service accounts like editing an internal user account on the UI? What metadata information can we display so users can view useful information about the service account?
  2. Delete and duplicate flows: Can users delete and duplicate service accounts?
  3. What actions do we expect users to do after the service account is created? Does users go to Roles and map the service account into a role?
  4. Enable/disable: The UI doesn’t provide this functionary for Internal users. Are we thinking of adding for both Internal users and service accounts?
  5. Turn internal user <> service accounts: Is this a common action users would do? Would it be important to surface in the UI or perhaps only in the API?
  6. Terminology: Would it make sense if we rename “internal users” to “Internal accounts”?
    """

To which I replied:

"""
1.An admin will be able to modify service accounts in the same way they can other internal users as well as configure whether the accounts are enabled or not.
2. Right now there is nothing stopping deletion or duplication of service accounts. That being said, my understanding is that duplication does not copy directly the account name. If it were to copy the name this would cause issues. Deleting a service account should be fine though unadvised.
3. The service accounts should generally not be modified by admins. The only interaction expected with service accounts would be enabling or disabling them and modifying permissions.
4. We will want to add enable/disable for service accounts at the very least. Right now this is done through the backend only.
5. Sorry I am not sure I understand this question... Could you rephrase?
6. This is a good suggestion. I would need to look at the impacts that this change would have on the backend but I think that it may be appropriate.

"""

For reference this all stems from the work that Sam is doing over on: #2704

@stephen-crawford
Copy link
Contributor Author

Tagging @kamingleung for his thoughts!

@kamingleung
Copy link

@peternied @scrawfor99 Thanks for opening up this issue. My thought behind this is:

  1. Is there an opportunity to generalize the term Internal users when we are expanding its functionality – in this case we are adding a new type Service Account. I also wonder if "Internal" is the fitting term for our users, it feels very technical. Since there are API contracts with this, this may be something we may think about in the future on improving the terminology.
  2. However, for the scope of the extensions security work, we should help admins to differentiate between a service account vs a non-service account: Perhaps having account types: Service account vs User account(non-service account)?

@kamingleung
Copy link

@scrawfor99 Is there an epic issue on Service Accounts for us to follow-up on some of my previous questions? We can focus this issue mainly on terminology.

@stephen-crawford
Copy link
Contributor Author

Hi @kamingleung,

The actual introduction of Service Accounts took place a while back with these two issues PRs: 1201335 & 24e08bd

The issue that this all stemmed from is here: #2704.

Let me know if that is what you were looking for.

@stephen-crawford
Copy link
Contributor Author

stephen-crawford commented Jun 9, 2023

These are excellent points. Let me clarify:

  1. Internal Users is the term used to refer to the users stored inside the Security Index. These users are authenticated with the Internal identity provider (IdP) as opposed to an external IdP. As opposed to the other type of "User" in the Security codebase, the Internal Users use the system index to track roles and things like Dls/Fls. The other "User" object in the Security plugin is a similar idea but makes use of stashing in the ThreadContext and is what allows for use of external IdPs such as the SAML backend. We could change the nomenclature at some point but we would have to wait for a breaking change and I suspect we are relatively locked into its use. An option would be the addition of an alternative naming scheme while also supporting the old one but this would add overhead.
  2. This is definitely true. It is more or less the thought process behind [Extensions] Filter internal users for Service Accounts and add associated API #2704 in the first place. Because we treat everything the same way, we want to be able to add some more specifics for an admin to be able to tell "oh these are all my employees" and "I can see this other list is all my extensions running."

I hope this helps.

@stephen-crawford
Copy link
Contributor Author

[Triage] Closing this issue since it is not realistic to change the nomenclature for the components. A better option for handling this is simply resolving on the front end side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking This issue is or proposes a breaking change enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants