Redesigned user profile page (#127624)

* Add user profile UI * add code doc * More * Fix nav control for anonymous users * Move presentational logic outside form row * Fix disabled state and update profile api * remove presentational logic from form row * wip * Fix initials * . * empty user avatar * simplify reserved user screen * . * unit tests * type fixes * . * . * . * . * . * . * . * . * . * . * . * . * . * . * . * . * . * [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix' * . * . * . * . * . * . * Review#1: handle UI inconsistencies, update avatar in the navigation bar when profile data is updated, etc. * Fix type errors. * Use different validation model for the first and subsequent submits. * Review#2: move change password form to a modal, support users authenticated via authentication proxies. * Use proper test subject for the password change Cancel button. * Review#3: retry profile activation requests. * Review#4: align text color of the non-editable profile fields with the designs. * Review#4: improve warning message for the change password form when changing password for Kibana system users. Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Aleh Zasypkin <[email protected]>
elastic · May 24, 2022 · 1775421 · 1775421
1 parent 169d503
commit 1775421
Show file tree

Hide file tree

Showing 74 changed files with 3,693 additions and 1,320 deletions.
diff --git a/package.json b/package.json
@@ -269,6 +269,7 @@
     "file-saver": "^1.3.8",
     "file-type": "^10.9.0",
     "font-awesome": "4.7.0",
+    "formik": "^2.2.9",
     "fp-ts": "^2.3.1",
     "geojson-vt": "^3.2.1",
     "get-port": "^5.0.0",

diff --git a/packages/kbn-optimizer/limits.yml b/packages/kbn-optimizer/limits.yml
@@ -52,7 +52,7 @@ pageLoadAssetSize:
   savedObjectsTagging: 59482
   savedObjectsTaggingOss: 20590
   searchprofiler: 67080
-  security: 95864
+  security: 100000
   snapshotRestore: 79032
   spaces: 57868
   telemetry: 51957

diff --git a/renovate.json b/renovate.json
@@ -115,6 +115,7 @@
       "groupName": "platform security modules",
       "matchPackageNames": [
         "node-forge",
+        "formik",
         "@types/node-forge",
         "require-in-the-middle",
         "tough-cookie",

diff --git a/x-pack/plugins/security/common/index.ts b/x-pack/plugins/security/common/index.ts
@@ -8,6 +8,7 @@
 export type { SecurityLicense, SecurityLicenseFeatures, LoginLayout } from './licensing';
 export type {
   AuthenticatedUser,
+  AuthenticatedUserProfile,
   AuthenticationProvider,
   PrivilegeDeprecationsService,
   PrivilegeDeprecationsRolesByFeatureIdRequest,
@@ -17,6 +18,10 @@ export type {
   RoleKibanaPrivilege,
   FeaturesPrivileges,
   User,
+  UserProfile,
+  UserData,
+  UserAvatar,
+  UserInfo,
   ApiKey,
   UserRealm,
 } from './model';
diff --git a/x-pack/plugins/security/common/model/authenticated_user.test.ts b/x-pack/plugins/security/common/model/authenticated_user.test.ts
@@ -5,8 +5,128 @@
  * 2.0.
  */
 
+import { applicationServiceMock } from '@kbn/core/public/mocks';
+
 import type { AuthenticatedUser } from './authenticated_user';
-import { canUserChangePassword } from './authenticated_user';
+import {
+  canUserChangeDetails,
+  canUserChangePassword,
+  canUserHaveProfile,
+  isUserAnonymous,
+} from './authenticated_user';
+import { mockAuthenticatedUser } from './authenticated_user.mock';
+
+describe('canUserChangeDetails', () => {
+  const { capabilities } = applicationServiceMock.createStartContract();
+
+  it('should indicate when user can change their details', () => {
+    expect(
+      canUserChangeDetails(
+        mockAuthenticatedUser({
+          authentication_realm: { type: 'native', name: 'native1' },
+        }),
+        {
+          ...capabilities,
+          management: {
+            security: {
+              users: true,
+            },
+          },
+        }
+      )
+    ).toBe(true);
+  });
+
+  it('should indicate when user cannot change their details', () => {
+    expect(
+      canUserChangeDetails(
+        mockAuthenticatedUser({
+          authentication_realm: { type: 'native', name: 'native1' },
+        }),
+        {
+          ...capabilities,
+          management: {
+            security: {
+              users: false,
+            },
+          },
+        }
+      )
+    ).toBe(false);
+
+    expect(
+      canUserChangeDetails(
+        mockAuthenticatedUser({
+          authentication_realm: { type: 'reserved', name: 'reserved1' },
+        }),
+        {
+          ...capabilities,
+          management: {
+            security: {
+              users: true,
+            },
+          },
+        }
+      )
+    ).toBe(false);
+  });
+});
+
+describe('isUserAnonymous', () => {
+  it('should indicate anonymous user', () => {
+    expect(
+      isUserAnonymous(
+        mockAuthenticatedUser({
+          authentication_provider: { type: 'anonymous', name: 'basic1' },
+        })
+      )
+    ).toBe(true);
+  });
+
+  it('should indicate non-anonymous user', () => {
+    expect(
+      isUserAnonymous(
+        mockAuthenticatedUser({
+          authentication_provider: { type: 'basic', name: 'basic1' },
+        })
+      )
+    ).toBe(false);
+  });
+});
+
+describe('canUserHaveProfile', () => {
+  it('anonymous users cannot have profiles', () => {
+    expect(
+      canUserHaveProfile(
+        mockAuthenticatedUser({
+          authentication_provider: { type: 'anonymous', name: 'basic1' },
+        })
+      )
+    ).toBe(false);
+  });
+
+  it('proxy authenticated users cannot have profiles', () => {
+    expect(
+      canUserHaveProfile(
+        mockAuthenticatedUser({
+          authentication_provider: { type: 'http', name: '__http__' },
+        })
+      )
+    ).toBe(false);
+  });
+
+  it('non-anonymous users that can have sessions can have profiles', () => {
+    for (const providerType of ['saml', 'oidc', 'basic', 'token', 'pki', 'kerberos']) {
+      expect(
+        canUserHaveProfile(
+          mockAuthenticatedUser({
+            authentication_provider: { type: providerType, name: `${providerType}_name` },
+          })
+        )
+      ).toBe(true);
+    }
+  });
+});
 
 describe('#canUserChangePassword', () => {
   ['reserved', 'native'].forEach((realm) => {

diff --git a/x-pack/plugins/security/common/model/authenticated_user.ts b/x-pack/plugins/security/common/model/authenticated_user.ts
@@ -5,6 +5,8 @@
  * 2.0.
  */
 
+import type { Capabilities } from '@kbn/core/types';
+
 import type { AuthenticationProvider } from './authentication_provider';
 import type { User } from './user';
 
@@ -42,9 +44,31 @@ export interface AuthenticatedUser extends User {
   authentication_type: string;
 }
 
-export function canUserChangePassword(user: AuthenticatedUser) {
+export function isUserAnonymous(user: Pick<AuthenticatedUser, 'authentication_provider'>) {
+  return user.authentication_provider.type === 'anonymous';
+}
+
+/**
+ * All users are supposed to have profiles except anonymous users and users authenticated
+ * via authentication HTTP proxies.
+ * @param user Authenticated user information.
+ */
+export function canUserHaveProfile(user: AuthenticatedUser) {
+  return !isUserAnonymous(user) && user.authentication_provider.type !== 'http';
+}
+
+export function canUserChangePassword(
+  user: Pick<AuthenticatedUser, 'authentication_realm' | 'authentication_provider'>
+) {
   return (
     REALMS_ELIGIBLE_FOR_PASSWORD_CHANGE.includes(user.authentication_realm.type) &&
-    user.authentication_provider.type !== 'anonymous'
+    !isUserAnonymous(user)
   );
 }
+
+export function canUserChangeDetails(
+  user: Pick<AuthenticatedUser, 'authentication_realm'>,
+  capabilities: Capabilities
+) {
+  return user.authentication_realm.type === 'native' && capabilities.management.security.users;
+}
diff --git a/x-pack/plugins/security/common/model/index.ts b/x-pack/plugins/security/common/model/index.ts
@@ -7,9 +7,26 @@
 
 export type { ApiKey, ApiKeyToInvalidate, ApiKeyRoleDescriptors } from './api_key';
 export type { User, EditUser } from './user';
+export type {
+  AuthenticatedUserProfile,
+  UserProfile,
+  UserData,
+  UserInfo,
+  UserAvatar,
+} from './user_profile';
+export {
+  getUserAvatarColor,
+  getUserAvatarInitials,
+  USER_AVATAR_MAX_INITIALS,
+} from './user_profile';
 export { getUserDisplayName } from './user';
 export type { AuthenticatedUser, UserRealm } from './authenticated_user';
-export { canUserChangePassword } from './authenticated_user';
+export {
+  canUserChangePassword,
+  canUserChangeDetails,
+  isUserAnonymous,
+  canUserHaveProfile,
+} from './authenticated_user';
 export type { AuthenticationProvider } from './authentication_provider';
 export { shouldProviderUseLoginForm } from './authentication_provider';
 export type { BuiltinESPrivileges } from './builtin_es_privileges';

diff --git a/x-pack/plugins/security/common/model/user.ts b/x-pack/plugins/security/common/model/user.ts
@@ -23,6 +23,6 @@ export interface EditUser extends User {
   confirmPassword?: string;
 }
 
-export function getUserDisplayName(user: User) {
+export function getUserDisplayName(user: Pick<User, 'username' | 'full_name'>) {
   return user.full_name || user.username;
 }
diff --git a/x-pack/plugins/security/common/model/user_profile.mock.ts b/x-pack/plugins/security/common/model/user_profile.mock.ts
@@ -0,0 +1,29 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import { mockAuthenticatedUser } from './authenticated_user.mock';
+import type { AuthenticatedUserProfile } from './user_profile';
+
+export const userProfileMock = {
+  create: (userProfile: Partial<AuthenticatedUserProfile> = {}): AuthenticatedUserProfile => {
+    const user = mockAuthenticatedUser({
+      username: 'some-username',
+      roles: [],
+      enabled: true,
+    });
+    return {
+      uid: 'some-profile-uid',
+      enabled: true,
+      user: {
+        ...user,
+        active: true,
+      },
+      data: {},
+      ...userProfile,
+    };
+  },
+};
diff --git a/x-pack/plugins/security/common/model/user_profile.ts b/x-pack/plugins/security/common/model/user_profile.ts
@@ -0,0 +1,116 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import { VISUALIZATION_COLORS } from '@elastic/eui';
+
+import type { User } from '..';
+import type { AuthenticatedUser } from './authenticated_user';
+import { getUserDisplayName } from './user';
+
+/**
+ * User information returned in user profile.
+ */
+export interface UserInfo extends User {
+  active: boolean;
+}
+
+/**
+ * Avatar stored in user profile.
+ */
+export interface UserAvatar {
+  initials?: string;
+  color?: string;
+  imageUrl?: string;
+}
+
+/**
+ * Placeholder for data stored in user profile.
+ */
+export type UserData = Record<string, unknown>;
+
+/**
+ * Describes properties stored in user profile.
+ */
+export interface UserProfile<T extends UserData = UserData> {
+  /**
+   * Unique ID for of the user profile.
+   */
+  uid: string;
+
+  /**
+   * Indicates whether user profile is enabled or not.
+   */
+  enabled: boolean;
+
+  /**
+   * Information about the user that owns profile.
+   */
+  user: UserInfo;
+
+  /**
+   * User specific data associated with the profile.
+   */
+  data: T;
+}
+
+/**
+ * User profile enriched with session information.
+ */
+export interface AuthenticatedUserProfile<T extends UserData = UserData> extends UserProfile<T> {
+  /**
+   * Information about the currently authenticated user that owns the profile.
+   */
+  user: UserProfile['user'] & Pick<AuthenticatedUser, 'authentication_provider'>;
+}
+
+export const USER_AVATAR_FALLBACK_CODE_POINT = 97; // code point for lowercase "a"
+export const USER_AVATAR_MAX_INITIALS = 2;
+
+/**
+ * Determines the color for the provided user profile.
+ * If a color is present on the user profile itself, then that is used.
+ * Otherwise, a color is provided from EUI's Visualization Colors based on the display name.
+ *
+ * @param {UserInfo} user User info
+ * @param {UserAvatar} avatar User avatar
+ */
+export function getUserAvatarColor(
+  user: Pick<UserInfo, 'username' | 'full_name'>,
+  avatar?: UserAvatar
+) {
+  if (avatar && avatar.color) {
+    return avatar.color;
+  }
+
+  const firstCodePoint = getUserDisplayName(user).codePointAt(0) || USER_AVATAR_FALLBACK_CODE_POINT;
+
+  return VISUALIZATION_COLORS[firstCodePoint % VISUALIZATION_COLORS.length];
+}
+
+/**
+ * Determines the initials for the provided user profile.
+ * If initials are present on the user profile itself, then that is used.
+ * Otherwise, the initials are calculated based off the words in the display name, with a max length of 2 characters.
+ *
+ * @param {UserInfo} user User info
+ * @param {UserAvatar} avatar User avatar
+ */
+export function getUserAvatarInitials(
+  user: Pick<UserInfo, 'username' | 'full_name'>,
+  avatar?: UserAvatar
+) {
+  if (avatar && avatar.initials) {
+    return avatar.initials;
+  }
+
+  const words = getUserDisplayName(user).split(' ');
+  const numInitials = Math.min(USER_AVATAR_MAX_INITIALS, words.length);
+
+  words.splice(numInitials, words.length);
+
+  return words.map((word) => word.substring(0, 1)).join('');
+}