From 32706a732b49ff5163841a7da3f64efd151f6189 Mon Sep 17 00:00:00 2001 From: Jacob Shandling <61553566+jacobshandling@users.noreply.github.com> Date: Mon, 20 Nov 2023 10:29:36 -0800 Subject: [PATCH] =?UTF-8?q?UI=20=E2=80=93=20Clarify=20activity=20items=20f?= =?UTF-8?q?or=20JIT=20provisioned=20SSO=20user=20initial=20logins=20(#1519?= =?UTF-8?q?2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Addresses #14345 ![Screenshot 2023-11-17 at 11 43 59 AM](https://github.com/fleetdm/fleet/assets/61553566/b97634dc-53c1-4ddd-910e-8dd7112c2623) - [x] Changes file added for user-visible changes in `changes/` - [x] Added/updated tests - [x] Manual QA for all new/changed functionality --------- Co-authored-by: Jacob Shandling --- .../14345-JIT-provisioned-login-activities | 2 + frontend/interfaces/activity.ts | 1 + .../ActivityItem/ActivityItem.tests.tsx | 158 +++++++++++++++++- .../ActivityItem/ActivityItem.tsx | 56 +++++-- 4 files changed, 204 insertions(+), 13 deletions(-) create mode 100644 changes/14345-JIT-provisioned-login-activities diff --git a/changes/14345-JIT-provisioned-login-activities b/changes/14345-JIT-provisioned-login-activities new file mode 100644 index 0000000000..76c76e557e --- /dev/null +++ b/changes/14345-JIT-provisioned-login-activities @@ -0,0 +1,2 @@ +- Update activity feed to elegantly communicate when a JIT-provisioned user logs in for the first + time, thereby creating their account. diff --git a/frontend/interfaces/activity.ts b/frontend/interfaces/activity.ts index 6057623bf4..1cb7bfc6a5 100644 --- a/frontend/interfaces/activity.ts +++ b/frontend/interfaces/activity.ts @@ -80,6 +80,7 @@ export interface IActivityDetails { specs?: IQuery[] | IPolicy[]; global?: boolean; public_ip?: string; + user_id?: number; user_email?: string; email?: string; role?: UserRole; diff --git a/frontend/pages/DashboardPage/cards/ActivityFeed/ActivityItem/ActivityItem.tests.tsx b/frontend/pages/DashboardPage/cards/ActivityFeed/ActivityItem/ActivityItem.tests.tsx index 03e03c7590..3735e97298 100644 --- a/frontend/pages/DashboardPage/cards/ActivityFeed/ActivityItem/ActivityItem.tests.tsx +++ b/frontend/pages/DashboardPage/cards/ActivityFeed/ActivityItem/ActivityItem.tests.tsx @@ -1,5 +1,5 @@ import React from "react"; -import { render, screen, getDefaultNormalizer } from "@testing-library/react"; +import { render, screen } from "@testing-library/react"; import createMockActivity from "__mocks__/activityMock"; import createMockQuery from "__mocks__/queryMock"; @@ -232,6 +232,10 @@ describe("Activity Feed", () => { ).toBeInTheDocument(); }); + // // // // // // // // // // // // + // created_user tests + // // // // // //// // // // // // + it("renders a created_user type activity globally", () => { const activity = createMockActivity({ type: ActivityType.UserCreated, @@ -245,6 +249,24 @@ describe("Activity Feed", () => { expect(screen.getByText("newuser@example.com")).toBeInTheDocument(); }); + it("correctly renders a created_user type activity for a premium SSO user created by JIT provisioning", () => { + const activity = createMockActivity({ + actor_full_name: "Jit Sso", + actor_id: 3, + type: ActivityType.UserCreated, + details: { + user_id: 3, + }, + }); + render(); + + // If actor_id is the same as user_id: + // " activated their account." + expect(screen.getByText("Jit Sso")).toBeInTheDocument(); + expect(screen.getByText(/activated their account\./)).toBeInTheDocument(); + }); + // // // // // //// // // // // // + it("renders a deleted_user type activity globally", () => { const activity = createMockActivity({ type: ActivityType.UserDeleted, @@ -258,6 +280,10 @@ describe("Activity Feed", () => { expect(screen.getByText("newuser@example.com")).toBeInTheDocument(); }); + // // // // // // // // // // // // + // changed_user_global_role tests + // // // // // //// // // // // // + it("renders a changed_user_global_role type activity globally for premium users", () => { const activity = createMockActivity({ type: ActivityType.UserChangedGlobalRole, @@ -287,7 +313,78 @@ describe("Activity Feed", () => { expect(forAllTeams).toBeNull(); }); - it("renders a changed_user_team_role type activity globally", () => { + it("correctly renders a changed_user_global_role type activity for a premium SSO user created by JIT provisioning", () => { + const activity = createMockActivity({ + actor_id: 3, + type: ActivityType.UserChangedGlobalRole, + details: { + user_id: 3, + user_email: "jit@sso.com", + role: "observer", + }, + }); + render(); + + // If actor_id is the same as user_id: + // " was assigned the for all teams." + expect(screen.getByText("jit@sso.com")).toBeInTheDocument(); + expect(screen.getByText(/was assigned the/)).toBeInTheDocument(); + expect(screen.getByText("observer")).toBeInTheDocument(); + expect(screen.getByText(/role for all teams./)).toBeInTheDocument(); + }); + + it("correctly renders a changed_user_global_role type activity when changing an existing user's global role, premium", () => { + const activity = createMockActivity({ + actor_id: 1, + actor_full_name: "Ally Admin", + type: ActivityType.UserChangedGlobalRole, + details: { + user_id: 3, + user_email: "user@example.com", + role: "maintainer", + }, + }); + render(); + + // If actor_id is different from user_id on premium: + // " changed to for all teams." + expect(screen.getByText("Ally Admin")).toBeInTheDocument(); + expect(screen.getByText(/changed/)).toBeInTheDocument(); + expect(screen.getByText("user@example.com")).toBeInTheDocument(); + expect(screen.getByText(/to/)).toBeInTheDocument(); + expect(screen.getByText("maintainer")).toBeInTheDocument(); + expect(screen.getByText(/for all teams/)).toBeInTheDocument(); + }); + + it("correctly renders a changed_user_global_role type activity when changing an existing user's global role, free", () => { + const activity = createMockActivity({ + actor_id: 1, + actor_full_name: "Ally Admin", + type: ActivityType.UserChangedGlobalRole, + details: { + user_id: 3, + user_email: "user@example.com", + role: "maintainer", + }, + }); + render(); + + // If actor_id is different from user_id on free: + // " changed to ." + expect(screen.getByText("Ally Admin")).toBeInTheDocument(); + expect(screen.getByText("changed", { exact: false })).toBeInTheDocument(); + expect(screen.getByText("user@example.com")).toBeInTheDocument(); + expect(screen.getByText("to", { exact: false })).toBeInTheDocument(); + expect(screen.getByText("maintainer")).toBeInTheDocument(); + const forAllTeams = screen.queryByText("for all teams."); + expect(forAllTeams).toBeNull(); + }); + + // // // // // // // // // // // // + // changed_user_team_role tests + // // // // // //// // // // // // + + it("renders a changed_user_team_role type activity", () => { const activity = createMockActivity({ type: ActivityType.UserChangedTeamRole, details: { @@ -304,6 +401,63 @@ describe("Activity Feed", () => { expect(screen.getByText("Test Team")).toBeInTheDocument(); }); + it("correctly renders a changed_user_team_role type activity when a new SSO team user is created via JIT provisioning", () => { + const activity = createMockActivity({ + actor_id: 1, + actor_full_name: "Ally Admin", + type: ActivityType.UserChangedTeamRole, + details: { + user_id: 1, + user_email: "jit@sso.com", + role: "maintainer", + team_name: "Test Team", + }, + }); + render(); + + // If actor_id is the same as user_id: + // " was assigned the role for the team." + expect(screen.getByText("jit@sso.com")).toBeInTheDocument(); + expect(screen.getByText(/was assigned the/)).toBeInTheDocument(); + expect(screen.getByText("maintainer")).toBeInTheDocument(); + expect(screen.getByText(/role for the/)).toBeInTheDocument(); + expect(screen.getByText(/Test Team/)).toBeInTheDocument(); + expect(screen.getByText(/team\./)).toBeInTheDocument(); + + expect(screen.queryByText("Ally Admin")).toBeNull(); + const forAllTeams = screen.queryByText("for all teams."); + expect(forAllTeams).toBeNull(); + }); + + it("correctly renders a changed_user_team_role type activity when changing an existing user's team role", () => { + const activity = createMockActivity({ + actor_id: 1, + actor_full_name: "Ally Admin", + type: ActivityType.UserChangedTeamRole, + details: { + user_id: 3, + user_email: "user@example.com", + role: "maintainer", + team_name: "Test Team", + }, + }); + render(); + + // If actor_id is different from user_id: + // " changed to for the team." + expect(screen.getByText("Ally Admin")).toBeInTheDocument(); + expect(screen.getByText(/changed/)).toBeInTheDocument(); + expect(screen.getByText("user@example.com")).toBeInTheDocument(); + expect(screen.getByText(/to/)).toBeInTheDocument(); + expect(screen.getByText("maintainer")).toBeInTheDocument(); + expect(screen.getByText(/for the/)).toBeInTheDocument(); + expect(screen.getByText(/Test Team/)).toBeInTheDocument(); + expect(screen.getByText(/team\./)).toBeInTheDocument(); + expect(screen.queryByText("for all teams.")).toBeNull(); + }); + + // // // // // // // // // // // // + it("renders a deleted_user_team_role type activity globally", () => { const activity = createMockActivity({ type: ActivityType.UserDeletedTeamRole, diff --git a/frontend/pages/DashboardPage/cards/ActivityFeed/ActivityItem/ActivityItem.tsx b/frontend/pages/DashboardPage/cards/ActivityFeed/ActivityItem/ActivityItem.tsx index 1582e5d291..996ab5f760 100644 --- a/frontend/pages/DashboardPage/cards/ActivityFeed/ActivityItem/ActivityItem.tsx +++ b/frontend/pages/DashboardPage/cards/ActivityFeed/ActivityItem/ActivityItem.tsx @@ -173,7 +173,9 @@ const TAGGED_TEMPLATES = { ); }, userCreated: (activity: IActivity) => { - return ( + return activity.actor_id === activity.details?.user_id ? ( + <>activated their account. + ) : ( <> created a user {activity.details?.user_email}. @@ -187,10 +189,22 @@ const TAGGED_TEMPLATES = { ); }, userChangedGlobalRole: (activity: IActivity, isPremiumTier: boolean) => { + const { actor_id } = activity; + const { user_id, user_email, role } = activity.details || {}; + + if (actor_id === user_id) { + // this is the case when SSO user is crated via JIT provisioning + // should only be possible for premium tier, but check anyway + return ( + <> + was assigned the {role} role{isPremiumTier && " for all teams"} + . + + ); + } return ( <> - changed {activity.details?.user_email} to{" "} - {activity.details?.role} + changed {user_email} to {activity.details?.role} {isPremiumTier && " for all teams"}. ); @@ -205,11 +219,22 @@ const TAGGED_TEMPLATES = { ); }, userChangedTeamRole: (activity: IActivity) => { + const { actor_id } = activity; + const { user_id, user_email, role, team_name } = activity.details || {}; + + const varText = + actor_id === user_id ? ( + <> + was assigned the {role} role + + ) : ( + <> + changed {user_email} to {role} + + ); return ( <> - changed {activity.details?.user_email} to{" "} - {activity.details?.role} for the{" "} - {activity.details?.team_name} team. + {varText} for the {team_name} team. ); }, @@ -751,6 +776,19 @@ const ActivityItem = ({ const indicatePremiumFeature = isSandboxMode && PREMIUM_ACTIVITIES.has(activity.type); + const renderActivityPrefix = () => { + if (activity.type === ActivityType.UserLoggedIn) { + return {activity.actor_email} ; + } + if ( + (activity.type === ActivityType.UserChangedGlobalRole || + activity.type === ActivityType.UserChangedTeamRole) && + activity.actor_id === activity.details?.user_id + ) { + return {activity.details?.user_email} ; + } + return {activity.actor_full_name} ; + }; return (
{indicatePremiumFeature && } - {activity.type === ActivityType.UserLoggedIn ? ( - {activity.actor_email} - ) : ( - {activity.actor_full_name} - )} + {renderActivityPrefix()} {getDetail(activity, isPremiumTier, onDetailsClick)}