pusher / pusher-http-ruby

Ruby library for Pusher Channels HTTP API
https://pusher.com/channels
MIT License
664 stars 124 forks source link

Can't set encryption_master_key_base64 using the global configuration style. #176

Closed cjstewart88 closed 3 years ago

cjstewart88 commented 3 years ago

Hey there, I'm working on using encrypted pusher channels to a project and we're currently using the global configuration style of setup in an initializer:

Pusher.app_id = ENV['APP_ID']
Pusher.key    = ENV['PUSHER_KEY']
Pusher.secret = ENV['PUSHER_SECRET']
Pusher.logger = Rails.logger

When trying to add:

Pusher.encryption_master_key_base64 = ENV['ENCRYPTION_MASTER_KEY_BASE64']

I get the following error:

NoMethodError:
  undefined method `encryption_master_key_base64=' for Pusher:Module
# ./config/initializers/pusher.rb:4:in `<top (required)>'
# ./config/environment.rb:5:in `<top (required)>'
# ./spec/rails_helper.rb:18:in `require'
# ./spec/rails_helper.rb:18:in `<top (required)>'
# ./spec/services/pusher_events/encrypted_event_test_spec.rb:1:in `require'
# ./spec/services/pusher_events/encrypted_event_test_spec.rb:1:in `<top (required)>'

After digging around in the source, it looks like the below line doesn't delegate the setting of encryption_master_key_base64: https://github.com/pusher/pusher-http-ruby/blob/b81f2973092ea43393c706984fdabb53971c8825/lib/pusher.rb#L31 Was this intentional or an oversight? If an oversight, I don't mind opening a PR!

gileswells commented 3 years ago

Your logic seems sound to me though I'd also expect it to cover line 30 as well. Also in the docs the other libraries allow this setting in the global config so I don't see a reason why it shouldn't be that way here.

If you're able to put in a PR for this change I'll submit it to the engineers to review for inclusion.

cjstewart88 commented 3 years ago

FWIW a co-worker pointed out work around for this is to just reference the default_client, ie:

Pusher.default_client.encryption_master_key_base64 = ENV['ENCRYPTION_MASTER_KEY_BASE64']
elverkilde commented 3 years ago

thank you for pointing this out and providing a fix @cjstewart88 -- your fix is now merged and released as version 2.0.2.