[Fix] 500 error by enforcing valid UUID patterns in admin URLs #682#978
[Fix] 500 error by enforcing valid UUID patterns in admin URLs #682#978Unnati-Gupta24 wants to merge 5 commits intoopenwisp:masterfrom
Conversation
|
Hi @Unnati-Gupta24 |
nemesifier
left a comment
nemesifier
left a comment
There was a problem hiding this comment.
Django has a built-in uuid path converter:
https://docs.djangoproject.com/en/5.1/topics/http/urls/#path-converters
There's no automated test in this PR. Please add a test which fails without the changes, effectively replicating the bug. This test will protect us from the chance that this bug may be reintroduced inadvertently again in the future by other contributors.
|
Ok thanks understood... |
|
I added a test case in admin.py, but the build is failing. I'm checking what might be causing this error. |
github-project-automation
bot
moved this to To do (general)
in OpenWISP Contributor's Board
| from .utils import CreateDeviceMixin | ||
|
|
||
| Device = load_model('config', 'Device') | ||
| Device = load_model("config", "Device") |
There was a problem hiding this comment.
Can you please switch from " " (double quotes)' ' (single quotes) to maintain consistency with the rest of the codebase?
github-project-automation
bot
moved this from To do (general)
to In progress
in OpenWISP Contributor's Board
| if not issubclass(self.model, AbstractVpn): | ||
| ctx['CONFIG_BACKEND_FIELD_SHOWN'] = app_settings.CONFIG_BACKEND_FIELD_SHOWN | ||
| if pk: | ||
| UUID_PATTERN = re.compile( |
There was a problem hiding this comment.
What's the reason for not using the built in uuid path converter provided by Django?
https://docs.djangoproject.com/en/4.2/topics/http/urls/#path-converters
| Group = load_model('openwisp_users', 'Group') | ||
|
|
||
|
|
||
| class TestUuidPattern(TestCase): |
There was a problem hiding this comment.
Is this the same exact test class added in openwisp_controller/config/tests/test_admin.py?
|
@Unnati-Gupta24 ping! |
|
Closing for lack of follow up. |
github-project-automation
bot
moved this from In progress
to Done
in OpenWISP Contributor's Board
4 participants
Checklist
Reference to Existing Issue
Closes #682 .
Description of Changes
This PR fixes a 500 Internal Server Error caused by an incorrect NoReverseMatch when certain admin URLs are accessed. The issue happened because the URL pattern was too loose, allowing invalid UUID formats.
What’s changed?