Permission check for Alertmanager#595
Merged
vincent-olivert-riera merged 2 commits intoline:masterfrom Jun 3, 2025
Merged
Conversation
hoangpn
changed the title
vincent-olivert-riera
requested changes
Jun 3, 2025
Contributor
vincent-olivert-riera
left a comment
vincent-olivert-riera
left a comment
There was a problem hiding this comment.
I have finished my review. This PR is a nice improvement in terms of security. Overall looks good, but I have made some requests. Please have a look.
vincent-olivert-riera
previously approved these changes
Jun 3, 2025
Contributor
@hoangpn , the other PR has been merged. Could you please do this? Thanks in advance. |
Introduced the "PromgenModelPermissions" class to handle custom permission checks of a user for a Django REST Framework API. - Ensures that users are authenticated before checking permissions. - Validates that "permission_required" is required and iterable. - Supports both "any_perm" and "all_perms" logic for permission evaluation. This change enhances flexibility and security in permission handling for views.
hoangpn
force-pushed
the
feature/permission_check_for_alertmanager
branch
from
19f34c7 to
8b0d658
Compare
Currently, the AlertReceiver API can be called without authentication or authorization, even though it should only be called from internal services like Alertmanager. Therefore, we have added logic to require authentication and the corresponding permission "alert_trigger" for this API. To set up AlertManager, please follow the detailed instructions in the docker/alertmanager.yml file.
hoangpn
force-pushed
the
feature/permission_check_for_alertmanager
branch
from
8b0d658 to
0b66070
Compare
Contributor
Author
|
I've force pushed because I found a misleading new line in test_rest.py 🙇 |
Contributor
Author
|
@vincent-olivert-riera |
vincent-olivert-riera
approved these changes
Jun 3, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently, the AlertReceiver API can be called without authentication or authorization, even though it should only be called from internal services like Alertmanager. Therefore, we have added logic to require authentication and the corresponding permission "alert_trigger" for this API.
Before adding this logic, I had to create a custom DRF's permission class to handle checking specific permission.
I also added a detailed instruction for setting Alertmanager in the docker/alertmanager.yml file.