django-DefectDojo icon indicating copy to clipboard operation
django-DefectDojo copied to clipboard

Improve Slack integration to allow notifying private, public, and group channels (fixes #8569)

Open tomaszn opened this issue 2 years ago • 6 comments

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_username is 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.

tomaszn avatar Nov 23 '23 13:11 tomaszn

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

dryrunsecurity[bot] avatar Nov 23 '23 13:11 dryrunsecurity[bot]

@cneill, @kiblik, please review if possible

tomaszn avatar Jan 16 '24 13:01 tomaszn

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 avatar Jan 16 '24 22:01 devGregA

@devGregA, sure. The simplest use case is setting up notifications for #general Slack channel. It is currently not possible, as far as I know.

tomaszn avatar Jan 16 '24 23:01 tomaszn

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.

paulOsinski avatar Jan 31 '24 18:01 paulOsinski

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 :)

tomaszn avatar Jan 31 '24 22:01 tomaszn

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions[bot] avatar Apr 06 '24 08:04 github-actions[bot]

Close as stale

mtesauro avatar Aug 20 '24 20:08 mtesauro