Merge pull request #962 from amatsuda/pure_omniauth

Decouple OmniAuth from Devise

Author: amatsuda

app/controllers/application_controller.rb

@@ -26,7 +26,7 @@ def after_sign_in_path_for(user)
       session[:pending_invite_accept_url]
     elsif !user.complete?
       edit_profile_path
-    elsif (referer = request.referer).present? && (URI.parse(referer).host == request.host) && (referer != new_user_session_url) && !referer.start_with?(edit_password_url(current_user)) && (referer != user_developer_omniauth_authorize_url)
+    elsif (referer = request.referer).present? && (URI.parse(referer).host == request.host) && (referer != new_user_session_url) && !referer.start_with?(edit_password_url(current_user)) && (referer != omniauth_authorize_url(provider: :developer))
       referer
     elsif session[:target]
       session.delete(:target)

app/controllers/users/omniauth_callbacks_controller.rb

@@ -1,4 +1,4 @@
-class Users::OmniauthCallbacksController < Devise::OmniauthCallbacksController
+class Users::OmniauthCallbacksController < ApplicationController
   class AuthHash < SimpleDelegator
     def account_name
       dig('info', 'nickname') || dig('extra', 'raw_info', 'screen_name')
@@ -9,24 +9,30 @@ def provider_name
     end
   end
 
-  before_action :link_identity_to_current_user, only: [:twitter, :github, :developer], if: -> { current_user }
-  skip_forgery_protection only: :developer
+  before_action :link_identity_to_current_user, only: :callback, if: -> { current_user }
+  skip_forgery_protection only: :callback
+  skip_before_action :current_event
 
-  def twitter
-    authenticate_with_hash
-  end
-
-  def github
-    authenticate_with_hash
+  def callback
+    case params[:provider]
+    when 'twitter'
+      authenticate_with_hash
+    when 'github'
+      authenticate_with_hash
+    when 'developer'
+      user = User.find_by(email: auth_hash.uid)
+      authenticate_with_hash(user)
+    else
+      failure
+    end
   end
 
-  def developer
-    user = User.find_by email: auth_hash.uid
-    authenticate_with_hash user
+  def passthru
+    render plain: 'Not Found', status: :not_found
   end
 
   def failure
-    redirect_to new_user_session_url, danger: "There was an error authenticating you. Please try again."
+    redirect_to new_user_session_path, flash: {danger: 'There was an error authenticating you. Please try again.'}
   end
 
   private
@@ -59,15 +65,15 @@ def authenticate_with_hash(user = nil)
     @user = user || User.from_omniauth(auth_hash, session[:pending_invite_email])
 
     if @user.persisted?
-      flash.now[:info] = "You have signed in with #{auth_hash.provider_name}."
+      flash[:info] = "You have signed in with #{auth_hash.provider_name}."
       logger.info "Signing in user #{@user.inspect}"
 
       @user.confirmed_at = Time.current
       sign_in @user
 
       redirect_to after_sign_in_path_for(@user)
     else
-      redirect_to new_user_session_url, danger: "There was an error authenticating via Auth provider: #{params[:provider]}."
+      redirect_to new_user_session_path, flash: {danger: "There was an error authenticating via Auth provider: #{params[:provider]}."}
     end
   end
 

app/models/user.rb

@@ -2,8 +2,7 @@ class User < ApplicationRecord
   # Include default devise modules. Others available are:
   # :confirmable, :lockable, :timeoutable
   devise :database_authenticatable, :registerable,
-         :recoverable, :rememberable, :trackable, :confirmable, #:validatable,
-         :omniauthable, omniauth_providers: [:twitter, :github, :developer]
+         :recoverable, :rememberable, :trackable, :confirmable #:validatable,
 
   has_many :identities,   dependent: :destroy
   has_many :invitations,  dependent: :destroy

app/views/devise/shared/_links.html.erb

@@ -21,16 +21,12 @@
 
   <hr />
 
-  <%- if devise_mapping.omniauthable? %>
-
-    <%= button_to user_twitter_omniauth_authorize_path, data: {turbo: false}, class: "btn btn-twitter", form: {style: "display: inline-block"} do %>
-      <i class="bi bi-twitter"></i>
-      <span class="btn-text-middle">Sign in with Twitter</span>
-    <% end %>
-    <%= button_to user_github_omniauth_authorize_path, data: {turbo: false}, class: "btn btn-github", form: {style: "display: inline-block"} do %>
-      <i class="bi bi-github"></i>
-      <span class="btn-text-middle">Sign in with GitHub</span>
-    <% end %>
-
+  <%= button_to omniauth_authorize_path(provider: :twitter), data: {turbo: false}, class: "btn btn-twitter", form: {style: "display: inline-block"} do %>
+    <i class="bi bi-twitter"></i>
+    <span class="btn-text-middle">Sign in with Twitter</span>
+  <% end %>
+  <%= button_to omniauth_authorize_path(provider: :github), data: {turbo: false}, class: "btn btn-github", form: {style: "display: inline-block"} do %>
+    <i class="bi bi-github"></i>
+    <span class="btn-text-middle">Sign in with GitHub</span>
   <% end %>
 </div>

app/views/profiles/edit.html.haml

@@ -72,7 +72,7 @@
                   = "Connected via #{name}"
             - else
               .service.mb-2
-                = button_to send("user_#{provider}_omniauth_authorize_path"), data: {turbo: false}, class: "btn btn-#{provider}", form: {style: 'display: inline-block'} do
+                = button_to omniauth_authorize_path(provider: provider), data: {turbo: false}, class: "btn btn-#{provider}", form: {style: 'display: inline-block'} do
                   %i{class: "bi bi-#{provider}"}
                   = "Connect #{name}"
 

config/initializers/devise.rb

@@ -241,9 +241,6 @@
   # ==> OmniAuth
   # Add a new OmniAuth provider. Check the wiki for more information on setting
   # up on your models and hooks.
-  config.omniauth :github, ENV['GITHUB_KEY'], ENV['GITHUB_SECRET'], scope: 'user:email'
-  config.omniauth :twitter, ENV['TWITTER_KEY'], ENV['TWITTER_SECRET']
-  config.omniauth :developer unless Rails.env.production?
 
   # ==> Warden configuration
   # If you want to use other strategies, that are not supported by Devise, or

config/initializers/omniauth.rb

@@ -0,0 +1,8 @@
+Rails.application.config.middleware.use OmniAuth::Builder do
+  provider :github, ENV['GITHUB_KEY'], ENV['GITHUB_SECRET'], scope: 'user:email'
+  provider :twitter, ENV['TWITTER_KEY'], ENV['TWITTER_SECRET']
+  provider :developer unless Rails.env.production?
+end
+
+OmniAuth.config.path_prefix = '/users/auth'
+OmniAuth.config.allowed_request_methods = [:post]

config/routes.rb

@@ -1,6 +1,11 @@
 Rails.application.routes.draw do
+  # OmniAuth
+  get  '/users/auth/:provider/callback', to: 'users/omniauth_callbacks#callback', as: :omniauth_callback
+  post '/users/auth/:provider', to: 'users/omniauth_callbacks#passthru', as: :omniauth_authorize
+  get  '/users/auth/failure', to: 'users/omniauth_callbacks#failure', as: :omniauth_failure
+
   root 'home#show'
-  devise_for :users, controllers: { omniauth_callbacks: 'users/omniauth_callbacks' }
+  devise_for :users
   mount ActionCable.server => '/cable'
 
   resource :profile, only: [:show, :edit, :update] do

spec/controllers/users/omniauth_callbacks_controller_spec.rb

@@ -20,7 +20,7 @@
     end
 
     it 'allows twitter login for new user' do
-      get :twitter
+      post :callback, params: {provider: 'twitter'}
       expect(response).to redirect_to(edit_profile_url)
       expect(User.last.identities.find_by(provider: 'twitter')).to be_present
     end
@@ -37,7 +37,7 @@
       let!(:identity) { user.identities.create!(provider: 'github', uid: github_auth_hash.uid) }
 
       it 'finds user via identity and signs in' do
-        expect { get :github }.not_to change { User.count }
+        expect { post :callback, params: {provider: 'github'} }.not_to change { User.count }
         expect(controller.current_user).to eq(user)
       end
     end
@@ -46,7 +46,7 @@
       let!(:user) { create(:user, provider: 'github', uid: github_auth_hash.uid) }
 
       it 'finds user via legacy lookup and signs in' do
-        expect { get :github }.not_to change { User.count }
+        expect { post :callback, params: {provider: 'github'} }.not_to change { User.count }
         expect(controller.current_user).to eq(user)
       end
     end
@@ -63,7 +63,7 @@
         user = create(:user)
         sign_in(user)
 
-        expect { get :github }.to change { user.identities.count }.by(1)
+        expect { post :callback, params: {provider: 'github'} }.to change { user.identities.count }.by(1)
         expect(response).to redirect_to(edit_profile_path)
         expect(flash[:info]).to eq('Successfully connected GitHub to your account.')
       end
@@ -75,7 +75,7 @@
         user.identities.create!(provider: 'github', uid: github_auth_hash.uid)
         sign_in(user)
 
-        expect { get :github }.not_to change { Identity.count }
+        expect { post :callback, params: {provider: 'github'} }.not_to change { Identity.count }
         expect(response).to redirect_to(edit_profile_path)
         expect(flash[:info]).to eq('GitHub is already connected to your account.')
       end
@@ -89,7 +89,7 @@
         user = create(:user)
         sign_in(user)
 
-        expect { get :github }.not_to change { Identity.count }
+        expect { post :callback, params: {provider: 'github'} }.not_to change { Identity.count }
         expect(response).to redirect_to(edit_profile_path)
         expect(flash[:danger]).to eq('This GitHub account is already connected to another user.')
       end
@@ -101,7 +101,7 @@
         user = create(:user)
         sign_in(user)
 
-        get :github
+        post :callback, params: {provider: 'github'}
 
         expect(response).to redirect_to(merge_profile_path)
         expect(session[:pending_merge_user_id]).to eq(legacy_user.id)