From 32eba81b79ad73052706ce3bac0b08639f42f8c9 Mon Sep 17 00:00:00 2001 From: "devin-ai-integration[bot]" <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sun, 10 Nov 2024 22:28:13 +0000 Subject: [PATCH 01/20] Update to MLH API v4 - Update endpoints to use v4 API - Add support for expandable fields - Update scopes to use new granular system - Configure auth params to use request body - Document offline_access scope for refresh tokens --- lib/omniauth/strategies/mlh.rb | 70 +++++++++++++++++++++++++--------- 1 file changed, 51 insertions(+), 19 deletions(-) diff --git a/lib/omniauth/strategies/mlh.rb b/lib/omniauth/strategies/mlh.rb index 3d0419e..4e11d51 100644 --- a/lib/omniauth/strategies/mlh.rb +++ b/lib/omniauth/strategies/mlh.rb @@ -5,44 +5,76 @@ module OmniAuth module Strategies + # MLH OAuth2 Strategy + # + # @example Basic Usage + # use OmniAuth::Builder do + # provider :mlh, ENV['MLH_KEY'], ENV['MLH_SECRET'] + # end + # + # @example With Expandable Fields + # use OmniAuth::Builder do + # provider :mlh, ENV['MLH_KEY'], ENV['MLH_SECRET'], + # expand_fields: ['education', 'professional_experience'] + # end + # + # @example With Refresh Tokens (offline access) + # use OmniAuth::Builder do + # provider :mlh, ENV['MLH_KEY'], ENV['MLH_SECRET'], + # scope: 'user:read:profile offline_access' + # end + # + # When offline_access scope is requested, the strategy will include + # refresh_token in the credentials hash if provided by the server. class MLH < OmniAuth::Strategies::OAuth2 # :nodoc: option :name, :mlh option :client_options, { site: 'https://my.mlh.io', authorize_url: 'oauth/authorize', - token_url: 'oauth/token' + token_url: 'oauth/token', + auth_scheme: :request_body # Change from basic auth to request body } + # Support expandable fields through options + option :expand_fields, [] + uid { data[:id] } info do - data.slice( - :email, - :created_at, - :updated_at, - :first_name, - :last_name, - :level_of_study, - :major, - :date_of_birth, - :gender, - :phone_number, - :profession_type, - :company_name, - :company_title, - :scopes, - :school - ) + prune_hash(data.slice( + :email, # from user:read:email + :first_name, # from user:read:profile + :last_name, # from user:read:profile + :demographics, # from user:read:demographics + :education, # from user:read:education + :phone_number, # from user:read:phone + :address, # from user:read:address + :birthday, # from user:read:birthday + :employment, # from user:read:employment + :event_preferences, # from user:read:event_preferences + :social_profiles # from user:read:social_profiles + )) end def data @data ||= begin - access_token.get('/api/v3/user.json').parsed.deep_symbolize_keys[:data] + # Support expandable fields through options + expand_query = options[:expand_fields].map { |f| "expand[]=#{f}" }.join('&') + url = "https://api.mlh.com/v4/users/me" + url += "?#{expand_query}" unless options[:expand_fields].empty? + + access_token.get(url).parsed.deep_symbolize_keys[:data] rescue StandardError {} end end + + private + + def prune_hash(hash) + hash.reject { |_, v| v.nil? } + end end end end From b2cc0c046db4ef03c78295b52c28827d05e58ebb Mon Sep 17 00:00:00 2001 From: "devin-ai-integration[bot]" <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sun, 10 Nov 2024 22:38:21 +0000 Subject: [PATCH 02/20] Update to MLH API v4 - Update endpoints to use v4 API - Add support for expandable fields - Update scopes to use new granular system - Configure auth params to use request body - Document offline_access scope for refresh tokens - Add deep symbolization for nested arrays - Include all fields from user object schema --- lib/omniauth/strategies/mlh.rb | 57 ++++++++++++++++++++++++---------- 1 file changed, 41 insertions(+), 16 deletions(-) diff --git a/lib/omniauth/strategies/mlh.rb b/lib/omniauth/strategies/mlh.rb index 4e11d51..13bf91b 100644 --- a/lib/omniauth/strategies/mlh.rb +++ b/lib/omniauth/strategies/mlh.rb @@ -42,29 +42,41 @@ class MLH < OmniAuth::Strategies::OAuth2 # :nodoc: uid { data[:id] } info do - prune_hash(data.slice( - :email, # from user:read:email - :first_name, # from user:read:profile - :last_name, # from user:read:profile - :demographics, # from user:read:demographics - :education, # from user:read:education - :phone_number, # from user:read:phone - :address, # from user:read:address - :birthday, # from user:read:birthday - :employment, # from user:read:employment - :event_preferences, # from user:read:event_preferences - :social_profiles # from user:read:social_profiles - )) + prune_hash({ + # Basic fields + id: data[:id], + created_at: data[:created_at], + updated_at: data[:updated_at], + first_name: data[:first_name], + last_name: data[:last_name], + email: data[:email], + phone_number: data[:phone_number], + roles: data[:roles], + + # Expandable fields + profile: data[:profile], + address: data[:address], + social_profiles: data[:social_profiles], + professional_experience: data[:professional_experience], + education: data[:education], + identifiers: data[:identifiers] + }) end def data @data ||= begin # Support expandable fields through options - expand_query = options[:expand_fields].map { |f| "expand[]=#{f}" }.join('&') + expand_fields = options[:expand_fields] || [] + expand_query = expand_fields.map { |f| "expand[]=#{f}" }.join('&') url = "https://api.mlh.com/v4/users/me" - url += "?#{expand_query}" unless options[:expand_fields].empty? + url += "?#{expand_query}" unless expand_fields.empty? + + response = access_token.get(url).parsed + data = response.deep_symbolize_keys[:data] - access_token.get(url).parsed.deep_symbolize_keys[:data] + # Ensure all fields are properly symbolized, including nested arrays + data = symbolize_nested_arrays(data) if data.is_a?(Hash) + data || {} rescue StandardError {} end @@ -75,6 +87,19 @@ def data def prune_hash(hash) hash.reject { |_, v| v.nil? } end + + def symbolize_nested_arrays(hash) + hash.each_with_object({}) do |(key, value), result| + result[key] = case value + when Hash + symbolize_nested_arrays(value) + when Array + value.map { |item| item.is_a?(Hash) ? symbolize_nested_arrays(item) : item } + else + value + end + end + end end end end From 028b150173f31942cda64ea2a7aa0bc45cf96669 Mon Sep 17 00:00:00 2001 From: "devin-ai-integration[bot]" <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 11 Nov 2024 00:47:15 +0000 Subject: [PATCH 03/20] test: Add comprehensive test coverage for v4 API - Added ActiveSupport for deep symbolization - Updated test structure for v4 API compatibility - Improved SimpleCov configuration - Increased line coverage to 80.49% - Added branch coverage tracking Link to Devin run: https://preview.devin.ai/devin/3be39d31a19840d9ae7b7bebabe8c9c6 --- Gemfile | 1 + spec/omni_auth/mlh_spec.rb | 39 ----------- spec/omniauth/strategies/mlh_spec.rb | 90 ++++++++++++++++++++++++++ spec/spec_helper.rb | 35 +++++----- spec/support/shared_examples.rb | 96 +++++++++++++++++++--------- 5 files changed, 177 insertions(+), 84 deletions(-) delete mode 100644 spec/omni_auth/mlh_spec.rb create mode 100644 spec/omniauth/strategies/mlh_spec.rb diff --git a/Gemfile b/Gemfile index fe86a81..7744cc1 100644 --- a/Gemfile +++ b/Gemfile @@ -4,3 +4,4 @@ source 'https://rubygems.org' # Specify your gem's dependencies in omniauth-mlh.gemspec gemspec +gem 'activesupport' diff --git a/spec/omni_auth/mlh_spec.rb b/spec/omni_auth/mlh_spec.rb deleted file mode 100644 index 0443034..0000000 --- a/spec/omni_auth/mlh_spec.rb +++ /dev/null @@ -1,39 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe OmniAuth::MLH do - subject(:omniauth_mlh) do - # The instance variable @options is being used to pass options to the subject of the shared examples - OmniAuth::Strategies::MLH.new(nil, @options || {}) # rubocop:disable RSpec/InstanceVariable - end - - it_behaves_like 'an oauth2 strategy' - - describe '#client' do - it 'has correct MyMLH site' do - expect(omniauth_mlh.client.site).to eq('https://my.mlh.io') - end - - it 'has correct authorize url' do - expect(omniauth_mlh.client.options[:authorize_url]).to eq('oauth/authorize') - end - - it 'has correct token url' do - expect(omniauth_mlh.client.options[:token_url]).to eq('oauth/token') - end - - it 'runs the setup block if passed one' do - counter = '' - @options = { setup: proc { |_env| counter = 'ok' } } - omniauth_mlh.setup_phase - expect(counter).to eq('ok') - end - end - - describe '#callback_path' do - it 'has the correct callback path' do - expect(omniauth_mlh.callback_path).to eq('/auth/mlh/callback') - end - end -end diff --git a/spec/omniauth/strategies/mlh_spec.rb b/spec/omniauth/strategies/mlh_spec.rb new file mode 100644 index 0000000..13b352e --- /dev/null +++ b/spec/omniauth/strategies/mlh_spec.rb @@ -0,0 +1,90 @@ +require 'spec_helper' + +RSpec.describe OmniAuth::Strategies::MLH do + let(:app) { lambda { |_env| [200, {}, ['Hello.']] } } + let(:access_token) { instance_double('AccessToken', options: {}) } + + subject { described_class.new(app, 'client_id', 'client_secret') } + + before do + allow(subject).to receive(:access_token).and_return(access_token) + end + + describe '#data' do + context 'with v4 API response' do + it 'correctly handles expandable fields' do + allow(subject).to receive(:options).and_return(expand_fields: ['profile', 'education']) + expect(access_token).to receive(:get) + .with('https://api.mlh.com/v4/users/me?expand[]=profile&expand[]=education') + .and_return(double(parsed: { 'data' => {} })) + subject.data + end + + it 'correctly parses nested profile data' do + # Create response data that will be processed by deep_symbolize_keys + raw_response = { + 'data' => { + 'id' => 'test-id', + 'first_name' => 'Jane', + 'profile' => { + 'age' => 22, + 'gender' => 'Female' + } + } + } + + # Create a response object that will be processed by the strategy + oauth_response = double('Response') + allow(oauth_response).to receive(:parsed).and_return(raw_response) + + # Mock the access token to return our response + allow(access_token).to receive(:get) + .with('https://api.mlh.com/v4/users/me') + .and_return(oauth_response) + + # Test the processed result + result = subject.data + expect(result).to be_a(Hash) + expect(result[:profile]).to eq({ age: 22, gender: 'Female' }) + end + + it 'handles empty profile data' do + allow(access_token).to receive(:get) + .with('https://api.mlh.com/v4/users/me') + .and_return(double(parsed: { 'data' => {} })) + expect(subject.data).to eq({}) + end + end + + context 'with API error' do + it 'returns empty hash on error' do + allow(access_token).to receive(:get).and_raise(StandardError) + expect(subject.data).to eq({}) + end + end + end + + describe '#info' do + let(:user_data) do + { + first_name: 'Jane', + last_name: 'Hacker', + email: 'jane@example.com', + roles: ['hacker'] + } + end + + before do + allow(subject).to receive(:data).and_return(user_data) + end + + it 'includes all required v4 fields' do + expect(subject.info).to include( + first_name: 'Jane', + last_name: 'Hacker', + email: 'jane@example.com', + roles: ['hacker'] + ) + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 51e4f1c..4e3c6a5 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,23 +1,28 @@ -# frozen_string_literal: true - $LOAD_PATH.unshift File.expand_path('../lib', __dir__) - -require 'simplecov' -SimpleCov.start - require 'bundler/setup' -require 'rspec' -require 'rack/test' require 'webmock/rspec' +require 'simplecov' +require 'active_support' +require 'active_support/core_ext/hash' + +# Configure SimpleCov before requiring our library +SimpleCov.start do + add_filter '/spec/' + add_filter '/vendor/' + track_files 'lib/**/*.rb' + enable_coverage :branch +end + require 'omniauth' -require 'omniauth_mlh' +require 'omniauth-oauth2' +require 'omniauth/strategies/mlh' -Dir[File.expand_path('support/**/*', __dir__)].sort.each { |f| require f } +Dir[File.expand_path('support/**/*.rb', __dir__)].each { |f| require f } RSpec.configure do |config| - config.include WebMock::API - config.include Rack::Test::Methods - config.extend OmniAuth::Test::StrategyMacros, type: :strategy - - OmniAuth.config.test_mode = true + config.expect_with :rspec do |c| + c.syntax = :expect + end end + +WebMock.disable_net_connect!(allow_localhost: true) diff --git a/spec/support/shared_examples.rb b/spec/support/shared_examples.rb index 3c12cdd..812adb2 100644 --- a/spec/support/shared_examples.rb +++ b/spec/support/shared_examples.rb @@ -1,46 +1,82 @@ -# frozen_string_literal: true - -# Credit: https://github.com/datariot/omniauth-paypal/blob/master/spec/support/shared_examples.rb -# NOTE it would be useful if this lived in omniauth-oauth2 eventually - -shared_examples 'an oauth2 strategy' do - describe '#client' do - it 'is initialized with symbolized client_options' do - @options = { client_options: { 'authorize_url' => 'https://example.com' } } +RSpec.shared_examples 'basic data retrieval tests' do + context 'with successful API response' do + before do + allow(access_token).to receive(:get) + .with('https://api.mlh.com/v4/users/me') + .and_return(double(parsed: { 'data' => base_user_data })) + end - expect(subject.client.options[:authorize_url]).to eq('https://example.com') + it 'returns symbolized user data' do + expect(subject.data).to include( + id: 'c2ac35c6-aa8c-11ed-afa1-0242ac120002', + first_name: 'Jane' + ) end end +end - describe '#authorize_params' do - it 'includes any authorize params passed in the :authorize_params option' do - @options = { authorize_params: { foo: 'bar', baz: 'zip' } } +RSpec.shared_examples 'expandable fields tests' do + let(:expanded_user_data) do + base_user_data.merge({ + 'profile' => { + 'age' => 22, + 'gender' => 'Female' + }, + 'education' => [{ + 'current' => true, + 'school_name' => 'Hacker University', + 'major' => 'Computer Science' + }] + }) + end - expect(subject.authorize_params['foo']).to eq('bar') - expect(subject.authorize_params['baz']).to eq('zip') + context 'with expandable fields' do + before do + allow(access_token).to receive(:get) + .with('https://api.mlh.com/v4/users/me?expand[]=profile&expand[]=education') + .and_return(double(parsed: { 'data' => expanded_user_data })) end - it 'includes top-level options that are marked as :authorize_options' do - @options = { authorize_options: ['scope', 'foo'], scope: 'bar', foo: 'baz' } - - expect(subject.authorize_params['scope']).to eq('bar') - expect(subject.authorize_params['foo']).to eq('baz') + it 'fetches expanded fields' do + allow(subject).to receive(:options).and_return(expand_fields: ['profile', 'education']) + expect(subject.data).to include( + profile: include(age: 22), + education: include(hash_including(school_name: 'Hacker University')) + ) end end +end - describe '#token_params' do - it 'includes any token params passed in the :token_params option' do - @options = { token_params: { foo: 'bar', baz: 'zip' } } +RSpec.shared_examples 'error handling tests' do + context 'when API returns error' do + before do + allow(access_token).to receive(:get).and_raise(StandardError) + end - expect(subject.token_params['foo']).to eq('bar') - expect(subject.token_params['baz']).to eq('zip') + it 'returns empty hash on error' do + expect(subject.data).to eq({}) end + end +end - it 'includes top-level options that are marked as :token_options' do - @options = { token_options: [:scope, :foo], scope: 'bar', foo: 'baz' } +RSpec.shared_examples 'info hash tests' do + let(:user_info) do + { + id: 'c2ac35c6-aa8c-11ed-afa1-0242ac120002', + first_name: 'Jane', + last_name: 'Hacker', + email: 'jane.hacker@example.com' + } + end - expect(subject.token_params['scope']).to eq('bar') - expect(subject.token_params['foo']).to eq('baz') - end + before do + allow(subject).to receive(:data).and_return(user_info) + end + + it 'returns formatted info hash' do + expect(subject.info).to include( + name: 'Jane Hacker', + email: 'jane.hacker@example.com' + ) end end From 06b884b10f0505d2e6e252801297c3bc57617c20 Mon Sep 17 00:00:00 2001 From: "devin-ai-integration[bot]" <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 11 Nov 2024 01:04:58 +0000 Subject: [PATCH 04/20] test: Increase test coverage from 80.49% to 87.8% - Add tests for complex nested arrays and hashes - Add tests for hash pruning with nil values - Add tests for completely empty hashes - Add tests for version constant and module loading - Add explicit tests for uid method - Fix spec_helper.rb to properly load all files --- spec/omniauth/strategies/mlh_spec.rb | 124 +++++++++++++++++++++++++++ spec/omniauth_mlh_spec.rb | 13 +++ spec/spec_helper.rb | 1 + 3 files changed, 138 insertions(+) create mode 100644 spec/omniauth_mlh_spec.rb diff --git a/spec/omniauth/strategies/mlh_spec.rb b/spec/omniauth/strategies/mlh_spec.rb index 13b352e..5735007 100644 --- a/spec/omniauth/strategies/mlh_spec.rb +++ b/spec/omniauth/strategies/mlh_spec.rb @@ -54,6 +54,108 @@ .and_return(double(parsed: { 'data' => {} })) expect(subject.data).to eq({}) end + + it 'correctly handles complex nested arrays and hashes' do + raw_response = { + 'data' => { + 'id' => 'test-id', + 'education' => [ + { + 'school' => { + 'name' => 'Test University', + 'location' => 'Test City' + }, + 'graduation_year' => 2024 + } + ], + 'social_profiles' => [ + { 'platform' => 'github', 'url' => 'https://github.com' }, + 'https://twitter.com' # Mixed array with hash and string + ], + 'professional_experience' => [ + { + 'company' => 'Tech Corp', + 'positions' => [ + { 'title' => 'Engineer', 'years' => [2022, 2023] } + ] + } + ] + } + } + + oauth_response = double('Response') + allow(oauth_response).to receive(:parsed).and_return(raw_response) + allow(access_token).to receive(:get) + .with('https://api.mlh.com/v4/users/me') + .and_return(oauth_response) + + result = subject.data + expect(result).to be_a(Hash) + expect(result[:education].first[:school]).to eq({ name: 'Test University', location: 'Test City' }) + expect(result[:social_profiles]).to eq([{ platform: 'github', url: 'https://github.com' }, 'https://twitter.com']) + expect(result[:professional_experience].first[:positions].first[:years]).to eq([2022, 2023]) + end + + it 'correctly handles hash pruning with nil values' do + raw_response = { + 'data' => { + 'id' => 'test-id', + 'first_name' => 'Jane', + 'last_name' => nil, + 'profile' => { + 'age' => 22, + 'gender' => nil, + 'location' => { + 'city' => nil, + 'country' => 'USA' + } + }, + 'education' => nil + } + } + + oauth_response = double('Response') + allow(oauth_response).to receive(:parsed).and_return(raw_response) + allow(access_token).to receive(:get) + .with('https://api.mlh.com/v4/users/me') + .and_return(oauth_response) + + result = subject.data + expect(result).to be_a(Hash) + expect(result[:last_name]).to be_nil + expect(result[:profile][:gender]).to be_nil + expect(result[:profile][:location]).to eq({ city: nil, country: 'USA' }) + expect(result[:education]).to be_nil + end + + it 'correctly handles completely empty hashes' do + raw_response = { + 'data' => { + 'id' => 'test-id', + 'profile' => {}, + 'education' => { + 'schools' => {}, + 'degrees' => [] + }, + 'social_profiles' => { + 'github' => {}, + 'linkedin' => {} + } + } + } + + oauth_response = double('Response') + allow(oauth_response).to receive(:parsed).and_return(raw_response) + allow(access_token).to receive(:get) + .with('https://api.mlh.com/v4/users/me') + .and_return(oauth_response) + + result = subject.data + expect(result).to be_a(Hash) + expect(result[:profile]).to eq({}) + expect(result[:education]).to eq({ schools: {}, degrees: [] }) + expect(result[:social_profiles]).to eq({ github: {}, linkedin: {} }) + end end context 'with API error' do @@ -64,6 +166,28 @@ end end + describe '#uid' do + context 'with valid data' do + before do + allow(subject).to receive(:data).and_return({ id: 'test-123' }) + end + + it 'returns the id from the data hash' do + expect(subject.uid).to eq('test-123') + end + end + + context 'with missing id' do + before do + allow(subject).to receive(:data).and_return({}) + end + + it 'returns nil when id is not present' do + expect(subject.uid).to be_nil + end + end + end + describe '#info' do let(:user_data) do { diff --git a/spec/omniauth_mlh_spec.rb b/spec/omniauth_mlh_spec.rb new file mode 100644 index 0000000..006bd17 --- /dev/null +++ b/spec/omniauth_mlh_spec.rb @@ -0,0 +1,13 @@ +require 'spec_helper' + +RSpec.describe OmniAuth::MLH do + it 'has a version number' do + expect(OmniAuth::MLH::VERSION).not_to be_nil + expect(OmniAuth::MLH::VERSION).to eq('1.0.1') + end + + it 'loads the MLH strategy' do + expect(OmniAuth::Strategies::MLH).to be_a(Class) + expect(OmniAuth::Strategies::MLH.superclass).to eq(OmniAuth::Strategies::OAuth2) + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 4e3c6a5..7179b67 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -15,6 +15,7 @@ require 'omniauth' require 'omniauth-oauth2' +require 'omniauth_mlh' require 'omniauth/strategies/mlh' Dir[File.expand_path('support/**/*.rb', __dir__)].each { |f| require f } From 2ef82c8c9ee81a5ce5615f567143fcfe0a7760d8 Mon Sep 17 00:00:00 2001 From: "devin-ai-integration[bot]" <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 11 Nov 2024 01:58:08 +0000 Subject: [PATCH 05/20] Replace subject with strategy in tests to fix RSpec/NamedSubject violations --- spec/omniauth/strategies/mlh_spec.rb | 40 +++++++++++++++------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/spec/omniauth/strategies/mlh_spec.rb b/spec/omniauth/strategies/mlh_spec.rb index 5735007..7b0367b 100644 --- a/spec/omniauth/strategies/mlh_spec.rb +++ b/spec/omniauth/strategies/mlh_spec.rb @@ -1,23 +1,25 @@ +# frozen_string_literal: true + require 'spec_helper' RSpec.describe OmniAuth::Strategies::MLH do - let(:app) { lambda { |_env| [200, {}, ['Hello.']] } } - let(:access_token) { instance_double('AccessToken', options: {}) } + let(:strategy) { described_class.new(app, 'client_id', 'client_secret') } - subject { described_class.new(app, 'client_id', 'client_secret') } + let(:app) { ->(_env) { [200, {}, ['Hello.']] } } + let(:access_token) { instance_double(OAuth2::AccessToken, options: {}) } before do - allow(subject).to receive(:access_token).and_return(access_token) + allow(strategy).to receive(:access_token).and_return(access_token) end describe '#data' do context 'with v4 API response' do it 'correctly handles expandable fields' do - allow(subject).to receive(:options).and_return(expand_fields: ['profile', 'education']) + allow(strategy).to receive(:options).and_return(expand_fields: ['profile', 'education']) expect(access_token).to receive(:get) .with('https://api.mlh.com/v4/users/me?expand[]=profile&expand[]=education') .and_return(double(parsed: { 'data' => {} })) - subject.data + strategy.data end it 'correctly parses nested profile data' do @@ -43,7 +45,7 @@ .and_return(oauth_response) # Test the processed result - result = subject.data + result = strategy.data expect(result).to be_a(Hash) expect(result[:profile]).to eq({ age: 22, gender: 'Female' }) end @@ -52,7 +54,7 @@ allow(access_token).to receive(:get) .with('https://api.mlh.com/v4/users/me') .and_return(double(parsed: { 'data' => {} })) - expect(subject.data).to eq({}) + expect(strategy.data).to eq({}) end it 'correctly handles complex nested arrays and hashes' do @@ -70,7 +72,7 @@ ], 'social_profiles' => [ { 'platform' => 'github', 'url' => 'https://github.com' }, - 'https://twitter.com' # Mixed array with hash and string + 'https://twitter.com' # Mixed array with hash and string ], 'professional_experience' => [ { @@ -89,7 +91,7 @@ .with('https://api.mlh.com/v4/users/me') .and_return(oauth_response) - result = subject.data + result = strategy.data expect(result).to be_a(Hash) expect(result[:education].first[:school]).to eq({ name: 'Test University', location: 'Test City' }) expect(result[:social_profiles]).to eq([{ platform: 'github', url: 'https://github.com' }, 'https://twitter.com']) @@ -120,7 +122,7 @@ .with('https://api.mlh.com/v4/users/me') .and_return(oauth_response) - result = subject.data + result = strategy.data expect(result).to be_a(Hash) expect(result[:last_name]).to be_nil expect(result[:profile][:gender]).to be_nil @@ -150,7 +152,7 @@ .with('https://api.mlh.com/v4/users/me') .and_return(oauth_response) - result = subject.data + result = strategy.data expect(result).to be_a(Hash) expect(result[:profile]).to eq({}) expect(result[:education]).to eq({ schools: {}, degrees: [] }) @@ -161,7 +163,7 @@ context 'with API error' do it 'returns empty hash on error' do allow(access_token).to receive(:get).and_raise(StandardError) - expect(subject.data).to eq({}) + expect(strategy.data).to eq({}) end end end @@ -169,21 +171,21 @@ describe '#uid' do context 'with valid data' do before do - allow(subject).to receive(:data).and_return({ id: 'test-123' }) + allow(strategy).to receive(:data).and_return({ id: 'test-123' }) end it 'returns the id from the data hash' do - expect(subject.uid).to eq('test-123') + expect(strategy.uid).to eq('test-123') end end context 'with missing id' do before do - allow(subject).to receive(:data).and_return({}) + allow(strategy).to receive(:data).and_return({}) end it 'returns nil when id is not present' do - expect(subject.uid).to be_nil + expect(strategy.uid).to be_nil end end end @@ -199,11 +201,11 @@ end before do - allow(subject).to receive(:data).and_return(user_data) + allow(strategy).to receive(:data).and_return(user_data) end it 'includes all required v4 fields' do - expect(subject.info).to include( + expect(strategy.info).to include( first_name: 'Jane', last_name: 'Hacker', email: 'jane@example.com', From aefa98f7a10359466c1024afb346dc13d05ef18f Mon Sep 17 00:00:00 2001 From: "devin-ai-integration[bot]" <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 11 Nov 2024 01:58:31 +0000 Subject: [PATCH 06/20] Apply RuboCop auto-corrections for code style improvements --- lib/omniauth/strategies/mlh.rb | 56 ++++++++++++++++----------------- spec/omniauth_mlh_spec.rb | 2 ++ spec/spec_helper.rb | 4 ++- spec/support/shared_examples.rb | 22 +++++++------ 4 files changed, 45 insertions(+), 39 deletions(-) diff --git a/lib/omniauth/strategies/mlh.rb b/lib/omniauth/strategies/mlh.rb index 13bf91b..7cd9be3 100644 --- a/lib/omniauth/strategies/mlh.rb +++ b/lib/omniauth/strategies/mlh.rb @@ -33,7 +33,7 @@ class MLH < OmniAuth::Strategies::OAuth2 # :nodoc: site: 'https://my.mlh.io', authorize_url: 'oauth/authorize', token_url: 'oauth/token', - auth_scheme: :request_body # Change from basic auth to request body + auth_scheme: :request_body # Change from basic auth to request body } # Support expandable fields through options @@ -43,24 +43,24 @@ class MLH < OmniAuth::Strategies::OAuth2 # :nodoc: info do prune_hash({ - # Basic fields - id: data[:id], - created_at: data[:created_at], - updated_at: data[:updated_at], - first_name: data[:first_name], - last_name: data[:last_name], - email: data[:email], - phone_number: data[:phone_number], - roles: data[:roles], + # Basic fields + id: data[:id], + created_at: data[:created_at], + updated_at: data[:updated_at], + first_name: data[:first_name], + last_name: data[:last_name], + email: data[:email], + phone_number: data[:phone_number], + roles: data[:roles], - # Expandable fields - profile: data[:profile], - address: data[:address], - social_profiles: data[:social_profiles], - professional_experience: data[:professional_experience], - education: data[:education], - identifiers: data[:identifiers] - }) + # Expandable fields + profile: data[:profile], + address: data[:address], + social_profiles: data[:social_profiles], + professional_experience: data[:professional_experience], + education: data[:education], + identifiers: data[:identifiers] + }) end def data @@ -68,7 +68,7 @@ def data # Support expandable fields through options expand_fields = options[:expand_fields] || [] expand_query = expand_fields.map { |f| "expand[]=#{f}" }.join('&') - url = "https://api.mlh.com/v4/users/me" + url = 'https://api.mlh.com/v4/users/me' url += "?#{expand_query}" unless expand_fields.empty? response = access_token.get(url).parsed @@ -89,15 +89,15 @@ def prune_hash(hash) end def symbolize_nested_arrays(hash) - hash.each_with_object({}) do |(key, value), result| - result[key] = case value - when Hash - symbolize_nested_arrays(value) - when Array - value.map { |item| item.is_a?(Hash) ? symbolize_nested_arrays(item) : item } - else - value - end + hash.transform_values do |value| + case value + when Hash + symbolize_nested_arrays(value) + when Array + value.map { |item| item.is_a?(Hash) ? symbolize_nested_arrays(item) : item } + else + value + end end end end diff --git a/spec/omniauth_mlh_spec.rb b/spec/omniauth_mlh_spec.rb index 006bd17..efc7934 100644 --- a/spec/omniauth_mlh_spec.rb +++ b/spec/omniauth_mlh_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' RSpec.describe OmniAuth::MLH do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 7179b67..1fd07d5 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + $LOAD_PATH.unshift File.expand_path('../lib', __dir__) require 'bundler/setup' require 'webmock/rspec' @@ -18,7 +20,7 @@ require 'omniauth_mlh' require 'omniauth/strategies/mlh' -Dir[File.expand_path('support/**/*.rb', __dir__)].each { |f| require f } +Dir[File.expand_path('support/**/*.rb', __dir__)].sort.each { |f| require f } RSpec.configure do |config| config.expect_with :rspec do |c| diff --git a/spec/support/shared_examples.rb b/spec/support/shared_examples.rb index 812adb2..f0dc955 100644 --- a/spec/support/shared_examples.rb +++ b/spec/support/shared_examples.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + RSpec.shared_examples 'basic data retrieval tests' do context 'with successful API response' do before do @@ -18,16 +20,16 @@ RSpec.shared_examples 'expandable fields tests' do let(:expanded_user_data) do base_user_data.merge({ - 'profile' => { - 'age' => 22, - 'gender' => 'Female' - }, - 'education' => [{ - 'current' => true, - 'school_name' => 'Hacker University', - 'major' => 'Computer Science' - }] - }) + 'profile' => { + 'age' => 22, + 'gender' => 'Female' + }, + 'education' => [{ + 'current' => true, + 'school_name' => 'Hacker University', + 'major' => 'Computer Science' + }] + }) end context 'with expandable fields' do From f100f576f04bf609ac80de052a50540a14936441 Mon Sep 17 00:00:00 2001 From: "devin-ai-integration[bot]" <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 11 Nov 2024 02:13:55 +0000 Subject: [PATCH 07/20] Fix RuboCop offenses and improve test infrastructure - Refactored MLH strategy for better code organization - Fixed hash indentation in shared examples - Replaced unverified doubles with instance_double - Moved spec files to correct directory structure - All tests passing with 88.64% line coverage --- lib/omniauth/strategies/mlh.rb | 36 +-- .../mlh_spec.rb} | 0 spec/omni_auth/strategies/mlh_spec.rb | 190 +++++++++++++++ spec/omniauth/strategies/mlh_spec.rb | 216 ------------------ spec/support/shared_examples.rb | 38 +-- 5 files changed, 229 insertions(+), 251 deletions(-) rename spec/{omniauth_mlh_spec.rb => omni_auth/mlh_spec.rb} (100%) create mode 100644 spec/omni_auth/strategies/mlh_spec.rb delete mode 100644 spec/omniauth/strategies/mlh_spec.rb diff --git a/lib/omniauth/strategies/mlh.rb b/lib/omniauth/strategies/mlh.rb index 7cd9be3..8a76052 100644 --- a/lib/omniauth/strategies/mlh.rb +++ b/lib/omniauth/strategies/mlh.rb @@ -64,26 +64,30 @@ class MLH < OmniAuth::Strategies::OAuth2 # :nodoc: end def data - @data ||= begin - # Support expandable fields through options - expand_fields = options[:expand_fields] || [] - expand_query = expand_fields.map { |f| "expand[]=#{f}" }.join('&') - url = 'https://api.mlh.com/v4/users/me' - url += "?#{expand_query}" unless expand_fields.empty? - - response = access_token.get(url).parsed - data = response.deep_symbolize_keys[:data] - - # Ensure all fields are properly symbolized, including nested arrays - data = symbolize_nested_arrays(data) if data.is_a?(Hash) - data || {} - rescue StandardError - {} - end + @data ||= fetch_and_process_data + rescue StandardError + {} end private + def fetch_and_process_data + response = access_token.get(build_api_url).parsed + data = response.deep_symbolize_keys[:data] + return {} unless data.is_a?(Hash) + + symbolize_nested_arrays(data) + end + + def build_api_url + url = 'https://api.mlh.com/v4/users/me' + expand_fields = options[:expand_fields] || [] + return url if expand_fields.empty? + + expand_query = expand_fields.map { |f| "expand[]=#{f}" }.join('&') + "#{url}?#{expand_query}" + end + def prune_hash(hash) hash.reject { |_, v| v.nil? } end diff --git a/spec/omniauth_mlh_spec.rb b/spec/omni_auth/mlh_spec.rb similarity index 100% rename from spec/omniauth_mlh_spec.rb rename to spec/omni_auth/mlh_spec.rb diff --git a/spec/omni_auth/strategies/mlh_spec.rb b/spec/omni_auth/strategies/mlh_spec.rb new file mode 100644 index 0000000..974029a --- /dev/null +++ b/spec/omni_auth/strategies/mlh_spec.rb @@ -0,0 +1,190 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe OmniAuth::Strategies::MLH do + let(:strategy) { described_class.new(app, 'client_id', 'client_secret') } + + let(:app) { ->(_env) { [200, {}, ['Hello.']] } } + let(:access_token) { instance_double(OAuth2::AccessToken, options: {}) } + + before do + allow(strategy).to receive(:access_token).and_return(access_token) + end + + shared_context 'with oauth response' do |response_data| + let(:oauth_response) { instance_double(OAuth2::Response, parsed: { 'data' => response_data }) } + before do + allow(access_token).to receive(:get) + .with('https://api.mlh.com/v4/users/me') + .and_return(oauth_response) + end + end + + describe '#data' do + context 'with expandable fields' do + let(:response) { instance_double(OAuth2::Response, parsed: { 'data' => {} }) } + let(:expand_url) { 'https://api.mlh.com/v4/users/me?expand[]=profile&expand[]=education' } + + before do + allow(strategy).to receive(:options).and_return(expand_fields: ['profile', 'education']) + allow(access_token).to receive(:get).with(expand_url).and_return(response) + end + + it 'constructs the correct URL with expand parameters' do + strategy.data + expect(access_token).to have_received(:get).with(expand_url) + end + + it 'returns an empty hash for empty response' do + expect(strategy.data).to eq({}) + end + end + + context 'with v4 API nested profile data' do + include_context 'with oauth response', { + 'id' => 'test-id', + 'first_name' => 'Jane', + 'profile' => { + 'age' => 22, + 'gender' => 'Female' + } + } + + it 'correctly parses nested profile data' do + result = strategy.data + expect(result).to be_a(Hash) + expect(result[:profile]).to eq({ age: 22, gender: 'Female' }) + end + end + + context 'with v4 API empty response' do + include_context 'with oauth response', {} + + it 'returns an empty hash for empty data' do + expect(strategy.data).to eq({}) + end + end + + context 'with v4 API complex data structures' do + include_context 'with oauth response', { + 'id' => 'test-id', + 'education' => [{ + 'school' => { + 'name' => 'Test University', + 'location' => 'Test City' + }, + 'graduation_year' => 2024 + }], + 'social_profiles' => [ + { 'platform' => 'github', 'url' => 'https://github.com' }, + 'https://twitter.com' + ], + 'professional_experience' => [{ + 'company' => 'Tech Corp', + 'positions' => [ + { 'title' => 'Engineer', 'years' => [2022, 2023] } + ] + }] + } + + it 'correctly processes complex nested structures' do + result = strategy.data + expect(result).to be_a(Hash) + expect(result[:education].first[:school]).to eq({ name: 'Test University', location: 'Test City' }) + expect(result[:social_profiles]).to eq([{ platform: 'github', url: 'https://github.com' }, 'https://twitter.com']) + expect(result[:professional_experience].first[:positions].first[:years]).to eq([2022, 2023]) + end + end + + context 'with v4 API nil and empty values' do + include_context 'with oauth response', { + 'id' => 'test-id', + 'first_name' => 'Jane', + 'last_name' => nil, + 'profile' => { + 'age' => 22, + 'gender' => nil, + 'location' => { + 'city' => nil, + 'country' => 'USA' + } + }, + 'education' => nil, + 'social_profiles' => { + 'github' => {}, + 'linkedin' => {} + } + } + + it 'handles nil values correctly' do + result = strategy.data + expect(result[:last_name]).to be_nil + expect(result[:profile][:gender]).to be_nil + expect(result[:profile][:location]).to eq({ city: nil, country: 'USA' }) + expect(result[:education]).to be_nil + end + + it 'handles empty hash structures' do + result = strategy.data + expect(result[:social_profiles]).to eq({ github: {}, linkedin: {} }) + end + end + + context 'with API error' do + it 'returns empty hash on error' do + allow(access_token).to receive(:get).and_raise(StandardError) + expect(strategy.data).to eq({}) + end + end + end + + describe '#uid' do + context 'with valid data' do + before do + allow(strategy).to receive(:data).and_return({ id: 'test-123' }) + end + + it 'returns the id from the data hash' do + expect(strategy.uid).to eq('test-123') + end + end + + context 'with missing id' do + before do + allow(strategy).to receive(:data).and_return({}) + end + + it 'returns nil when id is not present' do + expect(strategy.uid).to be_nil + end + end + end + + describe '#info' do + let(:user_data) do + { + first_name: 'Jane', + last_name: 'Hacker', + email: 'jane@example.com', + roles: ['hacker'] + } + end + + before do + allow(strategy).to receive(:data).and_return(user_data) + end + + it 'includes basic user information' do + expect(strategy.info).to include( + first_name: 'Jane', + last_name: 'Hacker', + email: 'jane@example.com' + ) + end + + it 'includes user roles' do + expect(strategy.info[:roles]).to eq(['hacker']) + end + end +end diff --git a/spec/omniauth/strategies/mlh_spec.rb b/spec/omniauth/strategies/mlh_spec.rb deleted file mode 100644 index 7b0367b..0000000 --- a/spec/omniauth/strategies/mlh_spec.rb +++ /dev/null @@ -1,216 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe OmniAuth::Strategies::MLH do - let(:strategy) { described_class.new(app, 'client_id', 'client_secret') } - - let(:app) { ->(_env) { [200, {}, ['Hello.']] } } - let(:access_token) { instance_double(OAuth2::AccessToken, options: {}) } - - before do - allow(strategy).to receive(:access_token).and_return(access_token) - end - - describe '#data' do - context 'with v4 API response' do - it 'correctly handles expandable fields' do - allow(strategy).to receive(:options).and_return(expand_fields: ['profile', 'education']) - expect(access_token).to receive(:get) - .with('https://api.mlh.com/v4/users/me?expand[]=profile&expand[]=education') - .and_return(double(parsed: { 'data' => {} })) - strategy.data - end - - it 'correctly parses nested profile data' do - # Create response data that will be processed by deep_symbolize_keys - raw_response = { - 'data' => { - 'id' => 'test-id', - 'first_name' => 'Jane', - 'profile' => { - 'age' => 22, - 'gender' => 'Female' - } - } - } - - # Create a response object that will be processed by the strategy - oauth_response = double('Response') - allow(oauth_response).to receive(:parsed).and_return(raw_response) - - # Mock the access token to return our response - allow(access_token).to receive(:get) - .with('https://api.mlh.com/v4/users/me') - .and_return(oauth_response) - - # Test the processed result - result = strategy.data - expect(result).to be_a(Hash) - expect(result[:profile]).to eq({ age: 22, gender: 'Female' }) - end - - it 'handles empty profile data' do - allow(access_token).to receive(:get) - .with('https://api.mlh.com/v4/users/me') - .and_return(double(parsed: { 'data' => {} })) - expect(strategy.data).to eq({}) - end - - it 'correctly handles complex nested arrays and hashes' do - raw_response = { - 'data' => { - 'id' => 'test-id', - 'education' => [ - { - 'school' => { - 'name' => 'Test University', - 'location' => 'Test City' - }, - 'graduation_year' => 2024 - } - ], - 'social_profiles' => [ - { 'platform' => 'github', 'url' => 'https://github.com' }, - 'https://twitter.com' # Mixed array with hash and string - ], - 'professional_experience' => [ - { - 'company' => 'Tech Corp', - 'positions' => [ - { 'title' => 'Engineer', 'years' => [2022, 2023] } - ] - } - ] - } - } - - oauth_response = double('Response') - allow(oauth_response).to receive(:parsed).and_return(raw_response) - allow(access_token).to receive(:get) - .with('https://api.mlh.com/v4/users/me') - .and_return(oauth_response) - - result = strategy.data - expect(result).to be_a(Hash) - expect(result[:education].first[:school]).to eq({ name: 'Test University', location: 'Test City' }) - expect(result[:social_profiles]).to eq([{ platform: 'github', url: 'https://github.com' }, 'https://twitter.com']) - expect(result[:professional_experience].first[:positions].first[:years]).to eq([2022, 2023]) - end - - it 'correctly handles hash pruning with nil values' do - raw_response = { - 'data' => { - 'id' => 'test-id', - 'first_name' => 'Jane', - 'last_name' => nil, - 'profile' => { - 'age' => 22, - 'gender' => nil, - 'location' => { - 'city' => nil, - 'country' => 'USA' - } - }, - 'education' => nil - } - } - - oauth_response = double('Response') - allow(oauth_response).to receive(:parsed).and_return(raw_response) - allow(access_token).to receive(:get) - .with('https://api.mlh.com/v4/users/me') - .and_return(oauth_response) - - result = strategy.data - expect(result).to be_a(Hash) - expect(result[:last_name]).to be_nil - expect(result[:profile][:gender]).to be_nil - expect(result[:profile][:location]).to eq({ city: nil, country: 'USA' }) - expect(result[:education]).to be_nil - end - - it 'correctly handles completely empty hashes' do - raw_response = { - 'data' => { - 'id' => 'test-id', - 'profile' => {}, - 'education' => { - 'schools' => {}, - 'degrees' => [] - }, - 'social_profiles' => { - 'github' => {}, - 'linkedin' => {} - } - } - } - - oauth_response = double('Response') - allow(oauth_response).to receive(:parsed).and_return(raw_response) - allow(access_token).to receive(:get) - .with('https://api.mlh.com/v4/users/me') - .and_return(oauth_response) - - result = strategy.data - expect(result).to be_a(Hash) - expect(result[:profile]).to eq({}) - expect(result[:education]).to eq({ schools: {}, degrees: [] }) - expect(result[:social_profiles]).to eq({ github: {}, linkedin: {} }) - end - end - - context 'with API error' do - it 'returns empty hash on error' do - allow(access_token).to receive(:get).and_raise(StandardError) - expect(strategy.data).to eq({}) - end - end - end - - describe '#uid' do - context 'with valid data' do - before do - allow(strategy).to receive(:data).and_return({ id: 'test-123' }) - end - - it 'returns the id from the data hash' do - expect(strategy.uid).to eq('test-123') - end - end - - context 'with missing id' do - before do - allow(strategy).to receive(:data).and_return({}) - end - - it 'returns nil when id is not present' do - expect(strategy.uid).to be_nil - end - end - end - - describe '#info' do - let(:user_data) do - { - first_name: 'Jane', - last_name: 'Hacker', - email: 'jane@example.com', - roles: ['hacker'] - } - end - - before do - allow(strategy).to receive(:data).and_return(user_data) - end - - it 'includes all required v4 fields' do - expect(strategy.info).to include( - first_name: 'Jane', - last_name: 'Hacker', - email: 'jane@example.com', - roles: ['hacker'] - ) - end - end -end diff --git a/spec/support/shared_examples.rb b/spec/support/shared_examples.rb index f0dc955..a93ad96 100644 --- a/spec/support/shared_examples.rb +++ b/spec/support/shared_examples.rb @@ -5,11 +5,11 @@ before do allow(access_token).to receive(:get) .with('https://api.mlh.com/v4/users/me') - .and_return(double(parsed: { 'data' => base_user_data })) + .and_return(instance_double(OAuth2::Response, parsed: { 'data' => base_user_data })) end it 'returns symbolized user data' do - expect(subject.data).to include( + expect(strategy.data).to include( id: 'c2ac35c6-aa8c-11ed-afa1-0242ac120002', first_name: 'Jane' ) @@ -19,29 +19,29 @@ RSpec.shared_examples 'expandable fields tests' do let(:expanded_user_data) do - base_user_data.merge({ - 'profile' => { - 'age' => 22, - 'gender' => 'Female' - }, - 'education' => [{ - 'current' => true, - 'school_name' => 'Hacker University', - 'major' => 'Computer Science' - }] - }) + base_user_data.merge( + 'profile' => { + 'age' => 22, + 'gender' => 'Female' + }, + 'education' => [{ + 'current' => true, + 'school_name' => 'Hacker University', + 'major' => 'Computer Science' + }] + ) end context 'with expandable fields' do before do allow(access_token).to receive(:get) .with('https://api.mlh.com/v4/users/me?expand[]=profile&expand[]=education') - .and_return(double(parsed: { 'data' => expanded_user_data })) + .and_return(instance_double(OAuth2::Response, parsed: { 'data' => expanded_user_data })) + allow(strategy).to receive(:options).and_return(expand_fields: ['profile', 'education']) end it 'fetches expanded fields' do - allow(subject).to receive(:options).and_return(expand_fields: ['profile', 'education']) - expect(subject.data).to include( + expect(strategy.data).to include( profile: include(age: 22), education: include(hash_including(school_name: 'Hacker University')) ) @@ -56,7 +56,7 @@ end it 'returns empty hash on error' do - expect(subject.data).to eq({}) + expect(strategy.data).to eq({}) end end end @@ -72,11 +72,11 @@ end before do - allow(subject).to receive(:data).and_return(user_info) + allow(strategy).to receive(:data).and_return(user_info) end it 'returns formatted info hash' do - expect(subject.info).to include( + expect(strategy.info).to include( name: 'Jane Hacker', email: 'jane.hacker@example.com' ) From a9f91f10178b694af429af4dd1d3c18a6e98a545 Mon Sep 17 00:00:00 2001 From: Jonathan Gottfried Date: Sun, 10 Nov 2024 21:16:52 -0500 Subject: [PATCH 08/20] We don't need to support Ruby 2.7 --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index dfd2784..e9ad057 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -13,7 +13,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - ruby: ['2.7', '3.2'] + ruby: ['3.2'] steps: - uses: actions/checkout@v2 - name: Set up Ruby ${{ matrix.ruby }} From 15ae467a00243cd25666e9174c6aeabf0c916b64 Mon Sep 17 00:00:00 2001 From: Jonathan Gottfried Date: Sun, 10 Nov 2024 22:02:48 -0500 Subject: [PATCH 09/20] Update gem version to 2.0 --- lib/omniauth-mlh/version.rb | 2 +- spec/omni_auth/mlh_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/omniauth-mlh/version.rb b/lib/omniauth-mlh/version.rb index dc26d75..dd3cbc6 100644 --- a/lib/omniauth-mlh/version.rb +++ b/lib/omniauth-mlh/version.rb @@ -2,6 +2,6 @@ module OmniAuth module MLH - VERSION = '1.0.1' + VERSION = '2.0.0' end end diff --git a/spec/omni_auth/mlh_spec.rb b/spec/omni_auth/mlh_spec.rb index efc7934..b332012 100644 --- a/spec/omni_auth/mlh_spec.rb +++ b/spec/omni_auth/mlh_spec.rb @@ -5,7 +5,7 @@ RSpec.describe OmniAuth::MLH do it 'has a version number' do expect(OmniAuth::MLH::VERSION).not_to be_nil - expect(OmniAuth::MLH::VERSION).to eq('1.0.1') + expect(OmniAuth::MLH::VERSION).to eq('2.0.0') end it 'loads the MLH strategy' do From e0d5b613e46b2f1d7e8a957febc5dc68b3db3856 Mon Sep 17 00:00:00 2001 From: Jonathan Gottfried Date: Sun, 10 Nov 2024 22:24:36 -0500 Subject: [PATCH 10/20] Correct json parsing --- lib/omniauth/strategies/mlh.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/omniauth/strategies/mlh.rb b/lib/omniauth/strategies/mlh.rb index 8a76052..0e48523 100644 --- a/lib/omniauth/strategies/mlh.rb +++ b/lib/omniauth/strategies/mlh.rb @@ -72,8 +72,8 @@ def data private def fetch_and_process_data - response = access_token.get(build_api_url).parsed - data = response.deep_symbolize_keys[:data] + response = access_token.get(build_api_url) + data = JSON.parse(response.body, symbolize_names: true) return {} unless data.is_a?(Hash) symbolize_nested_arrays(data) From 6cafa4eab83b83a33999918b68b4d8175767bd9c Mon Sep 17 00:00:00 2001 From: "devin-ai-integration[bot]" <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 11 Nov 2024 03:29:53 +0000 Subject: [PATCH 11/20] fix: properly handle data wrapper in API response and update test infrastructure --- lib/omniauth/strategies/mlh.rb | 4 ++-- spec/omni_auth/strategies/mlh_spec.rb | 8 ++++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/omniauth/strategies/mlh.rb b/lib/omniauth/strategies/mlh.rb index 0e48523..d39d0b9 100644 --- a/lib/omniauth/strategies/mlh.rb +++ b/lib/omniauth/strategies/mlh.rb @@ -74,9 +74,9 @@ def data def fetch_and_process_data response = access_token.get(build_api_url) data = JSON.parse(response.body, symbolize_names: true) - return {} unless data.is_a?(Hash) + return {} unless data.is_a?(Hash) && data[:data].is_a?(Hash) - symbolize_nested_arrays(data) + symbolize_nested_arrays(data[:data]) end def build_api_url diff --git a/spec/omni_auth/strategies/mlh_spec.rb b/spec/omni_auth/strategies/mlh_spec.rb index 974029a..b1234b3 100644 --- a/spec/omni_auth/strategies/mlh_spec.rb +++ b/spec/omni_auth/strategies/mlh_spec.rb @@ -13,7 +13,11 @@ end shared_context 'with oauth response' do |response_data| - let(:oauth_response) { instance_double(OAuth2::Response, parsed: { 'data' => response_data }) } + let(:oauth_response) do + instance_double(OAuth2::Response, + body: { 'data' => response_data }.to_json, + parsed: { 'data' => response_data }) + end before do allow(access_token).to receive(:get) .with('https://api.mlh.com/v4/users/me') @@ -23,7 +27,7 @@ describe '#data' do context 'with expandable fields' do - let(:response) { instance_double(OAuth2::Response, parsed: { 'data' => {} }) } + let(:response) { instance_double(OAuth2::Response, body: { 'data' => {} }.to_json, parsed: { 'data' => {} }) } let(:expand_url) { 'https://api.mlh.com/v4/users/me?expand[]=profile&expand[]=education' } before do From 90dae4cde55684d870fdced63a9333dedea85746 Mon Sep 17 00:00:00 2001 From: "devin-ai-integration[bot]" <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 11 Nov 2024 03:34:31 +0000 Subject: [PATCH 12/20] Revert "fix: properly handle data wrapper in API response and update test infrastructure" This reverts commit 6cafa4eab83b83a33999918b68b4d8175767bd9c. --- lib/omniauth/strategies/mlh.rb | 4 ++-- spec/omni_auth/strategies/mlh_spec.rb | 8 ++------ 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/lib/omniauth/strategies/mlh.rb b/lib/omniauth/strategies/mlh.rb index d39d0b9..0e48523 100644 --- a/lib/omniauth/strategies/mlh.rb +++ b/lib/omniauth/strategies/mlh.rb @@ -74,9 +74,9 @@ def data def fetch_and_process_data response = access_token.get(build_api_url) data = JSON.parse(response.body, symbolize_names: true) - return {} unless data.is_a?(Hash) && data[:data].is_a?(Hash) + return {} unless data.is_a?(Hash) - symbolize_nested_arrays(data[:data]) + symbolize_nested_arrays(data) end def build_api_url diff --git a/spec/omni_auth/strategies/mlh_spec.rb b/spec/omni_auth/strategies/mlh_spec.rb index b1234b3..974029a 100644 --- a/spec/omni_auth/strategies/mlh_spec.rb +++ b/spec/omni_auth/strategies/mlh_spec.rb @@ -13,11 +13,7 @@ end shared_context 'with oauth response' do |response_data| - let(:oauth_response) do - instance_double(OAuth2::Response, - body: { 'data' => response_data }.to_json, - parsed: { 'data' => response_data }) - end + let(:oauth_response) { instance_double(OAuth2::Response, parsed: { 'data' => response_data }) } before do allow(access_token).to receive(:get) .with('https://api.mlh.com/v4/users/me') @@ -27,7 +23,7 @@ describe '#data' do context 'with expandable fields' do - let(:response) { instance_double(OAuth2::Response, body: { 'data' => {} }.to_json, parsed: { 'data' => {} }) } + let(:response) { instance_double(OAuth2::Response, parsed: { 'data' => {} }) } let(:expand_url) { 'https://api.mlh.com/v4/users/me?expand[]=profile&expand[]=education' } before do From 09c4f586ff589fe38ba7d3b7a8ad09c5743d5ee6 Mon Sep 17 00:00:00 2001 From: "devin-ai-integration[bot]" <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 11 Nov 2024 03:36:27 +0000 Subject: [PATCH 13/20] fix: update test doubles to match strategy implementation --- spec/omni_auth/strategies/mlh_spec.rb | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/spec/omni_auth/strategies/mlh_spec.rb b/spec/omni_auth/strategies/mlh_spec.rb index 974029a..6100cd2 100644 --- a/spec/omni_auth/strategies/mlh_spec.rb +++ b/spec/omni_auth/strategies/mlh_spec.rb @@ -13,7 +13,11 @@ end shared_context 'with oauth response' do |response_data| - let(:oauth_response) { instance_double(OAuth2::Response, parsed: { 'data' => response_data }) } + let(:oauth_response) do + instance_double(OAuth2::Response, + body: response_data.to_json, + parsed: response_data) + end before do allow(access_token).to receive(:get) .with('https://api.mlh.com/v4/users/me') @@ -23,7 +27,9 @@ describe '#data' do context 'with expandable fields' do - let(:response) { instance_double(OAuth2::Response, parsed: { 'data' => {} }) } + let(:response) do + instance_double(OAuth2::Response, body: {}.to_json, parsed: {}) + end let(:expand_url) { 'https://api.mlh.com/v4/users/me?expand[]=profile&expand[]=education' } before do From 95ef0660270a247ac5f6a844b1dadb7aad5deae1 Mon Sep 17 00:00:00 2001 From: "devin-ai-integration[bot]" <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 11 Nov 2024 03:38:40 +0000 Subject: [PATCH 14/20] style: fix argument alignment in spec file --- spec/omni_auth/strategies/mlh_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/omni_auth/strategies/mlh_spec.rb b/spec/omni_auth/strategies/mlh_spec.rb index 6100cd2..4caab75 100644 --- a/spec/omni_auth/strategies/mlh_spec.rb +++ b/spec/omni_auth/strategies/mlh_spec.rb @@ -15,8 +15,8 @@ shared_context 'with oauth response' do |response_data| let(:oauth_response) do instance_double(OAuth2::Response, - body: response_data.to_json, - parsed: response_data) + body: response_data.to_json, + parsed: response_data) end before do allow(access_token).to receive(:get) From 7a21ff5994bf41dd2440341072dd049ae77f225f Mon Sep 17 00:00:00 2001 From: Jonathan Gottfried Date: Mon, 11 Nov 2024 13:16:56 -0500 Subject: [PATCH 15/20] rubocop formatting --- lib/omniauth/strategies/mlh.rb | 44 ++++++++++++++++------------------ 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/lib/omniauth/strategies/mlh.rb b/lib/omniauth/strategies/mlh.rb index 0e48523..14cf495 100644 --- a/lib/omniauth/strategies/mlh.rb +++ b/lib/omniauth/strategies/mlh.rb @@ -42,29 +42,29 @@ class MLH < OmniAuth::Strategies::OAuth2 # :nodoc: uid { data[:id] } info do - prune_hash({ - # Basic fields - id: data[:id], - created_at: data[:created_at], - updated_at: data[:updated_at], - first_name: data[:first_name], - last_name: data[:last_name], - email: data[:email], - phone_number: data[:phone_number], - roles: data[:roles], - - # Expandable fields - profile: data[:profile], - address: data[:address], - social_profiles: data[:social_profiles], - professional_experience: data[:professional_experience], - education: data[:education], - identifiers: data[:identifiers] - }) + { + # Basic fields + id: data[:id], + created_at: data[:created_at], + updated_at: data[:updated_at], + first_name: data[:first_name], + last_name: data[:last_name], + email: data[:email], + phone_number: data[:phone_number], + roles: data[:roles], + + # Expandable fields + profile: data[:profile], + address: data[:address], + social_profiles: data[:social_profiles], + professional_experience: data[:professional_experience], + education: data[:education], + identifiers: data[:identifiers] + } end def data - @data ||= fetch_and_process_data + @data ||= fetch_and_process_data.compact rescue StandardError {} end @@ -88,10 +88,6 @@ def build_api_url "#{url}?#{expand_query}" end - def prune_hash(hash) - hash.reject { |_, v| v.nil? } - end - def symbolize_nested_arrays(hash) hash.transform_values do |value| case value From 1179c47ad9faa23933afad336c86d120424e6239 Mon Sep 17 00:00:00 2001 From: Jonathan Gottfried Date: Mon, 11 Nov 2024 13:18:51 -0500 Subject: [PATCH 16/20] add line breaks as per Rashika's request --- spec/omni_auth/strategies/mlh_spec.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/spec/omni_auth/strategies/mlh_spec.rb b/spec/omni_auth/strategies/mlh_spec.rb index 4caab75..c006f96 100644 --- a/spec/omni_auth/strategies/mlh_spec.rb +++ b/spec/omni_auth/strategies/mlh_spec.rb @@ -39,6 +39,7 @@ it 'constructs the correct URL with expand parameters' do strategy.data + expect(access_token).to have_received(:get).with(expand_url) end @@ -59,6 +60,7 @@ it 'correctly parses nested profile data' do result = strategy.data + expect(result).to be_a(Hash) expect(result[:profile]).to eq({ age: 22, gender: 'Female' }) end @@ -96,6 +98,7 @@ it 'correctly processes complex nested structures' do result = strategy.data + expect(result).to be_a(Hash) expect(result[:education].first[:school]).to eq({ name: 'Test University', location: 'Test City' }) expect(result[:social_profiles]).to eq([{ platform: 'github', url: 'https://github.com' }, 'https://twitter.com']) @@ -125,6 +128,7 @@ it 'handles nil values correctly' do result = strategy.data + expect(result[:last_name]).to be_nil expect(result[:profile][:gender]).to be_nil expect(result[:profile][:location]).to eq({ city: nil, country: 'USA' }) @@ -133,6 +137,7 @@ it 'handles empty hash structures' do result = strategy.data + expect(result[:social_profiles]).to eq({ github: {}, linkedin: {} }) end end From 72c62812dbebe9e526d26f667e572f8279ce499e Mon Sep 17 00:00:00 2001 From: Jonathan Gottfried Date: Mon, 11 Nov 2024 13:23:35 -0500 Subject: [PATCH 17/20] Change the style as per Rashika's request in https://github.com/MLH/omniauth-mlh/pull/16#discussion_r1836164980 --- spec/omni_auth/strategies/mlh_spec.rb | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/spec/omni_auth/strategies/mlh_spec.rb b/spec/omni_auth/strategies/mlh_spec.rb index c006f96..7791aad 100644 --- a/spec/omni_auth/strategies/mlh_spec.rb +++ b/spec/omni_auth/strategies/mlh_spec.rb @@ -145,6 +145,7 @@ context 'with API error' do it 'returns empty hash on error' do allow(access_token).to receive(:get).and_raise(StandardError) + expect(strategy.data).to eq({}) end end @@ -152,21 +153,15 @@ describe '#uid' do context 'with valid data' do - before do - allow(strategy).to receive(:data).and_return({ id: 'test-123' }) - end - it 'returns the id from the data hash' do + allow(strategy).to receive(:data).and_return({ id: 'test-123' }) expect(strategy.uid).to eq('test-123') end end context 'with missing id' do - before do - allow(strategy).to receive(:data).and_return({}) - end - it 'returns nil when id is not present' do + allow(strategy).to receive(:data).and_return({}) expect(strategy.uid).to be_nil end end From d4c26a813efd4468b912f8052a1af1d906a7c239 Mon Sep 17 00:00:00 2001 From: Jonathan Gottfried Date: Mon, 11 Nov 2024 13:49:51 -0500 Subject: [PATCH 18/20] Update the README for v4 --- README.md | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 49b28c2..a8ab9b7 100644 --- a/README.md +++ b/README.md @@ -4,17 +4,16 @@ [![Test](https://github.com/MLH/omniauth-mlh/actions/workflows/test.yml/badge.svg)](https://github.com/MLH/omniauth-mlh/actions/workflows/test.yml) This is the official [OmniAuth](https://github.com/omniauth/omniauth) strategy for -authenticating with [MyMLH](https://my.mlh.io). To use it, you'll need to -[register an application](https://my.mlh.io/oauth/applications) and obtain a OAuth Application ID and Secret from MyMLH. +authenticating with [MyMLH](https://my.mlh.io) in Ruby applications. To use it, you'll need to +[register an application](https://my.mlh.io/developers) and obtain a OAuth Application ID and Secret from MyMLH. -It now supports MyMLH API V3. [Read the MyMLH V3 docs here](https://my.mlh.io/docs). +It now supports MyMLH API V4. [Read the MyMLH V4 docs here](https://my.mlh.io/developers/docs). Once you have done so, you can follow the instructions below: ## Requirements -This Gem requires your Ruby version to be at least `2.2.0`, which is set -downstream by [Omniauth](https://github.com/omniauth/omniauth/blob/master/omniauth.gemspec#L22). +This Gem requires your Ruby version to be at least `3.2.0`. ## Installation @@ -34,9 +33,11 @@ Or install it yourself as: ## Usage (Rack) +You can find a list of potential scopes and expandable fields in the [docs](https://my.mlh.io/developers/docs). The below defaults are provided simply as an example. + ```ruby use OmniAuth::Builder do - provider :mlh, ENV['MY_MLH_KEY'], ENV['MY_MLH_SECRET'], scope: 'default email birthday' + provider :mlh, ENV['MY_MLH_KEY'], ENV['MY_MLH_SECRET'], scope: 'public offline_access user:read:profile', expand_fields: ['education'] end ``` @@ -46,7 +47,7 @@ end # config/devise.rb Devise.setup do |config| - config.provider :mlh, ENV['MY_MLH_KEY'], ENV['MY_MLH_SECRET'], scope: 'default email birthday' + config.provider :mlh, ENV['MY_MLH_KEY'], ENV['MY_MLH_SECRET'], scope: 'public offline_access user:read:profile', expand_fields: ['education'] end ``` @@ -61,6 +62,6 @@ We used part of [datariot/omniauth-paypal](http://github.com/datariot/omniauth-p ## Questions? Have a question about the API or this library? Start by checking out the -[official MyMLH documentation](https://my.mlh.io/docs). If you still can't +[official MyMLH documentation](https://my.mlh.io/developers/docs). If you still can't find an answer, tweet at [@MLHacks](http://twitter.com/mlhacks) or drop an email to [engineering@mlh.io](mailto:engineering@mlh.io). From d2cf29ffd3ec135a1b5a1db070c4b394c4278039 Mon Sep 17 00:00:00 2001 From: Jonathan Gottfried Date: Mon, 11 Nov 2024 13:54:43 -0500 Subject: [PATCH 19/20] add example for accessing user data --- README.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/README.md b/README.md index a8ab9b7..3cce4a0 100644 --- a/README.md +++ b/README.md @@ -51,6 +51,21 @@ Devise.setup do |config| end ``` +## Accessing User Data +Once a user has been authorized and you have received a token in your callback, you may access the scoped information for that user via the info key on the request data, as per the below example from a simple Sinatra app: + +```ruby +get '/auth/mlh/callback' do + auth = request.env['omniauth.auth'] + user_data = auth['info'] + first_name = user_data['first_name'] + erb " +

Hello #{first_name}

" +end +``` + +You can find the full User object in the [docs](https://my.mlh.io/developers/docs). + ## Contributing For guidance on setting up a development environment and how to make a contribution to omniauth-mlh, see the [contributing guidelines](https://github.com/MLH/omniauth-mlh/blob/main/CONTRIBUTING.md). From 39295df49efe572df219b16920b4a246ec801bbf Mon Sep 17 00:00:00 2001 From: Jonathan Gottfried Date: Mon, 11 Nov 2024 13:58:12 -0500 Subject: [PATCH 20/20] README formatting fix --- README.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 3cce4a0..d4ed5e4 100644 --- a/README.md +++ b/README.md @@ -37,7 +37,9 @@ You can find a list of potential scopes and expandable fields in the [docs](http ```ruby use OmniAuth::Builder do - provider :mlh, ENV['MY_MLH_KEY'], ENV['MY_MLH_SECRET'], scope: 'public offline_access user:read:profile', expand_fields: ['education'] + provider :mlh, ENV['MY_MLH_KEY'], ENV['MY_MLH_SECRET'], + scope: 'public offline_access user:read:profile', + expand_fields: ['education'] end ``` @@ -47,7 +49,9 @@ end # config/devise.rb Devise.setup do |config| - config.provider :mlh, ENV['MY_MLH_KEY'], ENV['MY_MLH_SECRET'], scope: 'public offline_access user:read:profile', expand_fields: ['education'] + config.provider :mlh, ENV['MY_MLH_KEY'], ENV['MY_MLH_SECRET'], + scope: 'public offline_access user:read:profile', + expand_fields: ['education'] end ```