Improve Slack integration to allow notifying private, public, and group channels (fixes #8569)
This change enables users to set private, public, or group channels as the target for Slack notifications.
It achieves this by removing the slack_user_id intermediary field, which was filled with a response after querying the Slack API with the user's email, and which limited the notification targets to users' private Slack channels. Before removal, the value is copied into slack_username field, overwriting the email address, which is no longer needed (and no longer works).
Detailed explanations are in #8569.
Test results
- Unit tests pass.
- SLA breach notifications are delivered successfully when
slack_usernameis set to a channel names, with and without a "#" prefix. - Also success when a channel ID is used.
Documentation
No changes in the documentation are needed. However, I noticed that the Slack administration page looks now differently, so I updated the relevant screen shot in the "Notifications" section.
Checklist
This checklist is for your information.
- [x] Unit tests pass.
- [x] Your code is flake8 compliant.
- [x] Model changes must include the necessary migrations in the dojo/db_migrations folder.
Contextual Security Analysis
As DryRun Security performs checks, we’ll summarize them here. You can always dive into the detailed results in the section below for checks.
| Status | DryRun Security Check |
|---|---|
| ✅ | Sensitive Functions Analyzer |
| ❌ | Configured Sensitive Files Check |
| ✅ | Sensitive Files Analyzer |
Chat with your AI-powered Security Buddy by typing @dryrunsecurity followed by your question into a comment.
Example: @dryrunsecurity What are common security issues with web application cookies?
Install and configure more repositories at DryRun Security
@cneill, @kiblik, please review if possible
Hi @tomaszn, we reviewed this PR but weren't able to replicate the problems mentioned. Could you please provide additional details on the issues you experienced?
@devGregA, sure. The simplest use case is setting up notifications for #general Slack channel. It is currently not possible, as far as I know.
Hi @tomaszn , I tested creating and linking the Slack integration and I was able to post two different kinds of notifications:
- personal notifications to an individual user's Slackbot (which is what the Slack Email Address field is used for in the DefectDojo UI. Not sure what this maps to on the backend).
- system-wide notifications to a shared channel
I was able to define the Slack Channel for posts on the Slack-app side, when I created and installed it to my workplace.
I updated the documentation with what I found: https://github.com/DefectDojo/django-DefectDojo/pull/9420/commits/750e548e5e3f6393e755e3ae8bc3cd56ece9f972
Currently, Dojo does not support any customized or filtered notifications, or posts to multiple slack channels. Is that what you're intending to do by changing the notification targets from Slack Usernames to Slack Channels?
If you wanted to add that feature, I think it would be more appropriate to add a Slack Channel mapping field on a Product, rather than on each user's profile which is where the Slack email address currently exists.
Hello @paulOsinski, thank you for your input here, and for the update to the documentation.
Currently, Dojo does not support any customized or filtered notifications, or posts to multiple slack channels. Is that what you're intending to do by changing the notification targets from Slack Usernames to Slack Channels?
Correct. And by creating "virtual" users and assigning them Slack channels I can customize notifications. Several channels can get notifications about events in independently configured sets of products.
If you wanted to add that feature, I think it would be more appropriate to add a Slack Channel mapping field on a Product, rather than on each user's profile which is where the Slack email address currently exists.
In my case, I need several Slack channels to be informed about various events in the products. So a simple 1:1 mapping would not be enough.
I acknowledge that the current solution is not perfect and serves more as a workaround. A fully-featured notification manager capable of handling mappings and preferences is something we should aim for in the future.
Please bear in mind that this changeset was prepared in a feature freeze situation. The goal had to be obtained with a minimal changes not to disrupt work of others :)
This pull request has conflicts, please resolve those before we can evaluate the pull request.
Close as stale