attend to review feedback

Author: eokoneyo

src/core/packages/user-profile/common/src/user_profile.ts

@@ -60,9 +60,29 @@ export interface UserProfileUserInfo {
 }
 
 /**
- * Placeholder for data stored in user profile.
+ * Placeholder for data stored in user profile,
+ * services that store data in the user profile should specify said data by augmenting this type in it's implementation,
+ * like so:
+ *
+ * @example
+ * ```ts
+ * declare module '@kbn/core-user-profile-common' {
+ *   interface UserProfileData {
+ *     myService: {
+ *       myData: string;
+ *     };
+ *   }
+ * }
+ * ```
+ * This will make it such that the return value for the invocation of `getCurrent` is typed matching the defined augmentation
+ *
+ * ```ts
+ * const userProfile = await userProfileService.getCurrent();
+ * // accessing 'myService.myData' is now typed as 'string'
+ * console.log(userProfile.data.myService.myData);
+ * ```
  */
-// eslint-disable-next-line @typescript-eslint/no-empty-interface -- This is intentional, so that consumers can augment this type with values they intend to store in the user profile data
+// eslint-disable-next-line @typescript-eslint/no-empty-interface -- See the comment above for an explanation.
 export interface UserProfileData {}
 
 /**

src/core/packages/user-settings/server-internal/src/user_settings_service.test.ts

@@ -10,7 +10,7 @@
 import { mockCoreContext } from '@kbn/core-base-server-mocks';
 import { httpServerMock } from '@kbn/core-http-server-mocks';
 import { userProfileServiceMock } from '@kbn/core-user-profile-server-mocks';
-import type { UserProfileWithSecurity } from '@kbn/core-user-profile-common';
+import type { UserProfileWithSecurity, UserProfileData } from '@kbn/core-user-profile-common';
 import { UserSettingsService } from './user_settings_service';
 
 describe('#setup', () => {
@@ -26,18 +26,18 @@ describe('#setup', () => {
     };
   });
 
-  const createUserProfile = (darkMode: string | undefined): UserProfileWithSecurity => {
+  const createUserProfile = (
+    userSettings: Partial<NonNullable<UserProfileData['userSettings']>>
+  ): UserProfileWithSecurity => {
     return {
       data: {
-        userSettings: {
-          darkMode,
-        },
+        userSettings,
       },
     } as unknown as UserProfileWithSecurity;
   };
 
   it('fetches userSettings when client is set and returns `true` when `darkMode` is set to `dark`', async () => {
-    startDeps.userProfile.getCurrent.mockResolvedValue(createUserProfile('dark'));
+    startDeps.userProfile.getCurrent.mockResolvedValue(createUserProfile({ darkMode: 'dark' }));
 
     const { getUserSettingDarkMode } = service.setup();
     service.start(startDeps);
@@ -54,7 +54,7 @@ describe('#setup', () => {
   });
 
   it('fetches userSettings when client is set and returns `false` when `darkMode` is set to `light`', async () => {
-    startDeps.userProfile.getCurrent.mockResolvedValue(createUserProfile('light'));
+    startDeps.userProfile.getCurrent.mockResolvedValue(createUserProfile({ darkMode: 'light' }));
 
     const { getUserSettingDarkMode } = service.setup();
     service.start(startDeps);
@@ -71,7 +71,7 @@ describe('#setup', () => {
   });
 
   it('fetches userSettings when client is set and returns `system` when `darkMode` is set to `system`', async () => {
-    startDeps.userProfile.getCurrent.mockResolvedValue(createUserProfile('system'));
+    startDeps.userProfile.getCurrent.mockResolvedValue(createUserProfile({ darkMode: 'system' }));
 
     const { getUserSettingDarkMode } = service.setup();
     service.start(startDeps);
@@ -88,7 +88,7 @@ describe('#setup', () => {
   });
 
   it('fetches userSettings when client is set and returns `undefined` when `darkMode` is set to `` (the default value)', async () => {
-    startDeps.userProfile.getCurrent.mockResolvedValue(createUserProfile(''));
+    startDeps.userProfile.getCurrent.mockResolvedValue(createUserProfile({ darkMode: undefined }));
 
     const { getUserSettingDarkMode } = service.setup();
     service.start(startDeps);
@@ -105,7 +105,9 @@ describe('#setup', () => {
   });
 
   it('fetches userSettings when client is set and returns `undefined` when `darkMode` is set to `space_default`', async () => {
-    startDeps.userProfile.getCurrent.mockResolvedValue(createUserProfile('space_default'));
+    startDeps.userProfile.getCurrent.mockResolvedValue(
+      createUserProfile({ darkMode: 'space_default' })
+    );
 
     const { getUserSettingDarkMode } = service.setup();
     service.start(startDeps);
@@ -121,6 +123,63 @@ describe('#setup', () => {
     });
   });
 
+  it('fetches userSettings when client is set and returns `true` when `rememberSelectedSpace` is set to `true`', async () => {
+    startDeps.userProfile.getCurrent.mockResolvedValue(
+      createUserProfile({ rememberSelectedSpace: true })
+    );
+
+    const { getUserSettingRememberSelectedSpace } = service.setup();
+    service.start(startDeps);
+
+    const kibanaRequest = httpServerMock.createKibanaRequest();
+    const rememberSelectedSpace = await getUserSettingRememberSelectedSpace(kibanaRequest);
+
+    expect(rememberSelectedSpace).toEqual(true);
+    expect(startDeps.userProfile.getCurrent).toHaveBeenCalledTimes(1);
+    expect(startDeps.userProfile.getCurrent).toHaveBeenCalledWith({
+      request: kibanaRequest,
+      dataPath: 'userSettings',
+    });
+  });
+
+  it('fetches userSettings when client is set and returns `false` when `rememberSelectedSpace` is set to `false`', async () => {
+    startDeps.userProfile.getCurrent.mockResolvedValue(
+      createUserProfile({ rememberSelectedSpace: false })
+    );
+
+    const { getUserSettingRememberSelectedSpace } = service.setup();
+    service.start(startDeps);
+
+    const kibanaRequest = httpServerMock.createKibanaRequest();
+    const rememberSelectedSpace = await getUserSettingRememberSelectedSpace(kibanaRequest);
+
+    expect(rememberSelectedSpace).toEqual(false);
+    expect(startDeps.userProfile.getCurrent).toHaveBeenCalledTimes(1);
+    expect(startDeps.userProfile.getCurrent).toHaveBeenCalledWith({
+      request: kibanaRequest,
+      dataPath: 'userSettings',
+    });
+  });
+
+  it('fetches userSettings when client is set and returns `true` when `rememberSelectedSpace` is not set', async () => {
+    startDeps.userProfile.getCurrent.mockResolvedValue(
+      createUserProfile({ rememberSelectedSpace: undefined })
+    );
+
+    const { getUserSettingRememberSelectedSpace } = service.setup();
+    service.start(startDeps);
+
+    const kibanaRequest = httpServerMock.createKibanaRequest();
+    const rememberSelectedSpace = await getUserSettingRememberSelectedSpace(kibanaRequest);
+
+    expect(rememberSelectedSpace).toEqual(true);
+    expect(startDeps.userProfile.getCurrent).toHaveBeenCalledTimes(1);
+    expect(startDeps.userProfile.getCurrent).toHaveBeenCalledWith({
+      request: kibanaRequest,
+      dataPath: 'userSettings',
+    });
+  });
+
   it('does not fetch userSettings when client is not set, returns `undefined`, and logs a debug statement', async () => {
     const { getUserSettingDarkMode } = service.setup();
 

x-pack/platform/plugins/shared/security/server/routes/user_profile/update.test.ts

@@ -306,6 +306,7 @@ describe('Update profile routes', () => {
         userSettings: {
           darkMode: 'dark',
           contrastMode: 'high',
+          rememberSelectedSpace: true,
         },
       };
 

x-pack/platform/plugins/shared/security/server/routes/user_profile/update.ts

@@ -6,7 +6,9 @@
  */
 
 import { schema } from '@kbn/config-schema';
+import type { UserProfileData } from '@kbn/core-user-profile-common';
 import { isValidUserProfileAvatarColor } from '@kbn/user-profile-components';
+import type { DotKeysOf } from '@kbn/utility-types';
 
 import type { RouteDefinitionParams } from '..';
 import { IMAGE_FILE_TYPES } from '../../../common/constants';
@@ -15,6 +17,9 @@ import { flattenObject } from '../../lib';
 import { getPrintableSessionId } from '../../session_management';
 import { createLicensedRouteHandler } from '../licensed_route_handler';
 
+type UserProfileUpdatePayload = Pick<UserProfileData, 'userSettings'>;
+type UserProfileUpdatePaths = DotKeysOf<UserProfileUpdatePayload>;
+
 /** User profile data keys that are allowed to be updated by Cloud users */
 const ALLOWED_KEYS_UPDATE_CLOUD = [
   'userSettings.darkMode',
@@ -23,7 +28,7 @@ const ALLOWED_KEYS_UPDATE_CLOUD = [
   'userSettings.agentBuilderAnnouncementModalSeenBySpaceJson',
   'userSettings.locale',
   'userSettings.rememberSelectedSpace',
-];
+] as const satisfies readonly UserProfileUpdatePaths[];
 
 const MAX_STRING_FIELD_LENGTH = 1024;
 
@@ -59,7 +64,9 @@ const userProfileUpdateSchema = schema.object({
       ),
       locale: schema.maybe(schema.string({ maxLength: MAX_STRING_FIELD_LENGTH })),
       rememberSelectedSpace: schema.maybe(schema.boolean()),
-      lastSelectedSpaceId: schema.nullable(schema.string({ maxLength: MAX_STRING_FIELD_LENGTH })),
+      lastSelectedSpaceId: schema.maybe(
+        schema.nullable(schema.string({ maxLength: MAX_STRING_FIELD_LENGTH }))
+      ),
     })
   ),
 });
@@ -157,8 +164,9 @@ export function defineUpdateUserProfileDataRoute({
       if (currentUser?.elastic_cloud_user) {
         // We only allow specific user profile data to be updated by Elastic Cloud SSO users.
         const isUpdateAllowed = keysToUpdate.every((key) =>
-          ALLOWED_KEYS_UPDATE_CLOUD.includes(key)
+          (ALLOWED_KEYS_UPDATE_CLOUD as readonly string[]).includes(key)
         );
+
         if (keysToUpdate.length === 0 || !isUpdateAllowed) {
           logger.warn(
             `Elastic Cloud SSO users aren't allowed to update profiles in Kibana. (sid: ${getPrintableSessionId(