Skip to content

feat: allow disabling and dismissing the open source message banner#15089

Open
devGregA wants to merge 6 commits into
DefectDojo:devfrom
devGregA:devgrega/os-message-opt-out
Open

feat: allow disabling and dismissing the open source message banner#15089
devGregA wants to merge 6 commits into
DefectDojo:devfrom
devGregA:devgrega/os-message-opt-out

Conversation

@devGregA

Copy link
Copy Markdown
Contributor

The open source message banner (the "Upgrade to Pro" promo fetched from the
GCS bucket) currently can't be turned off. This adds two ways to do that.

  1. DD_OS_MESSAGE_ENABLED (default True) — an instance-wide switch.
    When it's off, get_os_banner() returns early, so we don't fetch the
    message or render the banner at all.

  2. A dismiss (×) button on the banner. Dismissing it is remembered per user
    by storing a hash of the current message on UserContactInfo. If the
    message later changes, the hash no longer matches and the banner shows
    again. This follows the same pattern as the existing announcement
    dismissal, minus the confirmation page.

Default behavior is unchanged: leave the env var unset and don't click ×,
and the banner behaves exactly as it does today.

Notes:

  • New os_message_dismissed_hash field on UserContactInfo (migration 0270).
  • The × posts to a small dismiss_os_message view (CSRF handled by the form);
    JS hides the banner immediately and falls back to a normal POST if JS is off.
    Handled in both the classic and v3 base templates.
  • Settings doc / template-env updated and a note added to the 3.1 upgrade page.

Testing: extended unittests/test_os_message.py (env gate, dismiss-token,
context-processor gating, and the dismiss view). All passing locally.

…r the OS promo banner

DD_OS_MESSAGE_ENABLED (bool, default True) is a global admin opt-out: when False, get_os_banner() returns None before any network call, so no request is made to the GCS bucket. Default behavior is unchanged.

Authenticated users can also dismiss the banner via a close (x) button. The dismissal is stored as a hash of the current message on UserContactInfo (mirroring the ui_use_tailwind preference), so the banner reappears only when the promo text changes. The button posts to a new dismiss_os_message endpoint (form-based CSRF) and hides the banner instantly via JS, degrading to a normal POST when JS is disabled.

Includes migration 0270, docs and template-env updates, and unit tests (36 passing in unittests/test_os_message.py).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions github-actions Bot added New Migration Adding a new migration file. Take care when merging. settings_changes Needs changes to settings.py based on changes in settings.dist.py included in this PR docs unittests ui labels Jun 26, 2026
…utton

Moves the dismiss (x) to lead the banner, before the headline, so it is clearly separated from the expand caret and avoids misclicks. Replaces the reused Bootstrap '.close' class (which collided with auto-generated v3 UI styles) with a dedicated 'os-message-dismiss' class styled as a small bordered button.

Lays out the OS banner as a centered flex row scoped to [data-source="os"], so the styling applies only to this banner; other banners and the messages notifications are unaffected.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@devGregA devGregA marked this pull request as ready for review June 26, 2026 03:45
@devGregA

Copy link
Copy Markdown
Contributor Author

@DryRunSecurity fp drs_d57257ef This isn't user input. banner.message is fetched from our own GCS bucket and already runs through bleach.clean() in parse_os_message() (allowlist of strong/em/a, strip=True) before it hits the template. The |safe also predates this PR; I just moved the line when I reordered the dismiss button.

@devGregA

Copy link
Copy Markdown
Contributor Author

@DryRunSecurity fp drs_f11c06a6 Same deal, this is the classic-UI copy of that same line.

@dryrunsecurity

Copy link
Copy Markdown

Finding d57257ef has been successfully dismissed and marked as False Positive.

@dryrunsecurity

Copy link
Copy Markdown

Finding f11c06a6 has been successfully dismissed and marked as False Positive.

…URE_LOCATIONS

TestDismissOsMessageView loaded dojo_testdata.json directly, which raises NotImplementedError under V3_FEATURE_LOCATIONS=True (the fixture contains deprecated Endpoint records). Adding the @versioned_fixtures decorator swaps it to dojo_testdata_locations.json under V3, matching the convention used by the rest of the suite. Test-only change; verified passing locally under both V3=True and V3=False.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@mtesauro mtesauro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved

@github-actions

Copy link
Copy Markdown
Contributor

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

@valentijnscholten

valentijnscholten commented Jun 26, 2026

Copy link
Copy Markdown
Member

I am going to push a slightly different way of implementing this flag so that we don't have to create a new column and migration for every future flag. But happy to stick to the original implementation if needed/wanted.

…ls JSON field

Replace the dedicated UserContactInfo.os_message_dismissed_hash column with a
single user_state_details JSONField that holds an open-ended set of small,
per-user UI flags (dismissed banners, 'don't show again' toggles, ...). The
banner-dismissal hash now lives under the OS_MESSAGE_DISMISSED_KEY key.

This avoids adding a new column (and migration) for every minor per-user toggle;
future ephemeral flags of this kind become a new JSON key with no schema change.
Behavioral/queryable fields (block_execution, token_*, ...) intentionally stay
as columns.
# Conflicts:
#	dojo/announcement/ui/views.py
#	dojo/models.py
@valentijnscholten

Copy link
Copy Markdown
Member

Nice feature. One design suggestion on the storage, which I've gone ahead and implemented on the branch (plus resolved the conflicts with dev).

Use an extensible JSON field instead of a column per flag

The dismissal was stored in a dedicated os_message_dismissed_hash column on UserContactInfo. That field is editable=False, never queried/filtered, and is one of a whole category of "remember the user dismissed/saw X" flags that keep showing up. Adding a column (and a migration) per minor per-user toggle is a lot of schema churn — and every UserContactInfo migration is a fresh source of migration-number collisions in the ongoing bugfix→dev merges.

So I replaced it with a single extensible field:

user_state_details = models.JSONField(default=dict, blank=True, editable=False, ...)

The dismissal hash now lives under a OS_MESSAGE_DISMISSED_KEY key inside it. Future ephemeral UI flags ("don't show again", "seen the X tour", etc.) become a new key — no column, no migration.

Deliberately scoped: this is only for open-ended, non-queried UI state. Behavioral/queryable/security fields (block_execution, token_*, etc.) should stay as real columns.

Caveats worth knowing:

  • Concurrency: writing a key is read-modify-write on the blob; fine for low-frequency UI prefs, but if two keys are ever written concurrently use a Postgres jsonb_set partial update to avoid clobbering.
  • Serializer/admin exposure is manual per key — fine here since it's editable=False internal state.

Conflicts with dev resolved

Merged current dev in. Two conflicts, both from the #14987 reorg that just landed:

  • UserContactInfo moved to dojo/user/models.py — added user_state_details there (took dev's re-export models.py).
  • announcement views moved to dojo/announcement/ui/views.py — kept dev's new-module imports and added the OS_MESSAGE_DISMISSED_KEY / UserContactInfo imports the dismiss view needs.

Migration renamed to 0270_usercontactinfo_user_state_details (adds the JSONField; no collision — dev has no 0270).

@github-actions

Copy link
Copy Markdown
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

…ilterset

The new user_state_details JSONField broke UserContactInfoViewSet, which used
filterset_fields = "__all__". django-filter's AutoFilterSet cannot build a
filter for a JSONField and raised:

  AssertionError: AutoFilterSet resolved field 'user_state_details' with
  'exact' lookup to an unrecognized field type JSONField.

Replace the auto filterset with an explicit ApiUserContactInfoFilter that
excludes the field (internal UI state, not meant to be filtered); all other
fields keep their previous auto-filter behaviour.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs New Migration Adding a new migration file. Take care when merging. settings_changes Needs changes to settings.py based on changes in settings.dist.py included in this PR ui unittests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants