Add option to skip signature verification #1635
Conversation
|
|
||
| /** | ||
| * Creates a new instance. | ||
| * | ||
| * @param signatureValidator LINE messaging API's signature validator | ||
| */ | ||
| public WebhookParser(SignatureValidator signatureValidator) { | ||
| public WebhookParser(SignatureValidator signatureValidator, |
There was a problem hiding this comment.
can you update javadoc(comment) to explain when we should use SkipSignatureVerificationSupplier?
|
|
||
| /** | ||
| * Creates a new instance. | ||
| * | ||
| * @param signatureValidator LINE messaging API's signature validator | ||
| */ | ||
| public WebhookParser(SignatureValidator signatureValidator) { | ||
| public WebhookParser(SignatureValidator signatureValidator, |
There was a problem hiding this comment.
Can you create another constructor that doesn't break the build, and keep current constructor? Alternatively, you could set a non-skipping SkipSignatureVerificationSupplier as a default value.
There was a problem hiding this comment.
In cases other than temporary migration for channel secret, it should often be specified as true.
Having it set to return true by default in line-bot-sdk-java, with the possibility for users to override it if they wish, seems more convenient. It might not be necessary to require users to specify it, if we set default value.
There was a problem hiding this comment.
Did you mean to say this?
In cases other than temporary migration for the channel secret, it should often be specified as false.
I have made the change to set the default to false. Please review it. b9c3430
| /** | ||
| * Special {@link BooleanSupplier} for Skip Signature Verification. | ||
| * | ||
| * <p>You can implement it to return whether to skip signature verification. |
There was a problem hiding this comment.
(add comment)
+ if true is passed, webhook signature verification is skipped. This may be helpful when you update channel secret and you want to skip the verification temporarily.
| new MockSkipSignatureVerificationSupplier(); | ||
|
|
||
| static class MockSkipSignatureVerificationSupplier | ||
| implements SkipSignatureVerificationSupplier { | ||
| @Override | ||
| public boolean getAsBoolean() { | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
can we use FixedSkipSignatureVerificationSupplier.of(false) instead of mock?
|
|
||
| /** | ||
| * Creates a new instance. | ||
| * | ||
| * @param signatureValidator LINE messaging API's signature validator | ||
| */ | ||
| public WebhookParser(SignatureValidator signatureValidator) { | ||
| public WebhookParser(SignatureValidator signatureValidator, |
Changes
Motivation
The signature returned with webhooks is calculated using a single channel secret. If the bot owner changes their channel secret, the signature for webhooks starts being calculated using the new channel secret. To avoid signature verification failures, the bot owner must update the channel secret on their server, which is used for signature verification. However, if there is a timing mismatch in the update—and such a mismatch is almost unavoidable—verification will fail during that period.
In such cases, having an option to skip signature verification for webhooks would be a convenient way to avoid these issues.