Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
36 changes: 21 additions & 15 deletions app/controllers/users/omniauth_callbacks_controller.rb
Original file line number Diff line number Diff line change
@@ -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')
Expand All @@ -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
Expand Down Expand Up @@ -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

Expand Down
3 changes: 1 addition & 2 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 7 additions & 11 deletions app/views/devise/shared/_links.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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>
2 changes: 1 addition & 1 deletion app/views/profiles/edit.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -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}"

Expand Down
3 changes: 0 additions & 3 deletions config/initializers/devise.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions config/initializers/omniauth.rb
Original file line number Diff line number Diff line change
@@ -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]
7 changes: 6 additions & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down
14 changes: 7 additions & 7 deletions spec/controllers/users/omniauth_callbacks_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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)
Expand Down