Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 24 additions & 5 deletions lib/line/bot/v2/webhook_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,20 @@ module V2
class WebhookParser
class InvalidSignatureError < StandardError; end

def initialize(channel_secret:)
# Initialize webhook parser
#
# @param channel_secret [String]
# The channel secret used for signature verification.
# @param skip_signature_verification [() -> bool, nil]
# A callable object with type `() -> bool` that determines whether to skip
# webhook signature verification. Signature verification is skipped if and
# only if this callable is provided and returns `true`.
# This can be useful in scenarios such as when you're in the process of
# updating the channel secret and need to temporarily bypass verification
# to avoid disruptions.
def initialize(channel_secret:, skip_signature_verification: nil)
Comment on lines +18 to +25
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better if skip_signature_verification accepts not only Procs but also boolean values. It is possible to pass Procs like -> { true } or -> { false } but just checking boolean (without calling) would be more efficient.

for example:

def should_skip_verification?
  if @skip_signature_verification.respond_to?(:call)
    @skip_signature_verification.call
  else
    @skip_signature_verification
  end
end

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much for your suggestion. Allowing boolean values could certainly be convenient in some cases. However, having both ()-> bool and bool in the same field would increase type complexity.

This feature is primarily intended for use cases where the decision to skip signature verification may change dynamically—for example, when controlled by environment variables.

Since the same behavior can be achieved by passing a function that simply returns a fixed true or false, we have decided not to adopt direct boolean support at this time.

@channel_secret = channel_secret
@skip_signature_verification = skip_signature_verification
end

# Parse events from the raw request body and validate the signature.
Expand All @@ -31,7 +43,10 @@ def initialize(channel_secret:)
#
# @example Sinatra usage
# def parser
# @parser ||= Line::Bot::V2::WebhookParser.new(channel_secret: ENV.fetch("LINE_CHANNEL_SECRET"))
# @parser ||= Line::Bot::V2::WebhookParser.new(
# channel_secret: ENV.fetch("LINE_CHANNEL_SECRET"),
# skip_signature_verification: -> { ENV['SKIP_SIGNATURE_VERIFICATION'] == 'true' }
# )
# end
#
# post '/callback' do
Expand All @@ -54,7 +69,11 @@ def initialize(channel_secret:)
# "OK"
# end
def parse(body:, signature:)
raise InvalidSignatureError.new("Invalid signature: #{signature}") unless verify_signature(body: body, signature: signature)
should_skip = @skip_signature_verification&.call || false

unless should_skip == true || verify_signature(body: body, signature: signature)
raise InvalidSignatureError.new("Invalid signature: #{signature}")
end

data = JSON.parse(body.chomp, symbolize_names: true)
data = Line::Bot::V2::Utils.deep_underscore(data)
Expand All @@ -66,14 +85,14 @@ def parse(body:, signature:)
end
end

private

def verify_signature(body:, signature:)
hash = OpenSSL::HMAC.digest(OpenSSL::Digest.new('SHA256'), @channel_secret, body)
expected = Base64.strict_encode64(hash)
variable_secure_compare(signature, expected)
end

private

# To avoid timing attacks
def variable_secure_compare(a, b)
secure_compare(::Digest::SHA256.hexdigest(a), ::Digest::SHA256.hexdigest(b))
Expand Down
7 changes: 4 additions & 3 deletions sig/line/bot/v2/webhook_parser.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,19 @@ module Line
class InvalidSignatureError < ::StandardError
end
@channel_secret: String
@skip_signature_verification: (^() -> bool) | nil

def initialize: (channel_secret: String) -> void
def initialize: (channel_secret: String, ?skip_signature_verification: (^() -> bool) | nil) -> void

def parse: (
body: String,
signature: String
) -> Array[Webhook::Event]

def verify_signature: (body: String, signature: String) -> bool

private

def verify_signature: (body: String, signature: String) -> bool

def variable_secure_compare: (String, String) -> bool

def secure_compare: (String, String) -> bool
Expand Down
75 changes: 75 additions & 0 deletions spec/line/bot/v2/webhook_parser_skip_verification_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
require 'spec_helper'

describe Line::Bot::V2::WebhookParser do
let(:channel_secret) { 'dummy_channel_secret' }
let(:signature) { 'invalid_signature' }
let(:webhook) do
<<~JSON
{
"destination": "xxxxxxxxxx",
"events": [
{
"type": "message",
"message": {
"type": "text",
"id": "123456789",
"quoteToken": "q3Plxr4AgKd...",
"text": "Hello, world"
},
"timestamp": 1462629479859,
"source": {
"type": "user",
"userId": "U4af4980629..."
},
"webhookEventId": "01FZ74A0TDDPYRVKNK77XKC3ZR",
"deliveryContext": {
"isRedelivery": false
},
"replyToken": "fbf94e269485410da6b7e3a5e33283e8",
"mode": "active"
}
]
}
JSON
end

describe '#parse with skip_signature_verification' do
context 'when skip_signature_verification is not provided' do
let(:parser) { Line::Bot::V2::WebhookParser.new(channel_secret: channel_secret) }

it 'verifies the signature' do
expect(parser).to receive(:verify_signature).and_return(false)
expect { parser.parse(body: webhook, signature: signature) }.to raise_error(Line::Bot::V2::WebhookParser::InvalidSignatureError)
end
end

context 'when skip_signature_verification returns false' do
let(:parser) { Line::Bot::V2::WebhookParser.new(channel_secret: channel_secret, skip_signature_verification: -> { false }) }

it 'verifies the signature' do
expect(parser).to receive(:verify_signature).and_return(false)
expect { parser.parse(body: webhook, signature: signature) }.to raise_error(Line::Bot::V2::WebhookParser::InvalidSignatureError)
end
end

context 'when skip_signature_verification returns true' do
let(:parser) { Line::Bot::V2::WebhookParser.new(channel_secret: channel_secret, skip_signature_verification: -> { true }) }

it 'skips signature verification and parses the webhook' do
expect(parser).not_to receive(:verify_signature)
events = parser.parse(body: webhook, signature: signature)
expect(events).not_to be_empty
expect(events.first).to be_a(Line::Bot::V2::Webhook::MessageEvent)
end
end

context 'when skip_signature_verification is nil' do
let(:parser) { Line::Bot::V2::WebhookParser.new(channel_secret: channel_secret, skip_signature_verification: nil) }

it 'verifies the signature' do
expect(parser).to receive(:verify_signature).and_return(false)
expect { parser.parse(body: webhook, signature: signature) }.to raise_error(Line::Bot::V2::WebhookParser::InvalidSignatureError)
end
end
end
end