Skip to content
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

fix(notification): Use site_url in notification contexts #11077

Open
wants to merge 1 commit into
base: bugfix
Choose a base branch
from

Conversation

kiblik
Copy link
Contributor

@kiblik kiblik commented Oct 16, 2024

Dojo was using a static URL while changing the context in the notification menu.

Now, URL is generated from SITE_URL

Reported on https://owasp.slack.com/archives/C2P5BA8MN/p1729065595314239

Copy link

dryrunsecurity bot commented Nov 11, 2024

DryRun Security Summary

The code change in this pull request updates the URL used to navigate to the notifications page based on the selected scope, replacing a hardcoded URL with a Django URL template tag to improve security, maintainability, and flexibility.

Expand for full summary

Summary:

The code change in this pull request updates the URL used to navigate to the notifications page based on the selected scope. The change replaces the hardcoded URL /notifications/ with a Django URL template tag {% url 'notifications' %}, which allows the URL to be generated dynamically based on the application's URL configuration. This is a positive improvement from a security perspective, as it helps to prevent potential security issues related to URL manipulation and improves the maintainability and flexibility of the application's URL structure.

Files Changed:

  • dojo/templates/dojo/notifications.html: The change in this file updates the JavaScript code that handles the change event for the notification scope selection. The hardcoded URL /notifications/ is replaced with a Django URL template tag {% url 'notifications' %}, which generates the URL dynamically based on the application's URL configuration. This change improves the security and maintainability of the application by avoiding hardcoded URLs and allowing the URL structure to be managed centrally in the application's URL configuration.

Code Analysis

We ran 9 analyzers against 1 file and 0 analyzers had findings. 9 analyzers had no findings.

View PR in the DryRun Dashboard.

@Maffooch
Copy link
Contributor

@kiblik is this one ready for review? it looks pretty straightforward to me 😄

@mtesauro
Copy link
Contributor

@kiblik Bumping this one again - can this be taken out of draft?

@kiblik
Copy link
Contributor Author

kiblik commented Dec 2, 2024

@kiblik Bumping this one again - can this be taken out of draft?

I came with idea, how to fix it but never had time to test it :)

@kiblik
Copy link
Contributor Author

kiblik commented Dec 2, 2024

Ok, it looks like it works. Feel free to review.

@kiblik kiblik marked this pull request as ready for review December 2, 2024 14:25
@kiblik kiblik requested review from mtesauro and Maffooch December 9, 2024 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants