Merge pull request increments#75 from tomoasleep/fix/oauth-redirection

Qiitaログイン時のリダイレクト先を修正

Author: tomoasleep

app/controllers/auth/omniauth_callbacks_controller.rb

@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
 class Auth::OmniauthCallbacksController < Devise::OmniauthCallbacksController
+  include AfterSignInPathLeadable
+
   def qiita
     auth_hash = request.env['omniauth.auth']
 
@@ -15,21 +17,27 @@ def qiita
       else
         flash[:alert] = I18n.t('omniauth_callbacks.failure')
       end
-      redirect_to settings_qiita_authorizations_path
+      redirect_to after_sign_in_path_for(current_user)
     else
       if authorization = QiitaAuthorization.find_by(uid: auth_hash[:uid])
         sign_in(authorization.user)
-        redirect_to web_path
+        redirect_to after_sign_in_path_for(authorization.user)
       else
-        store_omniauth_auth
+        store_omniauth_data
         redirect_to new_user_oauth_registration_path
       end
     end
   end
 
   private
 
-  def store_omniauth_auth
+  def store_omniauth_data
     session[:devise_omniauth_auth] = request.env['omniauth.auth']
+    session[:devise_omniauth_origin] = request.env['omniauth.origin']
+  end
+
+  # @override
+  def location_after_sign_in
+    request.env['omniauth.origin'].presence || stored_location_for(:user)
   end
 end

app/controllers/concerns/after_sign_in_path_leadable.rb

@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+module AfterSignInPathLeadable
+  extend ActiveSupport::Concern
+
+  protected
+
+  def after_sign_in_path_for(resource)
+    after_sign_in_path = resolve_url(location_after_sign_in)
+
+    if home_paths(resource).include?((after_sign_in_path || '').split('?').first)
+      root_path
+    else
+      after_sign_in_path || root_path
+    end
+  end
+
+  private
+
+  def home_paths(resource)
+    paths = [about_path]
+    if single_user_mode? && resource.is_a?(User)
+      paths << short_account_path(username: resource.account)
+    end
+    paths
+  end
+
+  def resolve_url(url)
+    uri = URI.parse(url)
+    if !uri.host || uri.host == request.host
+      uri.query ? "#{uri.path}?#{uri.query}" : uri.path
+    else
+      nil
+    end
+  rescue URI::InvalidURIError => e
+    nil
+  end
+
+  def location_after_sign_in
+    fail NotImplementedError
+  end
+end

app/controllers/oauth_registrations_controller.rb

@@ -1,4 +1,5 @@
 class OauthRegistrationsController < DeviseController
+  include AfterSignInPathLeadable
   layout 'auth'
 
   before_action :check_enabled_registrations
@@ -17,7 +18,7 @@ def create
 
     if @oauth_registration.save
       sign_in(@oauth_registration.user)
-      redirect_to web_path
+      redirect_to after_sign_in_path_for(@oauth_registration.user)
       flash[:notice] = I18n.t('oauth_registration.success')
     else
       render :new, status: :unprocessable_entity
@@ -37,4 +38,9 @@ def check_enabled_registrations
   def require_omniauth_auth
     redirect_to root_path unless omniauth_auth
   end
+
+  # @override
+  def location_after_sign_in
+    session[:devise_omniauth_origin].presence || stored_location_for(:user)
+  end
 end

app/models/form/oauth_registration.rb

@@ -10,6 +10,8 @@ class UnsupportedProviderError < StandardError; end
 
   class << self
     def from_omniauth_auth(auth)
+      auth = auth.deep_symbolize_keys
+      
       case auth[:provider]
       when 'qiita'
         new(

spec/controllers/auth/omniauth_callbacks_controller_spec.rb

@@ -0,0 +1,124 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe Auth::OmniauthCallbacksController, type: :controller do
+  describe 'GET #qiita' do
+    include_context 'mock qiita omniauth'
+
+    let(:omniauth_auth) { qiita_auth }
+    before do
+      request.env['devise.mapping'] = Devise.mappings[:user]
+      request.env['omniauth.auth'] = omniauth_auth
+      request.env['omniauth.origin'] = try(:omniauth_origin)
+    end
+
+    subject { -> { get :qiita } }
+
+    context 'when signed in' do
+      before { sign_in(user) }
+      let(:user) { Fabricate(:user) }
+
+      context 'and current user is not linked to an qiita account' do
+        it 'links the user to the qiita account' do
+          is_expected.to change { user.reload.qiita_authorization }.from(nil).to(be_truthy)
+        end
+      end
+
+      context 'and omniauth.origin is empty' do
+        it 'redirect_to root_path' do
+          subject.call
+          expect(response).to be_redirect
+          expect(response).to redirect_to(root_path)
+        end
+      end
+
+      context 'and omniauth.origin is about_url' do
+        let(:omniauth_origin) { about_url }
+
+        it 'redirect_to root_path' do
+          subject.call
+          expect(response).to be_redirect
+          expect(response).to redirect_to(root_path)
+        end
+      end
+
+      context 'and omniauth.origin is settings_qiita_authorizations_url' do
+        let(:omniauth_origin) { settings_qiita_authorizations_url }
+
+        it 'redirect_to settings_qiita_authorizations_path' do
+          subject.call
+          expect(response).to be_redirect
+          expect(response).to redirect_to(settings_qiita_authorizations_path)
+        end
+      end
+    end
+
+    context 'when not signed in' do
+      context 'and there are a user linked to the qiita account' do
+        let(:user) { Fabricate(:user) }
+        let!(:qiita_authorization) { Fabricate(:qiita_authorization, uid: omniauth_auth[:uid], user: user) }
+
+        it 'logs the user in' do
+          subject.call
+          expect(controller.current_user).to eq user
+        end
+
+        context 'and omniauth.origin is empty' do
+          it 'redirect_to root_path' do
+            subject.call
+            expect(response).to be_redirect
+            expect(response).to redirect_to(root_path)
+          end
+        end
+
+        context 'and omniauth.origin is about_url' do
+          let(:omniauth_origin) { about_url }
+
+          it 'redirect_to root_path' do
+            subject.call
+            expect(response).to be_redirect
+            expect(response).to redirect_to(root_path)
+          end
+        end
+
+        context 'and omniauth.origin is settings_qiita_authorizations_url' do
+          let(:omniauth_origin) { settings_qiita_authorizations_url }
+
+          it 'redirect_to settings_qiita_authorizations_path' do
+            subject.call
+            expect(response).to be_redirect
+            expect(response).to redirect_to(settings_qiita_authorizations_path)
+          end
+        end
+      end
+
+      context 'and there are no user linked to the qiita account' do
+        it 'redirects to new_user_oauth_registration_path' do
+          subject.call
+          expect(response).to redirect_to(new_user_oauth_registration_path)
+        end
+
+        context 'when omniauth.origin is a url' do
+          let(:omniauth_origin) { root_url }
+
+          it 'stores the omniauth origin to session' do
+            subject.call
+            expect(session[:devise_omniauth_origin]).to be(omniauth_origin)
+          end
+        end
+
+        it 'stores the omniauth data to session to build form' do
+          subject.call
+          expect(Form::OauthRegistration.from_omniauth_auth(session[:devise_omniauth_auth])).to have_attributes({
+            provider: omniauth_auth[:provider],
+            avatar: omniauth_auth[:info][:image],
+            uid: omniauth_auth[:uid],
+            username: omniauth_auth[:uid],
+            token: omniauth_auth[:credentials][:token],
+          })
+        end
+      end
+    end
+  end
+end

spec/controllers/oauth_registrations_controller_spec.rb

@@ -0,0 +1,97 @@
+require 'rails_helper'
+require 'securerandom'
+
+RSpec.describe OauthRegistrationsController, type: :controller do
+  include_context 'mock qiita omniauth'
+
+  let(:omniauth_auth) { qiita_auth }
+  before do
+    request.env['devise.mapping'] = Devise.mappings[:user]
+    stub_request(:get, qiita_auth[:info][:image]).to_return(request_fixture('avatar.txt'))
+  end
+
+  shared_context 'store omniauth data' do
+    before do
+      session[:devise_omniauth_auth] = omniauth_auth
+      session[:devise_omniauth_origin] = try(:omniauth_origin)
+    end
+  end
+
+  describe 'GET #new' do
+    subject { -> { get :new } }
+
+    context 'when omniauth data are stored' do
+      include_context 'store omniauth data'
+
+      it 'returns 200' do
+        subject.call
+        expect(response).to have_http_status(:success)
+      end
+    end
+
+    context 'when omniauth data are not stored' do
+      it 'redirect_to root_path' do
+        subject.call
+        expect(response).to be_redirect
+        expect(response).to redirect_to(root_path)
+      end
+    end
+  end
+
+  describe 'POST #create' do
+    subject { -> { post :create, { params: { form_oauth_registration: form_params } } } }
+
+    context 'when omniauth data are stored' do
+      include_context 'store omniauth data'
+
+      context 'and email and username is given' do
+        let(:form_params) { { email: email, username: username } }
+        let(:username) { 'foo' }
+        let(:email) { 'foo@example.com' }
+
+        it { is_expected.to change { User.find_by(account: Account.find_by(username: username), email: email) }.from(nil).to(be_truthy) }
+        it { is_expected.to change { QiitaAuthorization.find_by(user: User.find_by(email: email), uid: omniauth_auth[:uid], token: omniauth_auth[:credentials][:token]) }.from(nil).to(be_truthy) }
+
+        context 'and omniauth.origin is empty' do
+          it 'redirect_to root_path' do
+            subject.call
+            expect(response).to be_redirect
+            expect(response).to redirect_to(root_path)
+          end
+        end
+
+        context 'and omniauth.origin is about_url' do
+          let(:omniauth_origin) { about_url }
+
+          it 'redirect_to root_path' do
+            subject.call
+            expect(response).to be_redirect
+            expect(response).to redirect_to(root_path)
+          end
+        end
+
+        context 'and omniauth.origin is settings_qiita_authorizations_url' do
+          let(:omniauth_origin) { settings_qiita_authorizations_url }
+
+          it 'redirect_to settings_qiita_authorizations_path' do
+            subject.call
+            expect(response).to be_redirect
+            expect(response).to redirect_to(settings_qiita_authorizations_path)
+          end
+        end
+      end
+    end
+
+    context 'when omniauth data are not stored' do
+      let(:form_params) { { email: email, username: username } }
+      let(:username) { 'foo' }
+      let(:email) { 'foo@example.com' }
+
+      it 'redirect_to root_path' do
+        subject.call
+        expect(response).to be_redirect
+        expect(response).to redirect_to(root_path)
+      end
+    end
+  end
+end

spec/fabricators/qiita_authorization_fabricator.rb

@@ -1,5 +1,5 @@
 Fabricator(:qiita_authorization) do
   user              nil
   uid               "qiitan"
-  provider          "qiita"
+  token             "token"
 end

spec/rails_helper.rb

@@ -16,6 +16,7 @@
 Redis.current = Redis::Namespace.new("mastodon_test#{ENV['TEST_ENV_NUMBER']}", redis: Redis.current)
 Sidekiq::Testing.inline!
 Sidekiq::Logging.logger = nil
+OmniAuth.config.test_mode = true
 
 Devise::Test::ControllerHelpers.module_eval do
   alias_method :original_sign_in, :sign_in

spec/support/contexts/omniauth_qiita.rb

@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+shared_context 'mock qiita omniauth' do
+  let(:qiita_auth) do
+    {
+      provider: 'qiita',
+      uid: 'test_uid',
+      info: {
+        email: 'test@example.com',
+        name: 'test_name',
+        image: 'http://example.com/image.jpg'
+      },
+      credentials: {
+        token: 'credentialstoken',
+        refresh_token: 'credentialssecret',
+      },
+      extra: {
+        raw_info: {}
+      }
+    }
+  end
+
+  before { OmniAuth.config.mock_auth[:qiita] = OmniAuth::AuthHash.new(qiita_auth) }
+  after { OmniAuth.config.mock_auth[:qiita] = nil }
+end