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 }} 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/README.md b/README.md index 49b28c2..d4ed5e4 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,13 @@ 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,10 +49,27 @@ 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 ``` +## 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). @@ -61,6 +81,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). diff --git a/lib/omniauth-mlh/version.rb b/lib/omniauth-mlh/version.rb index dc26d75..3f3fb81 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 = '4.0.1' end end diff --git a/lib/omniauth/strategies/mlh.rb b/lib/omniauth/strategies/mlh.rb index 3d0419e..14cf495 100644 --- a/lib/omniauth/strategies/mlh.rb +++ b/lib/omniauth/strategies/mlh.rb @@ -5,42 +5,99 @@ 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 - ) + { + # 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 - access_token.get('/api/v3/user.json').parsed.deep_symbolize_keys[:data] - rescue StandardError - {} + @data ||= fetch_and_process_data.compact + rescue StandardError + {} + end + + private + + 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) + + 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 symbolize_nested_arrays(hash) + 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/omni_auth/mlh_spec.rb b/spec/omni_auth/mlh_spec.rb index 0443034..c821ee7 100644 --- a/spec/omni_auth/mlh_spec.rb +++ b/spec/omni_auth/mlh_spec.rb @@ -2,38 +2,14 @@ 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 +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('4.0.1') 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 + 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/omni_auth/strategies/mlh_spec.rb b/spec/omni_auth/strategies/mlh_spec.rb new file mode 100644 index 0000000..7791aad --- /dev/null +++ b/spec/omni_auth/strategies/mlh_spec.rb @@ -0,0 +1,196 @@ +# 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) 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') + .and_return(oauth_response) + end + end + + describe '#data' do + context 'with expandable fields' do + 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 + 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 + 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 + it 'returns nil when id is not present' do + allow(strategy).to receive(:data).and_return({}) + 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/spec_helper.rb b/spec/spec_helper.rb index 51e4f1c..1fd07d5 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,23 +1,31 @@ # 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-oauth2' require 'omniauth_mlh' +require 'omniauth/strategies/mlh' -Dir[File.expand_path('support/**/*', __dir__)].sort.each { |f| require f } +Dir[File.expand_path('support/**/*.rb', __dir__)].sort.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..a93ad96 100644 --- a/spec/support/shared_examples.rb +++ b/spec/support/shared_examples.rb @@ -1,46 +1,84 @@ # 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(instance_double(OAuth2::Response, parsed: { 'data' => base_user_data })) + end - expect(subject.client.options[:authorize_url]).to eq('https://example.com') + it 'returns symbolized user data' do + expect(strategy.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(instance_double(OAuth2::Response, parsed: { 'data' => expanded_user_data })) + allow(strategy).to receive(:options).and_return(expand_fields: ['profile', 'education']) 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 + expect(strategy.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(strategy.data).to eq({}) end + end +end + +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 - it 'includes top-level options that are marked as :token_options' do - @options = { token_options: [:scope, :foo], scope: 'bar', foo: 'baz' } + before do + allow(strategy).to receive(:data).and_return(user_info) + end - expect(subject.token_params['scope']).to eq('bar') - expect(subject.token_params['foo']).to eq('baz') - end + it 'returns formatted info hash' do + expect(strategy.info).to include( + name: 'Jane Hacker', + email: 'jane.hacker@example.com' + ) end end