-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Security Solution][Detections] Add assignees UI into alerts table (#7661) #167079
[Security Solution][Detections] Add assignees UI into alerts table (#7661) #167079
Conversation
Pinging @elastic/security-solution (Team: SecuritySolution) |
…eature/alert-user-assignment-7661
…eature/alert-user-assignment-7661
…eature/alert-user-assignment-7661
…eature/alert-user-assignment-7661
… flyout component (elastic#7662)
… details flyout component (elastic#7662)" This reverts commit 94a5f5c.
setLoading(true); | ||
const fetchData = async () => { | ||
try { | ||
const usersResponse = await fetchUserProfiles(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fetching all user profiles here then filtering them on the client side seems dangerous. Is it possible to only fetch the profiles for IDs on the current page of the alerts table?
I have a similar need for the kibana.alert.workflow_user
field, I'd like to display the username rather than the profile ID.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to present user the list of all available user to pick assignees.
This mimics similar functionality in cases
plugin (x-pack/plugins/cases/public/containers/user_profiles/use_suggest_user_profiles.ts
).
One this what we might need to improve is required privileges to fetch user profiles, right now I allow to fetch profiles if user can login (x-pack/plugins/security_solution/server/lib/detection_engine/routes/signals/get_user_profiles_route.ts
):
const users = await security.userProfiles.suggest({
name: '',
dataPath: 'avatar',
requiredPrivileges: {
spaceId,
privileges: {
kibana: [security.authz.actions.login],
},
},
});
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to pass the name
in to get the correct suggestions?
I saw that this hook is also used in getRenderCellValueHook
though to convert from profile UID to username for display in the table. In that case wouldn't we need to ensure that we're fetching the correct profiles by ID when rendering the table? By default it appears the suggest API only returns 10 users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I was playing around with three users in my environment that's why I did not hit the limit of 10 suggestions. I will adjust suggest APIs and also will add one more route to be able to fetch specific user profiles by passing their ids.
Do you think that will cover all the cases for kibana.alert.workflow_user
as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I think the solution here should cover what we'll need for workflow_user
. There's a UserProfileAPIClient
in public
already though, I wonder if we can use that instead of making our own new APIs? Something like
import type { UserProfileWithAvatar } from '@kbn/user-profile-components';
import { useEffect, useState } from 'react';
import { useAppToasts } from '../../../../common/hooks/use_app_toasts';
import { useKibana } from '../../../../common/lib/kibana';
import { USER_PROFILES_FAILURE } from './translations';
interface UserProfilesStatus {
loading: boolean;
userProfiles: UserProfileWithAvatar[];
}
// Gather all uids from one page of the alerts table and fetch those userprofiles in one request
export const useUserProfiles = ({ uids }: { uids: string[] }): UserProfilesStatus => {
const [loading, setLoading] = useState(false);
const [users, setUsers] = useState<UserProfileWithAvatar[]>([]);
const { addError } = useAppToasts();
const userProfiles = useKibana().services.security.userProfiles;
useEffect(() => {
// isMounted tracks if a component is mounted before changing state
let isMounted = true;
setLoading(true);
const fetchData = async () => {
try {
const usersResponse = await userProfiles.bulkGet({ uids: new Set(uids) });
if (isMounted) {
setUsers(usersResponse);
}
} catch (error) {
addError(error.message, { title: USER_PROFILES_FAILURE });
}
if (isMounted) {
setLoading(false);
}
};
fetchData();
return () => {
// updates to show component is unmounted
isMounted = false;
};
}, [addError, uids, userProfiles]);
return { loading, userProfiles: users };
};
for bulkGet
, there's also a suggest
method on the client so we might be able to use this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ohh, seems like I overcomplicated things around UserProfileAPIClient
APIs, I use those APIs, but on the server side of security solution :-) I will have a look and double check that it works correctly without an unnecessary trip to our server first.
I just updated and added both options to fetch user profiles and suggested user profiles for security solution. There are two hooks now: useSuggestUsers(serachTerm: string)
and useGetUserProfiles(userIds: string[])
. I will adjust those to use public APIs directly on security solution client side.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like we need to implement our own API at least for suggest
call. Here is the note from the UserProfileAPIClient
:
Note: This endpoint is not provided out-of-the-box by the platform. You need to expose your own
version within your app. An example of how to do this can be found in:
`examples/user_profile_examples/server/plugin.ts`
…eature/alert-user-assignment-7661
…eature/alert-user-assignment-7661
…eature/alert-user-assignment-7661
💔 Build FailedFailed CI StepsTest Failures
Metrics [docs]
History
To update your PR or re-run it, just comment with: cc @e40pud |
dc9fcff
into
elastic:security/feature/alert-user-assignment
Summary
Closes https://github.com/elastic/security-team/issues/7661
This PR adds Alert user assignment UI within alerts table.
Screen.Recording.2023-09-22.at.21.11.01.mov