Repository: paperless-ngx/paperless-ngx
Pull request: Change: remove credentials from redis url in system status
<!--
Note: All PRs with code changes should be targeted to the `dev` branch, pure documentation changes can target `main`
-->

## Proposed change

Closes #6098 

## Type of change

<!--
What type of change does your PR introduce to Paperless-ngx?
NOTE: Please check only one box!
-->

- [ ] Bug fix: non-breaking change which fixes an issue.
- [ ] New feature / Enhancement: non-breaking change which adds functionality. _Please read the important note above._
- [ ] Breaking change: fix or feature that would cause existing functionality to not work as expected.
- [ ] Documentation only.
- [x] Other. Please explain: change

## Checklist:

<!--
NOTE: PRs that do not address the following will not be merged, please do not skip any relevant items.
-->

- [x] I have read & agree with the [contributing guidelines](https://github.com/paperless-ngx/paperless-ngx/blob/main/CONTRIBUTING.md).
- [x] If applicable, I have included testing coverage for new code in this PR, for [backend](https://docs.paperless-ngx.com/development/#testing) and / or [front-end](https://docs.paperless-ngx.com/development/#testing-and-code-style) changes.
- [ ] If applicable, I have tested my code for new features & regressions on both mobile & desktop devices, using the latest version of major browsers.
- [x] If applicable, I have checked that all tests pass, see [documentation](https://docs.paperless-ngx.com/development/#back-end-development).
- [x] I have run all `pre-commit` hooks, see [documentation](https://docs.paperless-ngx.com/development/#code-formatting-with-pre-commit-hooks).
- [ ] I have made corresponding changes to the documentation as needed.
- [x] I have checked my modifications for any breaking changes.


Problem statement:
[BUG] Redis credentials displayed on the frontend
### Description

Hey everyone, after visiting the brand new System Status modal I was amazed how awesome this new feature is. Thank you for adding this to paperless! But while taking a closer look, I felt some anxiety because suddenly I was looking at my redis credentials. In plain text. And any other user I've given admin status would see it too. 
This was certainly by mistake, which is why I am reporting this as a bug.

This is a rather serious issue for me since I am also hosting instances for other people using the same redis instance. They are not supposed to see my redis credentials. Nobody is supposed to.

### Steps to reproduce

Open the new System Status modal. There are two ways to display the credentials: either by hovering over the icon for the redis status, or by clicking "copy" in the modal footer and then pasting the copied json somewhere. 

### Webserver logs

```bash
not applicable
```


### Browser logs

_No response_

### Paperless-ngx version

2.6.2

### Host OS

Archlinux

### Installation method

Docker - official image

### Browser

_No response_

### Configuration changes

_No response_

### Other

_No response_

### Please confirm the following

- [X] I believe this issue is a bug that affects all users of Paperless-ngx, not something specific to my installation.
- [X] I have already searched for relevant existing issues and discussions before opening this report.
- [X] I have updated the title field above with a concise description.

Hints from issue discussion:
Im happy to entertain changing this, but this is why it requires admin permissions.

Commit message: Change: remove credentials from redis url in system status

Patch to review:
diff --git a/src/documents/tests/test_api_status.py b/src/documents/tests/test_api_status.py
index 1f8940ea9bf..eb35974a642 100644
--- a/src/documents/tests/test_api_status.py
+++ b/src/documents/tests/test_api_status.py
@@ -46,7 +46,7 @@ def test_system_status(self):
         self.assertEqual(response.data["database"]["status"], "OK")
         self.assertIsNone(response.data["database"]["error"])
         self.assertIsNotNone(response.data["database"]["migration_status"])
-        self.assertEqual(response.data["tasks"]["redis_url"], "redis://localhost:6379")
+        self.assertEqual(response.data["tasks"]["redis_url"], "localhost:6379")
         self.assertEqual(response.data["tasks"]["redis_status"], "ERROR")
         self.assertIsNotNone(response.data["tasks"]["redis_error"])
 
@@ -100,6 +100,21 @@ def test_system_status_redis_ping(self, mock_ping):
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         self.assertEqual(response.data["tasks"]["redis_status"], "OK")
 
+    def test_system_status_redis_no_credentials(self):
+        """
+        GIVEN:
+            - Redis URL with credentials
+        WHEN:
+            - The user requests the system status
+        THEN:
+            - The response contains the redis URL but no credentials
+        """
+        with override_settings(_CELERY_REDIS_URL="redis://:password@localhost:6379/0"):
+            self.client.force_login(self.user)
+            response = self.client.get(self.ENDPOINT)
+            self.assertEqual(response.status_code, status.HTTP_200_OK)
+            self.assertEqual(response.data["tasks"]["redis_url"], "localhost:6379")
+
     @mock.patch("celery.app.control.Inspect.ping")
     def test_system_status_celery_ping(self, mock_ping):
         """
diff --git a/src/documents/views.py b/src/documents/views.py
index 99db0a5d8a2..6d1d0a7c957 100644
--- a/src/documents/views.py
+++ b/src/documents/views.py
@@ -12,6 +12,7 @@
 from time import mktime
 from unicodedata import normalize
 from urllib.parse import quote
+from urllib.parse import urlparse
 
 import pathvalidate
 from django.apps import apps
@@ -1617,6 +1618,7 @@ def get(self, request, format=None):
         media_stats = os.statvfs(settings.MEDIA_ROOT)
 
         redis_url = settings._CHANNELS_REDIS_URL
+        redis_url_parsed = urlparse(settings._CHANNELS_REDIS_URL)
         redis_error = None
         with Redis.from_url(url=redis_url) as client:
             try:
@@ -1728,7 +1730,7 @@ def get(self, request, format=None):
                     },
                 },
                 "tasks": {
-                    "redis_url": redis_url,
+                    "redis_url": f"{redis_url_parsed.hostname}:{redis_url_parsed.port}",
                     "redis_status": redis_status,
                     "redis_error": redis_error,
                     "celery_status": celery_active,


Code review comments:
diff_hunk: @@ -1728,7 +1730,7 @@ def get(self, request, format=None):
                     },
                 },
                 "tasks": {
-                    "redis_url": redis_url,
+                    "redis_url": f"{redis_url_parsed.hostname}:{redis_url_parsed.port}",
path: src/documents/views.py
text: @user1:
For a socket connection, the hostname and port will be empty.

Maybe something like `{.scheme}:{.hostname or .path}`.  That wouldn't include the port though, which is tricky as there may be an extra colon for socket schemes, followed by an empty string.

@author:
Ah thanks, let me know if youre good with the change, should handle that better.