-
Notifications
You must be signed in to change notification settings - Fork 74
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
feat(auth): secure admin api with hmac signatures #2709
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for brilliant-pasca-3e80ec ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
3127c8b
to
b0aba23
Compare
7ea0902
to
5bd359b
Compare
f75f811
to
cc173b6
Compare
|
||
async function canApiSignatureBeProcessed( | ||
signature: string, | ||
ctx: Context, |
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 would be good to type this Context such that we get the types for the services like redis
and logger
.
const key = `signature:${signature}` | ||
const op = redis.multi() | ||
op.set(key, signature) | ||
op.expire(key, 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.
should the expiry take in a num as the second argument?
}, | ||
'time differential' | ||
) | ||
if (currentTime - signatureTime > ttlMilliseconds) return false |
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 we should add a test for this as well
Changes proposed in this pull request
Context
Closes #2704.
Checklist
fixes #number