-
Notifications
You must be signed in to change notification settings - Fork 15
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
HMAC RequestValidator and GitHub Actions Setup #80
Conversation
|
||
return valid_signature_without_port or valid_signature_with_port | ||
|
||
return self.validate_with_compatibility(uri, params, signature) |
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.
Shouldn't this be a part of if isinstance(params, str)
condition? Considering the implementation of this feature in JS SDK.
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.
Good catch! fixed
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.
Correct me if I am wrong. This implementation is gonna call the compatibility_validator.validate
even when the params
is not an instance of a string. But the JS SDK throws an error if it is not a string.
I don't think it's a big deal but I am just highlighting it in terms of consistency across our SDKs.
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.
Yes, when it's a string there is a chance that is a SWML callback that the compatibility_validator
can not handle. But for the the other cases it's should be able to handle.
The RequestValidator class helps to validate Signalwire callback requests.
Added unit tests to confirm both LAML and SWML requests.
Also changes CI from Drone to Github Actions