-
Notifications
You must be signed in to change notification settings - Fork 31
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
RHCLOUD-33514 Implement blocklist #374
RHCLOUD-33514 Implement blocklist #374
Conversation
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good. I added a few comments. I can pick this up if needed.
internal/common/utils/misc.go
Outdated
@@ -109,3 +109,16 @@ func LoadSchemas(cfg *viper.Viper, schemaNames []string) (schemas []*jsonschema. | |||
} | |||
return | |||
} | |||
|
|||
func IsOrgIdBlocklisted(cfg *viper.Viper, orgId string) bool { | |||
blocklistedOrgIds := strings.Split(cfg.GetString("blocklist.orgids"), ",") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is unfortunate that we have to retrieve the block/deny list from the viper object each time and split it into a list. I wonder if there is a different way to do this. 🤔
internal/validator/handler.go
Outdated
@@ -156,6 +156,10 @@ func (this *handler) validateRequest(request *messageModel.IngressValidationRequ | |||
return fmt.Errorf("Rejecting payload due to file size: %d", request.Size) | |||
} | |||
|
|||
if utils.IsOrgIdBlocklisted(cfg, request.OrgID) { | |||
return fmt.Errorf("Rejecting payload because the org_id is blocklisted: %s", request.OrgID) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this might cause us to trigger PlaybookDispatcherValidator alerts. We might need to move this call up into the main message processing function and avoid returning an error and just log (debug level...probably, maybe even trace level) that we rejected a payload due to the org-id being blocked.
@dehort Unfortunately, you or someone else from the new Integrations team will have to take over this task from me. I'm occupied with PlatEx Services right now. 😅 |
57628bb
to
8198b50
Compare
8198b50
to
9577f10
Compare
What?
Explain what the change is linking any relevant JIRAs or Issues.
When the
org_id
is in the blocklist:org_id
Why?
Consider what business or engineering goal does this PR achieves.
How?
Describe how the change is implemented. Any noteable new libaries, APIs, or features.
Testing
Did you add any tests for the change?
Anything Else?
Any other notes about the PR that would be useful for the reviewer.
Secure Coding Practices Checklist Link
Secure Coding Checklist