-
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] adjust policy onboarding view, check for Ingest permissions #70536
Conversation
@elasticmachine merge upstream |
@elasticmachine merge upstream |
…na into task/onboarding-ux-updates
/> | ||
} | ||
bodyComponent={ | ||
<FormattedMessage | ||
id="xpack.securitySolution.endpoint.endpointList.noEndpointsInstructions" | ||
defaultMessage="Elastic Endpoint Security gives you the power to keep your endpoints safe from attack, as well as unparalleled visibility into any threat in your environment." | ||
defaultMessage="You’ve created your security policy. Now you need to enable the Elastic Endpoint Security capabilities on your agents following the steps below. For additional information, view our ‘getting started guide’." |
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.
what the abbreviation of 've
dataTestSubj="emptyEndpointsTable" | ||
steps={policySteps} | ||
headerComponent={ | ||
<FormattedMessage | ||
id="xpack.securitySolution.endpoint.endpointList.noEndpointsPrompt" | ||
defaultMessage="You have a policy, but no Endpoints are deployed!" | ||
defaultMessage="Enable Elastic Endpoint Security on your agents" |
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.
agents? Is something people are expected to know?
<EuiText size="s" color="subdued"> | ||
<FormattedMessage | ||
id="xpack.securitySolution.endpoint.policyList.onboardingSectionTwo" | ||
defaultMessage="You’ll be able to view and manage hosts in your environment running the Elastic Endpoint from this page." |
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.
You'll
may be You will
?
id="xpack.securitySolution.endpoint.policyList.stepThree" | ||
defaultMessage="If you haven’t already, enroll your agents through Fleet using the same agent configuration." | ||
id="xpack.securitySolution.endpoint.policyList.placeholdForPictures" | ||
defaultMessage="Pictures!!!" |
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.
Suggestion that you use a placeholder pic while the official one is being determined. Maybe:
<EuiIcon type="logoSecurity" size="xl" />
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.
wasn't planning on merging until I got the asset, but that's a good alternative to keep moving
}); | ||
|
||
it('should display the No Permissions view when Ingest is OFF', async () => { | ||
(useIngestEnabledCheck as jest.Mock).mockReturnValue({ allEnabled: false }); |
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.
Why do you need to cast the useIngestEnabledCheck
? You should not have to, so I'm thinking something in the mocked interface is not right.
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.
you're right that it's not required, was just following other examples - will remove
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.
return ( | ||
<Switch> | ||
<Route path={MANAGEMENT_ROUTING_ENDPOINTS_PATH} component={EndpointsContainer} /> | ||
<Route path={MANAGEMENT_ROUTING_POLICIES_PATH} component={PolicyContainer} /> | ||
<Route |
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.
I had been wondering about this. You had mentioned before (maybe standup) that the tab was disabled, but rally, what is happening is that we still show/enable the tab, but display a different page - correct?
Also - Instead of using the conditional below in all <Route>
's, just add one at before this return
, so that we don't have to keep writing the condition on each <Route>
- like :
const { allEnabled: isIngestEnabled } = useIngestEnabledCheck();
if (!isIngestEnabled) {
return `<Route path="*" component={NoPermissions} />`;
}
return (
<Switch>
<Route path={MANAGEMENT_ROUTING_ENDPOINTS_PATH} component={EndpointsContainer} />
<Route path={MANAGEMENT_ROUTING_POLICIES_PATH} component={PolicyContainer} />
//....
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.
@paul-tavares yes, we do want the tab to still show up, that's an ask from product so that we do not hide newer features. This way users can discover why they are not able to use the feature and adjust permissions accordingly.
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.
Ok - The issue is on how the mock is defined then. It probably just needs to be adjusted there to something like jest.Mock<Typehere>
.
We can address it later (maybe open an issue so we don't forget :) )
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.
lgtm
@elasticmachine merge upstream |
@elasticmachine merge upstream |
💛 Build succeeded, but was flaky
Test FailuresKibana Pipeline / kibana-xpack-agent / Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/lens/smokescreen·ts.lens app lens smokescreen tests should allow creation of lens visualizationsStandard Out
Stack Trace
Build metrics@kbn/optimizer bundle module count
History
To update your PR or re-run it, just comment with: |
…ermissions (elastic#70536) * adjust policy onboarding view * correct test subj * fix tests * re-enable tests * add no permissions view * adjust onbording look * adjust text * use ingest hook, add tests * adjust text * address comments * beta badges * fix test * correct timeline flyout Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Pinging @elastic/siem (Team:SIEM) |
Pinging @elastic/security-solution (Team: SecuritySolution) |
Summary
In this PR:
New onboarding screen for No Policy (graphic from designers pending):

New oboarding screen for No Endpoint (docs link pending):

Admin disabled screen:

Checklist
Delete any items that are not applicable to this PR.