-
Notifications
You must be signed in to change notification settings - Fork 90
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(backend): bypass sig validation #741
Conversation
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 could add tests that bypassSignatureValidation
:
- works with invalid signature
- doesn't work with invalid token
@@ -27,7 +27,7 @@ export function createAuthMiddleware({ | |||
const token = parts[1] | |||
if ( | |||
process.env.NODE_ENV !== 'production' && | |||
token === config.devAccessToken | |||
(token === config.devAccessToken || config.bypassSignatureValidation) |
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 the check should be moved below to wrap:
rafiki/packages/backend/src/open_payments/auth/middleware.ts
Lines 48 to 55 in e796ded
try { | |
if (!(await verifySigAndChallenge(grant.key.jwk, ctx))) { | |
ctx.throw(401, 'Invalid signature') | |
} | |
} catch (e) { | |
ctx.status = 401 | |
ctx.throw(401, `Invalid signature`) | |
} |
Co-authored-by: Brandon Wilson <[email protected]>
Co-authored-by: Brandon Wilson <[email protected]>
try { | ||
if (!(await verifySigAndChallenge(grant.key.jwk, ctx))) { | ||
ctx.throw(401, 'Invalid signature') | ||
if (!config.bypassSignatureValidation) { |
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.
Log might be handy when signature validation is being bypassed, could also do an early return to avoid nesting
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.
could also do an early return to avoid nesting
I think we do still want the ctx.grant = grant
below
Changes proposed in this pull request
Context
Checklist
fixes #number