[DEV-2013] V4 updates

.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 }}

Gemfile

@@ -4,3 +4,4 @@ source 'https://rubygems.org'
 
 # Specify your gem's dependencies in omniauth-mlh.gemspec
 gemspec
+gem 'activesupport'

lib/omniauth-mlh/version.rb

@@ -2,6 +2,6 @@
 
 module OmniAuth
   module MLH
-    VERSION = '1.0.1'
+    VERSION = '2.0.0'
   end
 end

lib/omniauth/strategies/mlh.rb

@@ -5,42 +5,103 @@
 
 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({
+                     # 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
+      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 prune_hash(hash)
+        hash.reject { |_, v| v.nil? }
+      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

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('2.0.0')
   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

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
+      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: '[email protected]',
+        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: '[email protected]'
+      )
+    end
+
+    it 'includes user roles' do
+      expect(strategy.info[:roles]).to eq(['hacker'])
+    end
+  end
+end