-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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 Plugin #549
HMAC Plugin #549
Conversation
clock skew check
return username, signature, algorithm | ||
end | ||
|
||
local function validate_signature(request, secret, signature, algorithm, defaultClockSkew) |
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.
doesn't look like we're using the algorithm
variable in this function. should it be used in the -- validate signature
section or can it be removed as an argument?
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.
@harlow for now only sha1 would be supported and would be mentioned in the manual, here argument is optional field and any value passed by user will be discarded.
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.
Makes sense 👍
|
||
function _M.execute(conf) | ||
-- If both headers are missing, return 401 | ||
if not (ngx.req.get_headers()[AUTHORIZATION] or ngx.req.get_headers()[PROXY_AUTHORIZATION]) then |
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.
You are calling twice ngx.req.get_headers()
(and once in validate_signature
and another time in retrieve_hmac_fields
). This is an expansive operation and the result should be cached.
Also you have local ngx_set_header = ngx.req.set_header
but not the equivalent for get_headers()
.
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.
sorry missed it, updating it.
HMAC Plugin