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

[Extensions] Filter internal users for Service Accounts and add associated API #2704

Closed
Tracked by #2944
stephen-crawford opened this issue Apr 18, 2023 · 4 comments · Fixed by #2786
Closed
Tracked by #2944
Assignees
Labels
enhancement New feature or request triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable.

Comments

@stephen-crawford
Copy link
Contributor

As a follow-up to #2594, a filtering option can be added to the existing internal users functionality. The filtering should allow a client to fetch just true internal user (not service) accounts or just service accounts. This filtering should be applied to the backend as well as the internal users list capabilities of the frontend code. An API route will need to be added to the handleGet method of the internalUsersAPI since there is not currently group filtering functionality.

The backend logic can look something like this inside the UserService:

public static List<String> listServiceAccounts() {

        final SecurityDynamicConfiguration<?> internalUsersConfiguration = load(getConfigName(), false);

        List<String> serviceAccounts = new ArrayList<>();
        for (Map.Entry<String, ?> entry : internalUsersConfiguration.getCEntries().entrySet()) {

            final InternalUserV7 internalUserEntry = (InternalUserV7) entry.getValue();
            final Map accountAttributes = internalUserEntry.getAttributes();
            final String accountName = entry.getKey();
            if (accountAttributes.containsKey("service") && accountAttributes.get("service") == "true") {
                serviceAccounts.add(accountName);
            }
        }
        return serviceAccounts;
    }

    public static List<String> listInternalUsers() {

        final SecurityDynamicConfiguration<?> internalUsersConfiguration = load(getConfigName(), false);

        List<String> internalUserAccounts = new ArrayList<>();
        for (Map.Entry<String, ?> entry : internalUsersConfiguration.getCEntries().entrySet()) {

            final InternalUserV7 internalUserEntry = (InternalUserV7) entry.getValue();
            final Map accountAttributes = internalUserEntry.getAttributes();
            final String accountName = entry.getKey();
            if (!accountAttributes.containsKey("service") || accountAttributes.get("service") == "false") {
                internalUserAccounts.add(accountName);
            }
        }
        return internalUserAccounts;
    }

    public static Set<String> listUserAccounts() {

        final SecurityDynamicConfiguration<?> internalUsersConfiguration = load(getConfigName(), false);
        return internalUsersConfiguration.getCEntries().keySet();
    }

For the frontend logic, the PR will need to extend the existing code that fetches the internal users and displays it as part of the Security tab.

Completion of this issue should look like two PR:

One in the backend code base that introduces the filtering capabilities shown above as well as the associated API routes.
One in the frontend code base that updates the internal users tab to allow for filtering the shown users based on whether they are service accounts or not.

@stephen-crawford stephen-crawford converted this from a draft issue Apr 18, 2023
@github-actions github-actions bot added the untriaged Require the attention of the repository maintainers and may need to be prioritized label Apr 18, 2023
@stephen-crawford stephen-crawford added enhancement New feature or request triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable. and removed untriaged Require the attention of the repository maintainers and may need to be prioritized labels Apr 18, 2023
@samuelcostae
Copy link
Contributor

Hi @scrawfor99

The preliminary code that you provided in the issue description returns only the list of names of the accounts of the specified type, but the existing implementations on internalUsersAPI return the full account details.

Can you confirm if the objective on this is
1 - Just return the list of names
2 - Return the full account details.

I believe the latter is more appropriate to keep in line with existing functionality, but just want to confirm.

Thanks

@samuelcostae
Copy link
Contributor

Confirmed during last week's call that the full account details is the correct approach. PR was refactored to reflect this

samuelcostae added a commit to samuelcostae/security that referenced this issue Jul 10, 2023
@stephen-crawford stephen-crawford moved this from Todo to Awaiting Review in Security for Extensions Jul 18, 2023
@peternied peternied mentioned this issue Aug 16, 2023
14 tasks
stephen-crawford pushed a commit that referenced this issue Sep 22, 2023
…2786)

### Description
Update UserAPIAction to Filter internal accounts and Service accounts.  

Dashboards Plugin changes via PR
opensearch-project/security-dashboards-plugin#1502

### Issues Resolved
- Resolves #2704 

### Testing
Unit Tests created 

### Check List
- [x] New functionality includes testing
- [ ] New functionality has been documented
- [x] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and
signing off your commits, please check
[here](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin).

---------

Signed-off-by: scosta <[email protected]>
Signed-off-by: Ryan Liang <[email protected]>
Signed-off-by: Sam <[email protected]>
Signed-off-by: Sam <[email protected]>
Co-authored-by: Ryan Liang <[email protected]>
Co-authored-by: Darshit Chanpura <[email protected]>
@github-project-automation github-project-automation bot moved this from Awaiting Review to Done in Security for Extensions Sep 22, 2023
@github-actions github-actions bot added the untriaged Require the attention of the repository maintainers and may need to be prioritized label Sep 26, 2023
@samuelcostae
Copy link
Contributor

samuelcostae commented Sep 26, 2023

Reopened as the PR on the dashboards side opensearch-project/security-dashboards-plugin#1502 still needs to be merged/reviewed

@davidlago davidlago removed the untriaged Require the attention of the repository maintainers and may need to be prioritized label Oct 2, 2023
@samuelcostae
Copy link
Contributor

Pr merged, can be closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants