[Enterprise Search] Add User management feature #103173

scottybollinger · 2021-06-23T19:54:08Z

Summary

This is the remaining monster PR for the User management feature in Kibana. It follows #102826 and is a combined port of https://github.com/elastic/ent-search/pull/3861 and https://github.com/elastic/ent-search/pull/3853.

There are also some bugfix ports added as the last several commits. When applicable, a link to the ent-search equivalent and/or issue is linked.

This is a DRYtemare so please ignore duplicated code.

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios

This is shared with the forthcoming user flyouts closeRoleMappingFlyout -> closeUsersAndRolesFlyout

- Showing and hiding flyouts - Select and text input values - User created state to turn flyout into a success message state

- Users & roles -> Users and roles - roleId -> roleMappingId (matches other places in code) - also added a missing prop to the actions col

The UI sets the default group on page load but did not cover the case where the user has chosen a group in a previous interaction and the closed the flyout. This commit adds a method that resets that state when the flyout is closed Part of porting of https://github.com/elastic/ent-search/pull/3865 Specifically: https://github.com/elastic/ent-search/commit/a4131b95dab7c0df97bd78e660f25e09ac3e7cec

This was missed from the design Part of porting of https://github.com/elastic/ent-search/pull/3865 Specifically: https://github.com/elastic/ent-search/commit/03aa349cab4fb32069b64ab8c51a7252ba52e805

Role-> RoleTypes 🤷🏽‍♀️

Wasn’t needed in ent-search; already done for RomeMappingFlyout. Hide whitespace changes plskthx

Since we're moving fully into Kibana, we're losing our concept of auth providers. In 8.0, role mappings the specify an auth provider will no longer work, so this adds a small deprecation warning in the role mappings table. https://github.com/elastic/ent-search/pull/3885

After a slack discussion, it was determined that email should be optional. This commit also fixes another instance of the App Search role type being wrong.

kibanamachine · 2021-06-23T20:10:04Z

⏳ Build in-progress, with failures

continuous-integration/kibana-ci/pull-request
Commit: 76c0c4c
Storybooks not built yet
This comment will update when the build is complete

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

kibanamachine · 2021-06-23T20:10:05Z

⏳ Build in-progress, with failures

continuous-integration/kibana-ci/pull-request
Commit: 76c0c4c
Storybooks not built yet
This comment will update when the build is complete

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

kibanamachine · 2021-06-23T20:10:05Z

⏳ Build in-progress, with failures

continuous-integration/kibana-ci/pull-request
Commit: 76c0c4c
Storybooks not built yet
This comment will update when the build is complete

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

kibanamachine · 2021-06-23T20:10:06Z

⏳ Build in-progress, with failures

continuous-integration/kibana-ci/pull-request
Commit: 76c0c4c
Storybooks not built yet
This comment will update when the build is complete

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

scottybollinger · 2021-06-23T20:11:06Z

@elasticmachine merge upstream

x-pack/plugins/enterprise_search/server/routes/workplace_search/role_mappings.test.ts

cee-chen · 2021-06-23T20:23:52Z

.../enterprise_search/public/applications/app_search/components/role_mappings/role_mappings.tsx

  } = useValues(RoleMappingsLogic);

  useEffect(() => {
    initializeRoleMappings();
    return resetState;
  }, []);

+  const hasUsers = singleUserRoleMappings.length > 0;


[optional techdebt for the future] I've recently been refactoring flags like these to Kea selectors, it just generally feels nicer to have them in Kea

Need to change folder and file names but will punt until after 7.14FF I did throw in updating the logic file path

scottybollinger · 2021-06-23T22:42:22Z

@elasticmachine merge upstream

scottybollinger · 2021-06-24T01:37:10Z

@elasticmachine merge upstream

kibanamachine · 2021-06-24T03:44:10Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`enterpriseSearch`	1455	1457	+2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`enterpriseSearch`	2.1MB	2.2MB	+31.9KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cee-chen · 2021-06-24T04:36:04Z

...k/plugins/enterprise_search/public/applications/app_search/components/role_mappings/user.tsx

+const standardRoles = (['owner', 'admin'] as unknown) as RoleTypes[];
+const advancedRoles = (['dev', 'editor', 'analyst'] as unknown) as RoleTypes[];


Ha, fun with Typescript - this is where enums are a little nicer to use than string unions I think (not a change request, just me rambling out loud)

cee-chen · 2021-06-24T04:48:03Z

...ins/enterprise_search/public/applications/workplace_search/views/role_mappings/user.test.tsx

+    setMockActions({
+      handleSaveUser,


[not a change request, just a comment] in general in AS we put our actions in a single obj, similar to mockValues, so they don't need to be redeclared individually here:

const mockActions = { a: jest.fn(), b: jest.fn() } // etc setMockActions(mockActions);

just a little less repetitive is all

cee-chen · 2021-06-24T04:55:13Z

...ck/plugins/enterprise_search/public/applications/shared/role_mapping/role_mappings_table.tsx

 import { RoleRules } from '../types';

 import './role_mappings_table.scss';

+const AUTH_PROVIDER_DOCUMENTATION_URL = `${docLinks.enterpriseSearchBase}/users-access.html`;


[optional] It might be nice to move documentation links to constants.ts as well for organization

cee-chen · 2021-06-24T04:58:14Z

...ck/plugins/enterprise_search/public/applications/shared/role_mapping/role_mappings_table.tsx

+          <EuiLink href={AUTH_PROVIDER_DOCUMENTATION_URL} target="_blank">
+            <EuiIconTip type="alert" color="warning" content={AUTH_PROVIDER_TOOLTIP} />
+          </EuiLink>
+        </span>


This is QAing well for me 👍

cee-chen · 2021-06-24T04:59:33Z

...k/plugins/enterprise_search/public/applications/shared/role_mapping/user_added_info.test.tsx

+  it('renders without email', () => {
+    const wrapper = shallow(<UserAddedInfo {...props} email="" />);
+
+    expect(wrapper).toMatchInlineSnapshot(`


nice, glad you got to use inline snapshots so soon! these test names are great for helping understand/look for the rendered diffs, thanks for that 💯

cee-chen · 2021-06-24T05:00:13Z

x-pack/plugins/enterprise_search/public/applications/shared/role_mapping/user_added_info.tsx

@@ -19,6 +19,8 @@ interface Props {
  roleType: string;
 }

+const noItemsPlaceholder = <EuiTextColor color="subdued">&mdash;</EuiTextColor>;


[optional/personal preference] If this isn't being reused in multiple places, maybe just leave it in inline? 🤷

cee-chen · 2021-06-24T05:03:26Z

x-pack/plugins/enterprise_search/public/applications/shared/role_mapping/user_flyout.tsx

+    <EuiPortal>
+      <EuiFlyout ownFocus onClose={closeUserFlyout} size="s" aria-labelledby="userFlyoutTitle">


Nice catch on this EuiPortal!

Super optional, but as of EUI 34.x I believe EuiFlyout now defaults to ownFocus={true} so you could leave it out if you wanted. Don't worry if you'd rather not though, there's plenty of other flyouts in our app that still have ownFocus, so we could opt to clean it up across the board in a separate PR

cee-chen · 2021-06-24T05:07:14Z

...rprise_search/public/applications/app_search/components/role_mappings/role_mappings_logic.ts

 }

 export const RoleMappingsLogic = kea<MakeLogicType<RoleMappingsValues, RoleMappingsActions>>({
-  path: ['enterprise_search', 'app_search', 'role_mappings'],
+  path: ['enterprise_search', 'app_search', 'users_and_roles'],


[not a change request] I wonder if we should similarly change all our folder/file names (e.g. components/role_mappings -> components/user_and_roles). Might make more sense in 7.15 when we're cleaning/DRYing things out though, especially if we split up the logic files into role_mappings_logic vs users_logic

cee-chen · 2021-06-24T05:13:25Z

...rprise_search/public/applications/app_search/components/role_mappings/role_mappings_logic.ts

+      const singleUserRoleMapping = values.singleUserRoleMappings.find(
+        ({ roleMapping }) => roleMapping.id === roleMappingId
+      );


[not a change request] in a future refactor, do you think it would be worth storing a map copy of of user roles with id as key so lookup is immediate rather than having to search through an array?... 🤔

... i'm definitely overthinking this. just give me my own paragraph in this blog post already

EDIT: HMMM wait tho, we're already using Set() in this logic file! I mean, if we're ALREADY being all cool and microperfing, I'm just sayin'

cee-chen · 2021-06-24T05:20:06Z

...rprise_search/public/applications/app_search/components/role_mappings/role_mappings_logic.ts

+      const user = values.elasticsearchUsers.find((u) => u.username === username);
+      if (user) actions.setElasticsearchUser(user);


[optional, just me being a jerk] i'm generally not a super huge fan of single letter vars, maybe this could help w/ specificity?

Suggested change

const user = values.elasticsearchUsers.find((u) => u.username === username);

if (user) actions.setElasticsearchUser(user);

const selectedUser = values.elasticsearchUsers.find((user) => user.username === username);

if (selectedUser) actions.setElasticsearchUser(selectedUser);

cee-chen

Optional comments only!! My QA was not super thorough but from what I could tell everything on AS worked with both role mappings and users 🎉

Seriously can't applaud y'all for getting this out as fast as you did, huge kudos!

* Rename method to close both flyouts This is shared with the forthcoming user flyouts closeRoleMappingFlyout -> closeUsersAndRolesFlyout * Add logic for elasticsearch users and single user role mappings * Add logic for various form states - Showing and hiding flyouts - Select and text input values - User created state to turn flyout into a success message state * Add User server routes * Add logic for saving a user * Add User components * Add User list and User flyout to RoleMappings view * Fix path * Rename things - Users & roles -> Users and roles - roleId -> roleMappingId (matches other places in code) - also added a missing prop to the actions col * Set default group when modal closed The UI sets the default group on page load but did not cover the case where the user has chosen a group in a previous interaction and the closed the flyout. This commit adds a method that resets that state when the flyout is closed Part of porting of elastic/ent-search#3865 Specifically: elastic/ent-search@a4131b9 * Adds tooltip for external attribute This was missed from the design Part of porting of elastic/ent-search#3865 Specifically: elastic/ent-search@03aa349 * Fix invitations link * Fix incorrect role type Role-> RoleTypes 🤷🏽‍♀️ * Add EuiPortal to Flyout Wasn’t needed in ent-search; already done for RomeMappingFlyout. Hide whitespace changes plskthx * Auth provider deprecation warning in mapping UI Since we're moving fully into Kibana, we're losing our concept of auth providers. In 8.0, role mappings the specify an auth provider will no longer work, so this adds a small deprecation warning in the role mappings table. elastic/ent-search#3885 * Email is no longer required After a slack discussion, it was determined that email should be optional. This commit also fixes another instance of the App Search role type being wrong. * Existing users’ usernames should not be editable * Use EuiLink instead of anchor * Add validation tests * Change URL for users_and_roles Need to change folder and file names but will punt until after 7.14FF I did throw in updating the logic file path * Remove unused import Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

kibanamachine · 2021-06-24T13:00:55Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

scottybollinger · 2021-06-24T13:02:35Z

Thanks @constancecchen! CI issues yesterday had me worried and I have PRs depending on this so I merged and just made notes of all the change requests in the refactor doc for 7.15.

* Rename method to close both flyouts This is shared with the forthcoming user flyouts closeRoleMappingFlyout -> closeUsersAndRolesFlyout * Add logic for elasticsearch users and single user role mappings * Add logic for various form states - Showing and hiding flyouts - Select and text input values - User created state to turn flyout into a success message state * Add User server routes * Add logic for saving a user * Add User components * Add User list and User flyout to RoleMappings view * Fix path * Rename things - Users & roles -> Users and roles - roleId -> roleMappingId (matches other places in code) - also added a missing prop to the actions col * Set default group when modal closed The UI sets the default group on page load but did not cover the case where the user has chosen a group in a previous interaction and the closed the flyout. This commit adds a method that resets that state when the flyout is closed Part of porting of elastic/ent-search#3865 Specifically: elastic/ent-search@a4131b9 * Adds tooltip for external attribute This was missed from the design Part of porting of elastic/ent-search#3865 Specifically: elastic/ent-search@03aa349 * Fix invitations link * Fix incorrect role type Role-> RoleTypes 🤷🏽‍♀️ * Add EuiPortal to Flyout Wasn’t needed in ent-search; already done for RomeMappingFlyout. Hide whitespace changes plskthx * Auth provider deprecation warning in mapping UI Since we're moving fully into Kibana, we're losing our concept of auth providers. In 8.0, role mappings the specify an auth provider will no longer work, so this adds a small deprecation warning in the role mappings table. elastic/ent-search#3885 * Email is no longer required After a slack discussion, it was determined that email should be optional. This commit also fixes another instance of the App Search role type being wrong. * Existing users’ usernames should not be editable * Use EuiLink instead of anchor * Add validation tests * Change URL for users_and_roles Need to change folder and file names but will punt until after 7.14FF I did throw in updating the logic file path * Remove unused import Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Scotty Bollinger <scotty.bollinger@elastic.co>

scottybollinger added 17 commits June 23, 2021 14:49

Rename method to close both flyouts

d62ea2b

This is shared with the forthcoming user flyouts closeRoleMappingFlyout -> closeUsersAndRolesFlyout

Add logic for elasticsearch users and single user role mappings

1a6cf9d

Add logic for various form states

1881ef5

- Showing and hiding flyouts - Select and text input values - User created state to turn flyout into a success message state

Add User server routes

c154afc

Add logic for saving a user

859f9d1

Add User components

ccf902c

Add User list and User flyout to RoleMappings view

cf8aad3

Fix path

c668bef

Rename things

0f6977e

- Users & roles -> Users and roles - roleId -> roleMappingId (matches other places in code) - also added a missing prop to the actions col

Adds tooltip for external attribute

cb7307e

This was missed from the design Part of porting of https://github.com/elastic/ent-search/pull/3865 Specifically: https://github.com/elastic/ent-search/commit/03aa349cab4fb32069b64ab8c51a7252ba52e805

Fix invitations link

2fcccc3

Fix incorrect role type

80c4f0e

Role-> RoleTypes 🤷🏽‍♀️

Add EuiPortal to Flyout

9d1fb67

Wasn’t needed in ent-search; already done for RomeMappingFlyout. Hide whitespace changes plskthx

Email is no longer required

08e954d

After a slack discussion, it was determined that email should be optional. This commit also fixes another instance of the App Search role type being wrong.

Existing users’ usernames should not be editable

76c0c4c

scottybollinger added Feature:Plugins release_note:skip Skip the PR/issue when compiling release notes v7.14.0 auto-backport Deprecated - use backport:version if exact versions are needed labels Jun 23, 2021

scottybollinger requested review from a team June 23, 2021 19:54

Use EuiLink instead of anchor

9d85270

Merge branch 'master' into scottybollinger/es-user-management

bb6c539

cee-chen reviewed Jun 23, 2021

View reviewed changes

x-pack/plugins/enterprise_search/server/routes/workplace_search/role_mappings.test.ts Show resolved Hide resolved

cee-chen reviewed Jun 23, 2021

View reviewed changes

scottybollinger added 3 commits June 23, 2021 15:38

Add validation tests

ae4c9bf

Change URL for users_and_roles

92e493e

Need to change folder and file names but will punt until after 7.14FF I did throw in updating the logic file path

Remove unused import

ea7776a

Merge branch 'master' into scottybollinger/es-user-management

e0924c2

Merge branch 'master' into scottybollinger/es-user-management

c14ebf5

cee-chen reviewed Jun 24, 2021

View reviewed changes

cee-chen approved these changes Jun 24, 2021

View reviewed changes

scottybollinger merged commit a50d949 into elastic:master Jun 24, 2021

kibanamachine mentioned this pull request Jun 24, 2021

[7.x] [Enterprise Search] Add User management feature (#103173) #103267

Merged

scottybollinger deleted the scottybollinger/es-user-management branch June 24, 2021 13:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Enterprise Search] Add User management feature #103173

[Enterprise Search] Add User management feature #103173

scottybollinger commented Jun 23, 2021

kibanamachine commented Jun 23, 2021

kibanamachine commented Jun 23, 2021

kibanamachine commented Jun 23, 2021

kibanamachine commented Jun 23, 2021

scottybollinger commented Jun 23, 2021

cee-chen Jun 23, 2021

scottybollinger commented Jun 23, 2021

scottybollinger commented Jun 24, 2021

kibanamachine commented Jun 24, 2021

cee-chen Jun 24, 2021

cee-chen Jun 24, 2021

cee-chen Jun 24, 2021

cee-chen Jun 24, 2021

cee-chen Jun 24, 2021 •

edited

Loading

cee-chen Jun 24, 2021

cee-chen Jun 24, 2021

cee-chen Jun 24, 2021

cee-chen Jun 24, 2021 •

edited

Loading

cee-chen Jun 24, 2021 •

edited

Loading

cee-chen left a comment

kibanamachine commented Jun 24, 2021

scottybollinger commented Jun 24, 2021

		const standardRoles = (['owner', 'admin'] as unknown) as RoleTypes[];
		const advancedRoles = (['dev', 'editor', 'analyst'] as unknown) as RoleTypes[];

		<EuiPortal>
		<EuiFlyout ownFocus onClose={closeUserFlyout} size="s" aria-labelledby="userFlyoutTitle">

		const user = values.elasticsearchUsers.find((u) => u.username === username);
		if (user) actions.setElasticsearchUser(user);

[Enterprise Search] Add User management feature #103173

[Enterprise Search] Add User management feature #103173

Conversation

scottybollinger commented Jun 23, 2021

closes https://github.com/elastic/enterprise-search-team/issues/597

closes https://github.com/elastic/enterprise-search-team/issues/599

closes https://github.com/elastic/enterprise-search-team/issues/668

closes https://github.com/elastic/enterprise-search-team/issues/669

closes https://github.com/elastic/enterprise-search-team/issues/654

closes https://github.com/elastic/enterprise-search-team/issues/678

Summary

Checklist

kibanamachine commented Jun 23, 2021

⏳ Build in-progress, with failures

kibanamachine commented Jun 23, 2021

⏳ Build in-progress, with failures

kibanamachine commented Jun 23, 2021

⏳ Build in-progress, with failures

kibanamachine commented Jun 23, 2021

⏳ Build in-progress, with failures

scottybollinger commented Jun 23, 2021

Choose a reason for hiding this comment

scottybollinger commented Jun 23, 2021

scottybollinger commented Jun 24, 2021

kibanamachine commented Jun 24, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

History

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cee-chen Jun 24, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cee-chen Jun 24, 2021 • edited Loading

Choose a reason for hiding this comment

cee-chen Jun 24, 2021 • edited Loading

Choose a reason for hiding this comment

cee-chen left a comment

Choose a reason for hiding this comment

kibanamachine commented Jun 24, 2021

💚 Backport successful

scottybollinger commented Jun 24, 2021

cee-chen Jun 24, 2021 •

edited

Loading

cee-chen Jun 24, 2021 •

edited

Loading

cee-chen Jun 24, 2021 •

edited

Loading