-
Notifications
You must be signed in to change notification settings - Fork 419
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
[Slack] Add slack integration #3278
Conversation
/test |
Pinging @elastic/security-external-integrations (Team:Security-External Integrations) |
@legoguy1000 Thanks for the PR. Do you know how the outh2 authentication works for this Slack integration? @P1llus and I had some discussions with Slack in the past and it seemed each Slack user would have to approve it, rather than an admin approving for the entire organization. I think this was the case for the Events API, but looks like you're hitting the Alerts API with this integration so hopefully it just requires admin approval. |
So there are different kinds of apps that can be made with different oauth scopes. For things like Google drive/mail/calendar... Each user can enable the app to connect their account to their slack account. But for something like this, the scopes are org/enterprise wide. Slack got rid of the legacy method to create API keys to manage the workspace, so an "app" is the only way to create a key with scopes like that. I think the misleading thing is it's not really an "app" in the traditional way when thinking of slack integrations. |
While I'm limited in my ability to test the audit logs because of the enterprise plan requirement, I did test the concept on the access logs API and a few others and was able to pull the data without issue. |
/test |
c6deef6
to
c684bbb
Compare
fixed errors, ready to retest |
@andrewkroh can we rerun the tests? |
/test |
🌐 Coverage report
|
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 with the exception of the rate limit behavior; I think that needs a manual verification that it's working.
/test |
/test |
@andrewkroh I was able to mock the API and the Rate Limiting works as expected. When a 429 is returned the agent waits the set amount of seconds. |
f74b2a4
to
4f25a58
Compare
/test |
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. Once elastic/elastic-package#821 is implemented we should revisit the testing to make the assertion automated.
What does this PR do?
Create an initial Slack integration to view audit logs
Checklist
changelog.yml
file.Author's Checklist
How to test this PR locally
Related issues
Screenshots