From 19db557f514d414ee82be5dfa57a54c113e01dd8 Mon Sep 17 00:00:00 2001 From: Joel Van Horn Date: Tue, 12 Sep 2017 23:46:12 -0400 Subject: [PATCH 1/4] Refactored for clarity, removed unneeded configurations, and expose more `info` --- README.md | 11 ++++-- lib/omniauth/strategies/intercom.rb | 61 +++++++++++++---------------- 2 files changed, 35 insertions(+), 37 deletions(-) diff --git a/README.md b/README.md index 48b266d..0444d7b 100644 --- a/README.md +++ b/README.md @@ -49,8 +49,11 @@ Here's an example *Auth Hash* available in `request.env['omniauth.auth']`: :provider => 'intercom', :uid => '342324', :info => { + :name => 'Kevin Antoine', :email => 'kevin.antoine@intercom.io', - :name => 'Kevin Antoine' + :verified => true, + :image => 'https://static.intercomassets.com/avatars/343616/square_128/me.jpg?1454165491', + :time_zone => 'Dublin' }, :credentials => { :token => 'dG9rOmNdrWt0ZjtgzzE0MDdfNGM5YVe4MzsmXzFmOGd2MDhiMfJmYTrxOtA=', # OAuth 2.0 access_token, which you may wish to store @@ -67,11 +70,11 @@ Here's an example *Auth Hash* available in `request.env['omniauth.auth']`: :id_code => 'abc123', # Company app_id :type => 'app', :secure => true, # Secure mode enabled for this app - :timezone => "Dublin", - :name => "ProjectMap" + :timezone => 'Dublin', + :name => 'ProjectMap' }, :avatar => { - :image_url => "https://static.intercomassets.com/avatars/343616/square_128/me.jpg?1454165491" + :image_url => 'https://static.intercomassets.com/avatars/343616/square_128/me.jpg?1454165491' } } } diff --git a/lib/omniauth/strategies/intercom.rb b/lib/omniauth/strategies/intercom.rb index 97e34ee..d3b451c 100644 --- a/lib/omniauth/strategies/intercom.rb +++ b/lib/omniauth/strategies/intercom.rb @@ -7,9 +7,9 @@ class Intercom < OmniAuth::Strategies::OAuth2 option :name, 'intercom' option :client_options, { - :site => 'https://api.intercom.io', - :authorize_url => 'https://app.intercom.com/oauth', - :token_url => 'https://api.intercom.io/auth/eagle/token' + site: 'https://api.intercom.io', + token_url: 'https://api.intercom.io/auth/eagle/token', + authorize_url: 'https://app.intercom.com/oauth' } option :verify_email, true @@ -17,61 +17,56 @@ class Intercom < OmniAuth::Strategies::OAuth2 uid { raw_info['id'] } info do - { - :name => raw_info['name'], - :email => raw_info['email'], - }.tap do |info| - avatar = raw_info['avatar'] && raw_info['avatar']['image_url'] + return {} if raw_info.empty? - info[:image] = avatar if avatar - end + { + name: raw_info['name'], + email: raw_info['email'], + verfied: raw_info['email_verified'], + image: raw_info['avatar']['image_url'], + time_zone: raw_info['app']['timezone'] + } end extra do { - :raw_info => raw_info + raw_info: raw_info } end def raw_info - accept_headers - access_token.options[:mode] = :body - @raw_info ||= begin - parsed = access_token.get('/me').parsed - if options.verify_email && parsed['email_verified'] != true - return {} + headers = { 'Accept' => 'application/vnd.intercom.3+json' } + + @raw_info ||= access_token.get('/me', headers: headers).parsed.tap do |hash| + if options.verify_email && hash['email_verified'] != true + hash.clear end - parsed end end def request_phase - prepopulate_signup_fields_form if request.params.fetch('signup', false) + prepare_request_phase super end protected - def accept_headers - access_token.client.connection.headers['Authorization'] = access_token.client.connection.basic_auth(access_token.token, '') - access_token.client.connection.headers['Accept'] = "application/vnd.intercom.3+json" - access_token.client.connection.headers['User-Agent'] = "omniauth-intercom/#{::OmniAuth::Intercom::VERSION}" - end + def prepare_request_phase + return unless request.params['signup'] - def prepopulate_signup_fields_form options.client_options[:authorize_url] += '/signup' - signup_hash = signup_fields_hash - options.client_options[:authorize_url] += '?' + signup_hash.map{|k,v| [CGI.escape(k.to_s), "=", CGI.escape(v.to_s)]}.map(&:join).join("&") unless signup_hash.empty? + authorize_url_params = build_authorize_url_params(request.params) + + return if authorize_url_params.empty? + + options.client_options[:authorize_url] += "?#{URI.encode_www_form(authorize_url_params)}" end - def signup_fields_hash - hash = {} - ['name', 'email', 'app_name'].each do |field_name| - hash[field_name] = request.params.fetch(field_name) if request.params.fetch(field_name, false) + def build_authorize_url_params(params) + %w[name email app_name].each_with_object({}) do |field_name, hash| + hash.merge(field_name => params[field_name]) if params[field_name] end - return hash end - end end end From 55c67ac4c146bafeb717db9d812ae98dd8a5026a Mon Sep 17 00:00:00 2001 From: Joel Van Horn Date: Tue, 12 Sep 2017 23:55:25 -0400 Subject: [PATCH 2/4] Renamed methods for clarity --- lib/omniauth/strategies/intercom.rb | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/lib/omniauth/strategies/intercom.rb b/lib/omniauth/strategies/intercom.rb index d3b451c..c7a37b8 100644 --- a/lib/omniauth/strategies/intercom.rb +++ b/lib/omniauth/strategies/intercom.rb @@ -36,7 +36,7 @@ class Intercom < OmniAuth::Strategies::OAuth2 def raw_info headers = { 'Accept' => 'application/vnd.intercom.3+json' } - + @raw_info ||= access_token.get('/me', headers: headers).parsed.tap do |hash| if options.verify_email && hash['email_verified'] != true hash.clear @@ -45,24 +45,23 @@ def raw_info end def request_phase - prepare_request_phase + prepare_request_phase_for_signup super end - protected + protected - def prepare_request_phase + def prepare_request_phase_for_signup return unless request.params['signup'] - options.client_options[:authorize_url] += '/signup' - authorize_url_params = build_authorize_url_params(request.params) - return if authorize_url_params.empty? + signup_params = build_signup_params(request.params) + return if signup_params.empty? - options.client_options[:authorize_url] += "?#{URI.encode_www_form(authorize_url_params)}" + options.client_options[:authorize_url] += "?#{URI.encode_www_form(signup_params)}" end - def build_authorize_url_params(params) + def build_signup_params(params) %w[name email app_name].each_with_object({}) do |field_name, hash| hash.merge(field_name => params[field_name]) if params[field_name] end From 002abcc54dad41ae1942460cf358252b819adafc Mon Sep 17 00:00:00 2001 From: Joel Van Horn Date: Wed, 13 Sep 2017 00:16:44 -0400 Subject: [PATCH 3/4] Satisfy tests --- lib/omniauth/strategies/intercom.rb | 14 ++++++++++---- spec/omniauth/strategies/intercom_spec.rb | 7 ++++--- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/lib/omniauth/strategies/intercom.rb b/lib/omniauth/strategies/intercom.rb index c7a37b8..b22fc99 100644 --- a/lib/omniauth/strategies/intercom.rb +++ b/lib/omniauth/strategies/intercom.rb @@ -17,14 +17,17 @@ class Intercom < OmniAuth::Strategies::OAuth2 uid { raw_info['id'] } info do - return {} if raw_info.empty? + next {} if raw_info.empty? + + avatar = raw_info.fetch('avatar', {}) + app = raw_info.fetch('app', {}) { name: raw_info['name'], email: raw_info['email'], verfied: raw_info['email_verified'], - image: raw_info['avatar']['image_url'], - time_zone: raw_info['app']['timezone'] + image: avatar['image_url'], + time_zone: app['timezone'] } end @@ -52,18 +55,21 @@ def request_phase protected def prepare_request_phase_for_signup + puts "request.params: #{request.params.inspect}" return unless request.params['signup'] options.client_options[:authorize_url] += '/signup' signup_params = build_signup_params(request.params) + puts "signup_params: #{signup_params.inspect}" return if signup_params.empty? options.client_options[:authorize_url] += "?#{URI.encode_www_form(signup_params)}" end def build_signup_params(params) + puts "params: #{params.inspect}" %w[name email app_name].each_with_object({}) do |field_name, hash| - hash.merge(field_name => params[field_name]) if params[field_name] + hash.merge!(field_name => params[field_name]) if params[field_name] end end end diff --git a/spec/omniauth/strategies/intercom_spec.rb b/spec/omniauth/strategies/intercom_spec.rb index 7a25e80..f3b81e4 100644 --- a/spec/omniauth/strategies/intercom_spec.rb +++ b/spec/omniauth/strategies/intercom_spec.rb @@ -2,6 +2,7 @@ describe OmniAuth::Strategies::Intercom do let(:access_token) { double('AccessToken', options: {}) } + let(:access_token_headers) { {headers: {"Accept" => "application/vnd.intercom.3+json"}} } let(:token) { 'some-token' } let(:client) { double('Client') } let(:connection) { double('Connection') } @@ -29,13 +30,13 @@ let(:response) { double('Response', :parsed => parsed_response) } before do - allow(access_token).to receive(:get).with('/me').and_return response + allow(access_token).to receive(:get).with('/me', access_token_headers).and_return response allow(response).to receive(:parsed).and_return parsed_response end context '#raw_info' do it 'request "me"' do - expect(access_token).to receive(:get).with('/me').and_return response + expect(access_token).to receive(:get).with('/me', access_token_headers).and_return response subject.raw_info end @@ -106,7 +107,7 @@ let(:response) { double('Response', :parsed => parsed_response) } before do - allow(access_token).to receive(:get).with('/me').and_return response + allow(access_token).to receive(:get).with('/me', access_token_headers).and_return response allow(response).to receive(:parsed).and_return parsed_response end From 91b43a8cb52744ec6c5189e0de589438fb4ea58f Mon Sep 17 00:00:00 2001 From: Joel Van Horn Date: Wed, 13 Sep 2017 22:17:29 -0400 Subject: [PATCH 4/4] Removed extraneous output --- lib/omniauth/strategies/intercom.rb | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/omniauth/strategies/intercom.rb b/lib/omniauth/strategies/intercom.rb index b22fc99..cb7d4b6 100644 --- a/lib/omniauth/strategies/intercom.rb +++ b/lib/omniauth/strategies/intercom.rb @@ -55,19 +55,16 @@ def request_phase protected def prepare_request_phase_for_signup - puts "request.params: #{request.params.inspect}" return unless request.params['signup'] options.client_options[:authorize_url] += '/signup' signup_params = build_signup_params(request.params) - puts "signup_params: #{signup_params.inspect}" return if signup_params.empty? options.client_options[:authorize_url] += "?#{URI.encode_www_form(signup_params)}" end def build_signup_params(params) - puts "params: #{params.inspect}" %w[name email app_name].each_with_object({}) do |field_name, hash| hash.merge!(field_name => params[field_name]) if params[field_name] end