[deploy] Authentication refactoring: cleanups (#7576)
* Use Devise registry to list available providers * Cleanups following Devise+Omniauth docs * More cleanups * Use helpers * More cleanups * Oops, forgot to remove these specs
This commit is contained in:
parent
346ce44169
commit
8407bf9b4e
34 changed files with 243 additions and 163 deletions
|
|
@ -1,5 +1,4 @@
|
|||
class OmniauthCallbacksController < Devise::OmniauthCallbacksController
|
||||
# Don't need a policy for this since this is our sign up/in route
|
||||
include Devise::Controllers::Rememberable
|
||||
|
||||
# Called upon successful redirect from Twitter
|
||||
|
|
@ -36,22 +35,25 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
|
|||
|
||||
private
|
||||
|
||||
# TODO: [thepracticaldev/oss] investigate if there's a way to simplify this
|
||||
# by using Devise+OmniAuth builtin integration
|
||||
# <https://github.com/heartcombo/devise/wiki/OmniAuth:-Overview>
|
||||
def callback_for(provider)
|
||||
auth_payload = request.env["omniauth.auth"]
|
||||
cta_variant = request.env["omniauth.params"]["state"].to_s
|
||||
|
||||
@user = Authentication::Authenticator.call(
|
||||
request.env["omniauth.auth"],
|
||||
auth_payload,
|
||||
current_user: current_user,
|
||||
cta_variant: cta_variant,
|
||||
)
|
||||
|
||||
if user_persisted_and_valid?
|
||||
# Devise's Omniauthable does not automatically remember users
|
||||
# see <https://github.com/heartcombo/devise/wiki/Omniauthable,-sign-out-action-and-rememberable>
|
||||
remember_me(@user)
|
||||
|
||||
set_flash_message(:notice, :success, kind: provider.to_s.capitalize) if is_navigational_format?
|
||||
set_flash_message(:notice, :success, kind: provider.to_s.titleize) if is_navigational_format?
|
||||
|
||||
# `event: authentication` is only needed for Warden callbacks
|
||||
# see <config/initializers/persistent_csrf_token_cookie.rb>
|
||||
sign_in_and_redirect(@user, event: :authentication)
|
||||
# NOTE: I can't find a way to test this path
|
||||
# as `User` will assign a temporary username if the username already exists
|
||||
|
|
@ -65,6 +67,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
|
|||
# here in case of validation errors, see:
|
||||
# https://github.com/thepracticaldev/dev.to/blob/80737b540453afe8775128cb37bd379b7c09c7e8/app/services/authorization_service.rb#L77
|
||||
else
|
||||
# Devise will clean this data when the user is not persisted
|
||||
session["devise.#{provider}_data"] = request.env["omniauth.auth"]
|
||||
|
||||
user_errors = @user.errors.full_messages
|
||||
|
|
|
|||
|
|
@ -1,10 +1,9 @@
|
|||
class RegistrationsController < Devise::RegistrationsController
|
||||
prepend_before_action :require_no_authentication, only: []
|
||||
# No authorization required for public registration route
|
||||
|
||||
def new
|
||||
if user_signed_in?
|
||||
redirect_to "/dashboard?signed-in-already&t=#{Time.current.to_i}"
|
||||
redirect_to dashboard_path
|
||||
else
|
||||
super
|
||||
end
|
||||
|
|
|
|||
11
app/helpers/authentication_helper.rb
Normal file
11
app/helpers/authentication_helper.rb
Normal file
|
|
@ -0,0 +1,11 @@
|
|||
module AuthenticationHelper
|
||||
def authentication_provider(provider_name)
|
||||
Authentication::Providers.get!(provider_name)
|
||||
end
|
||||
|
||||
def authentication_enabled_providers
|
||||
Authentication::Providers.enabled.map do |provider_name|
|
||||
Authentication::Providers.get!(provider_name)
|
||||
end
|
||||
end
|
||||
end
|
||||
23
app/services/authentication/paths.rb
Normal file
23
app/services/authentication/paths.rb
Normal file
|
|
@ -0,0 +1,23 @@
|
|||
module Authentication
|
||||
# These are meant to be called from the specific providers
|
||||
module Paths
|
||||
# Returns the authentication path for the given provider
|
||||
def self.authentication_path(provider_name, params = {})
|
||||
Rails.application.routes.url_helpers.public_send(
|
||||
"user_#{provider_name}_omniauth_authorize_path", params
|
||||
)
|
||||
end
|
||||
|
||||
# Returns the sign in URL for the given provider
|
||||
def self.sign_in_path(provider_name, params = {})
|
||||
url_helpers = Rails.application.routes.url_helpers
|
||||
|
||||
callback_url_helper = "user_#{provider_name}_omniauth_callback_path"
|
||||
mandatory_params = {
|
||||
callback_url: URL.url(url_helpers.public_send(callback_url_helper))
|
||||
}
|
||||
|
||||
authentication_path(provider_name, params.merge(mandatory_params))
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
@ -11,7 +11,7 @@ module Authentication
|
|||
def self.get!(provider_name)
|
||||
name = provider_name.to_s.titleize
|
||||
|
||||
unless Authentication::Providers.const_defined?(name)
|
||||
unless available?(provider_name)
|
||||
raise(
|
||||
::Authentication::Errors::ProviderNotFound,
|
||||
"Provider #{name} is not available!",
|
||||
|
|
@ -28,14 +28,16 @@ module Authentication
|
|||
Authentication::Providers.const_get(name)
|
||||
end
|
||||
|
||||
# Returns available providers
|
||||
def self.available
|
||||
# the magic is done in <config/initializers/authentication_providers.rb>
|
||||
Authentication::Providers::Provider.subclasses.map do |subclass|
|
||||
subclass.name.demodulize.downcase.to_sym
|
||||
end.sort
|
||||
end
|
||||
|
||||
def self.available?(provider_name)
|
||||
Authentication::Providers.const_defined?(provider_name.to_s.titleize)
|
||||
end
|
||||
|
||||
# Returns enabled providers
|
||||
# TODO: [thepracticaldev/oss] ideally this should be "available - disabled"
|
||||
# we can get there once we have feature flags
|
||||
|
|
@ -43,28 +45,8 @@ module Authentication
|
|||
SiteConfig.authentication_providers.map(&:to_sym).sort
|
||||
end
|
||||
|
||||
# Returns true if a provider is enabled, false otherwise
|
||||
def self.enabled?(provider_name)
|
||||
enabled.include?(provider_name.to_sym)
|
||||
end
|
||||
|
||||
# Returns the authentication path for the given provider
|
||||
def self.authentication_path(provider_name, params = {})
|
||||
Rails.application.routes.url_helpers.public_send(
|
||||
"user_#{provider_name}_omniauth_authorize_path", params
|
||||
)
|
||||
end
|
||||
|
||||
# Returns the sign in URL for the given provider
|
||||
def self.sign_in_path(provider_name, params = {})
|
||||
url_helpers = Rails.application.routes.url_helpers
|
||||
|
||||
callback_url_helper = "user_#{provider_name}_omniauth_callback_path"
|
||||
mandatory_params = {
|
||||
callback_url: URL.url(url_helpers.public_send(callback_url_helper))
|
||||
}
|
||||
|
||||
authentication_path(provider_name, params.merge(mandatory_params))
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -29,6 +29,13 @@ module Authentication
|
|||
OFFICIAL_NAME
|
||||
end
|
||||
|
||||
def self.sign_in_path(params = {})
|
||||
::Authentication::Paths.sign_in_path(
|
||||
provider_name,
|
||||
params,
|
||||
)
|
||||
end
|
||||
|
||||
protected
|
||||
|
||||
def cleanup_payload(auth_payload)
|
||||
|
|
|
|||
|
|
@ -41,11 +41,23 @@ module Authentication
|
|||
self.class::USERNAME_FIELD
|
||||
end
|
||||
|
||||
def self.provider_name
|
||||
name.demodulize.downcase.to_sym
|
||||
end
|
||||
|
||||
# Returns the official name of the authentication provider
|
||||
def self.official_name
|
||||
raise SubclassResponsibility
|
||||
end
|
||||
|
||||
def self.authentication_path(params = {})
|
||||
::Authentication::Paths.authentication_path(provider_name, params)
|
||||
end
|
||||
|
||||
def self.sign_in_path(_params = {})
|
||||
raise SubclassResponsibility
|
||||
end
|
||||
|
||||
protected
|
||||
|
||||
# Remove sensible data from the payload
|
||||
|
|
|
|||
|
|
@ -34,6 +34,16 @@ module Authentication
|
|||
OFFICIAL_NAME
|
||||
end
|
||||
|
||||
def self.sign_in_path(params = {})
|
||||
# see https://github.com/arunagw/omniauth-twitter#authentication-options
|
||||
mandatory_params = { secure_image_url: true }
|
||||
|
||||
::Authentication::Paths.sign_in_path(
|
||||
provider_name,
|
||||
params.merge(mandatory_params),
|
||||
)
|
||||
end
|
||||
|
||||
protected
|
||||
|
||||
def cleanup_payload(auth_payload)
|
||||
|
|
|
|||
|
|
@ -1,8 +1,6 @@
|
|||
<% Authentication::Providers.enabled.each do |provider_name| %>
|
||||
<% provider = Authentication::Providers.get!(provider_name) %>
|
||||
|
||||
<% authentication_enabled_providers.each do |provider| %>
|
||||
<a
|
||||
href="<%= Authentication::Providers.sign_in_path(provider_name, state: "navbar_basic") %>"
|
||||
href="<%= provider.sign_in_path(state: "navbar_basic") %>"
|
||||
class="crayons-link crayons-link--block pl-5"
|
||||
data-no-instant>
|
||||
Via <%= provider.official_name %>
|
||||
|
|
|
|||
|
|
@ -1,10 +1,8 @@
|
|||
<% Authentication::Providers.enabled.each do |provider_name| %>
|
||||
<% provider = Authentication::Providers.get!(provider_name) %>
|
||||
|
||||
<% authentication_enabled_providers.each do |provider| %>
|
||||
<a
|
||||
href="<%= Authentication::Providers.sign_in_path(provider_name, state: "join-club-page_basic") %>"
|
||||
href="<%= provider.sign_in_path(state: "join-club-page_basic") %>"
|
||||
class="sign-up-link"
|
||||
data-no-instant>
|
||||
<%= inline_svg_tag("#{provider_name}-logo.svg", class: "icon-img", aria: true, title: "#{provider_name} logo") %> Sign In with <%= provider.official_name %>
|
||||
<%= inline_svg_tag("#{provider.provider_name}-logo.svg", class: "icon-img", aria: true, title: "#{provider.name} logo") %> Sign In with <%= provider.official_name %>
|
||||
</a>
|
||||
<% end %>
|
||||
|
|
|
|||
|
|
@ -1,8 +1,6 @@
|
|||
<% Authentication::Providers.enabled.each do |provider_name| %>
|
||||
<% provider = Authentication::Providers.get!(provider_name) %>
|
||||
|
||||
<% authentication_enabled_providers.each do |provider| %>
|
||||
<a
|
||||
href="<%= Authentication::Providers.sign_in_path(provider_name, state: "navbar_basic") %>"
|
||||
href="<%= provider.sign_in_path(state: "navbar_basic") %>"
|
||||
class="cta cta-button login-cta-button"
|
||||
data-no-instant>
|
||||
Sign In With <%= provider.official_name %>
|
||||
|
|
|
|||
|
|
@ -1,13 +1,11 @@
|
|||
<% Authentication::Providers.enabled.each do |provider_name| %>
|
||||
<% provider = Authentication::Providers.get!(provider_name) %>
|
||||
|
||||
<% authentication_enabled_providers.each do |provider| %>
|
||||
<a
|
||||
href="<%= Authentication::Providers.sign_in_path(provider_name, state: "signup-modal") %>"
|
||||
href="<%= provider.sign_in_path(state: "signup-modal") %>"
|
||||
class="sign-up-link cta"
|
||||
data-no-instant>
|
||||
<img
|
||||
src="<%= asset_path("#{provider_name}-logo.svg") %>"
|
||||
src="<%= asset_path("#{provider.provider_name}-logo.svg") %>"
|
||||
class="icon-img"
|
||||
alt="<%= provider.official_name %> logo" /> Auth With <%= provider.official_name %>
|
||||
alt="<%= provider.official_name %> logo" /> Sign In With <%= provider.official_name %>
|
||||
</a>
|
||||
<% end %>
|
||||
|
|
|
|||
|
|
@ -1,15 +1,14 @@
|
|||
<% Authentication::Providers.enabled.each do |provider_name| %>
|
||||
<% unless @user.identities.exists?(provider: provider_name) %>
|
||||
<% provider = Authentication::Providers.get!(provider_name) %>
|
||||
<% authentication_enabled_providers.each do |provider| %>
|
||||
<% unless @user.identities.exists?(provider: provider.provider_name) %>
|
||||
<a
|
||||
href="<%= Authentication::Providers.authentication_path(provider_name) %>"
|
||||
href="<%= provider.authentication_path %>"
|
||||
class="crayons-btn crayons-btn--outlined mr-2"
|
||||
data-no-instant>
|
||||
<%= inline_svg_tag(
|
||||
"#{provider_name}.svg",
|
||||
"#{provider.provider_name}.svg",
|
||||
aria: true,
|
||||
class: "crayons-icon",
|
||||
title: provider_name.to_s.titleize.to_s,
|
||||
title: provider.official_name,
|
||||
) %>
|
||||
Connect <%= provider.official_name %> Account
|
||||
</a>
|
||||
|
|
|
|||
|
|
@ -235,6 +235,8 @@ Devise.setup do |config|
|
|||
# Add a new OmniAuth provider. Check the wiki for more information on setting
|
||||
# up on your models and hooks.
|
||||
# config.omniauth :github, 'APP_ID', 'APP_SECRET', scope: 'user,public_repo'
|
||||
config.omniauth :github, ApplicationConfig["GITHUB_KEY"], ApplicationConfig["GITHUB_SECRET"], scope: "user:email"
|
||||
config.omniauth :twitter, ApplicationConfig["TWITTER_KEY"], ApplicationConfig["TWITTER_SECRET"]
|
||||
|
||||
# ==> Warden configuration
|
||||
# If you want to use other strategies, that are not supported by Devise, or
|
||||
|
|
@ -258,8 +260,4 @@ Devise.setup do |config|
|
|||
# When using OmniAuth, Devise cannot automatically set OmniAuth path,
|
||||
# so you need to do it manually. For the users scope, it would be:
|
||||
# config.omniauth_path_prefix = '/my_engine/users/auth'
|
||||
|
||||
config.omniauth :github,
|
||||
ApplicationConfig["GITHUB_KEY"], ApplicationConfig["GITHUB_SECRET"], scope: "user:email"
|
||||
config.omniauth :twitter, ApplicationConfig["TWITTER_KEY"], ApplicationConfig["TWITTER_SECRET"]
|
||||
end
|
||||
|
|
|
|||
|
|
@ -1,5 +1,3 @@
|
|||
# config/initializers/monkey_patches/persistent_csrf_token_cookie.rb
|
||||
#
|
||||
# Workaround for CSRF protection bug.
|
||||
#
|
||||
# https://github.com/rails/rails/issues/21948
|
||||
|
|
|
|||
|
|
@ -10,6 +10,11 @@ Rails.application.routes.draw do
|
|||
registrations: "registrations"
|
||||
}
|
||||
|
||||
devise_scope :user do
|
||||
get "/enter", to: "registrations#new", as: :sign_up
|
||||
delete "/sign_out", to: "devise/sessions#destroy"
|
||||
end
|
||||
|
||||
require "sidekiq/web"
|
||||
require "sidekiq_unique_jobs/web"
|
||||
|
||||
|
|
@ -23,11 +28,6 @@ Rails.application.routes.draw do
|
|||
mount FieldTest::Engine, at: "abtests"
|
||||
end
|
||||
|
||||
devise_scope :user do
|
||||
delete "/sign_out" => "devise/sessions#destroy"
|
||||
get "/enter" => "registrations#new", :as => :sign_up
|
||||
end
|
||||
|
||||
namespace :admin do
|
||||
# Check administrate gem docs
|
||||
DashboardManifest::DASHBOARDS.each do |dashboard_resource|
|
||||
|
|
|
|||
|
|
@ -113,7 +113,7 @@ RSpec.describe BadgeRewarder, type: :labor do
|
|||
end
|
||||
|
||||
before do
|
||||
mock_github
|
||||
omniauth_mock_github_payload
|
||||
allow(Octokit::Client).to receive(:new).and_return(my_ocktokit_client)
|
||||
allow(my_ocktokit_client).to receive(:commits).and_return(stubbed_github_commit)
|
||||
create(:badge, title: "DEV Contributor")
|
||||
|
|
|
|||
|
|
@ -4,7 +4,7 @@ RSpec.describe GithubRepo, type: :model do
|
|||
let(:user) { create(:user, :with_identity, identities: ["github"]) }
|
||||
let(:repo) { build(:github_repo, user_id: user.id) }
|
||||
|
||||
before { mock_github }
|
||||
before { omniauth_mock_github_payload }
|
||||
|
||||
it { is_expected.to validate_presence_of(:name) }
|
||||
it { is_expected.to validate_presence_of(:url) }
|
||||
|
|
|
|||
|
|
@ -27,7 +27,7 @@ RSpec.describe Identity, type: :model do
|
|||
let(:user) { create(:user) }
|
||||
|
||||
before do
|
||||
mock_auth_hash
|
||||
omniauth_mock_providers_payload
|
||||
end
|
||||
|
||||
context "with Github payload" do
|
||||
|
|
|
|||
|
|
@ -15,7 +15,7 @@ RSpec.describe User, type: :model do
|
|||
let(:user_with_user_optional_fields) { create(:user, :with_user_optional_fields) }
|
||||
let(:org) { create(:organization) }
|
||||
|
||||
before { mock_auth_hash }
|
||||
before { omniauth_mock_providers_payload }
|
||||
|
||||
describe "validations" do
|
||||
describe "builtin validations" do
|
||||
|
|
|
|||
|
|
@ -67,7 +67,7 @@ RSpec.configure do |config|
|
|||
config.include Devise::Test::IntegrationHelpers, type: :system
|
||||
config.include Devise::Test::IntegrationHelpers, type: :request
|
||||
config.include FactoryBot::Syntax::Methods
|
||||
config.include OmniauthMacros
|
||||
config.include OmniauthHelpers
|
||||
config.include SidekiqTestHelpers
|
||||
config.include ElasticsearchHelpers
|
||||
|
||||
|
|
|
|||
|
|
@ -19,7 +19,7 @@ RSpec.describe "GithubRepos", type: :request do
|
|||
end
|
||||
|
||||
before do
|
||||
mock_github
|
||||
omniauth_mock_github_payload
|
||||
allow(Octokit::Client).to receive(:new).and_return(my_octokit_client)
|
||||
allow(my_octokit_client).to receive(:repositories) { stubbed_github_repos }
|
||||
end
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ require "rails_helper"
|
|||
|
||||
RSpec.describe "internal/users", type: :request do
|
||||
let!(:user) do
|
||||
mock_github
|
||||
omniauth_mock_github_payload
|
||||
create(:user, :with_identity, identities: ["github"])
|
||||
end
|
||||
let(:admin) { create(:user, :super_admin) }
|
||||
|
|
|
|||
|
|
@ -15,10 +15,8 @@ RSpec.describe "Registrations", type: :request do
|
|||
it "redirects to /dashboard" do
|
||||
sign_in user
|
||||
|
||||
Timecop.freeze(Time.current) do
|
||||
get "/enter"
|
||||
expect(response).to redirect_to("/dashboard?signed-in-already&t=#{Time.current.to_i}")
|
||||
end
|
||||
get "/enter"
|
||||
expect(response).to redirect_to("/dashboard")
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
@ -79,7 +79,7 @@ RSpec.describe "UserSettings", type: :request do
|
|||
|
||||
describe "connect providers accounts" do
|
||||
before do
|
||||
mock_auth_hash
|
||||
omniauth_mock_providers_payload
|
||||
end
|
||||
|
||||
it "does not render the text for the enabled provider the user has an identity for" do
|
||||
|
|
@ -296,7 +296,7 @@ RSpec.describe "UserSettings", type: :request do
|
|||
let(:user) { create(:user, :with_identity, identities: %w[github twitter]) }
|
||||
|
||||
before do
|
||||
mock_auth_hash
|
||||
omniauth_mock_providers_payload
|
||||
sign_in user
|
||||
end
|
||||
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
require "rails_helper"
|
||||
|
||||
RSpec.describe Authentication::Authenticator, type: :service do
|
||||
before { mock_auth_hash }
|
||||
before { omniauth_mock_providers_payload }
|
||||
|
||||
context "when authenticating through an unknown provider" do
|
||||
it "raises ProviderNotFound" do
|
||||
|
|
|
|||
35
spec/services/authentication/providers/github_spec.rb
Normal file
35
spec/services/authentication/providers/github_spec.rb
Normal file
|
|
@ -0,0 +1,35 @@
|
|||
require "rails_helper"
|
||||
|
||||
RSpec.describe Authentication::Providers::Github, type: :service do
|
||||
describe ".authentication_path" do
|
||||
it "returns the correct authentication path" do
|
||||
expected_path = Rails.application.routes.url_helpers.user_github_omniauth_authorize_path
|
||||
expect(described_class.authentication_path).to eq(expected_path)
|
||||
end
|
||||
|
||||
it "supports additional parameters" do
|
||||
path = described_class.authentication_path(state: "state")
|
||||
expect(path).to include("state=state")
|
||||
end
|
||||
end
|
||||
|
||||
describe ".sign_in_path" do
|
||||
let(:expected_path) do
|
||||
"/users/auth/github?callback_url=http%3A%2F%2Flocalhost%3A3000%2Fusers%2Fauth%2Fgithub%2Fcallback"
|
||||
end
|
||||
|
||||
it "returns the correct sign in path" do
|
||||
expect(described_class.sign_in_path).to eq(expected_path)
|
||||
end
|
||||
|
||||
it "supports additional parameters" do
|
||||
path = described_class.sign_in_path(state: "state")
|
||||
expect(path).to include("state=state")
|
||||
end
|
||||
|
||||
it "does not override the callback_url parameter" do
|
||||
path = described_class.sign_in_path(callback_url: "https://example.com/callback")
|
||||
expect(path).to eq(expected_path)
|
||||
end
|
||||
end
|
||||
end
|
||||
40
spec/services/authentication/providers/twitter_spec.rb
Normal file
40
spec/services/authentication/providers/twitter_spec.rb
Normal file
|
|
@ -0,0 +1,40 @@
|
|||
require "rails_helper"
|
||||
|
||||
RSpec.describe Authentication::Providers::Twitter, type: :service do
|
||||
describe ".authentication_path" do
|
||||
it "returns the correct authentication path" do
|
||||
expected_path = Rails.application.routes.url_helpers.user_twitter_omniauth_authorize_path
|
||||
expect(described_class.authentication_path).to eq(expected_path)
|
||||
end
|
||||
|
||||
it "supports additional parameters" do
|
||||
path = described_class.authentication_path(state: "state")
|
||||
expect(path).to include("state=state")
|
||||
end
|
||||
end
|
||||
|
||||
describe ".sign_in_path" do
|
||||
let(:expected_path) do
|
||||
"/users/auth/twitter?callback_url=http%3A%2F%2Flocalhost%3A3000%2Fusers%2Fauth%2Ftwitter%2Fcallback&secure_image_url=true"
|
||||
end
|
||||
|
||||
it "returns the correct sign in path" do
|
||||
expect(described_class.sign_in_path).to eq(expected_path)
|
||||
end
|
||||
|
||||
it "supports additional parameters" do
|
||||
path = described_class.sign_in_path(state: "state")
|
||||
expect(path).to include("state=state")
|
||||
end
|
||||
|
||||
it "does not override the callback_url parameter" do
|
||||
path = described_class.sign_in_path(callback_url: "https://example.com/callback")
|
||||
expect(path).to eq(expected_path)
|
||||
end
|
||||
|
||||
it "does not override the secure_image_url parameter" do
|
||||
path = described_class.sign_in_path(secure_image_url: "https://dummyimage.com/300")
|
||||
expect(path).to eq(expected_path)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
@ -58,36 +58,4 @@ RSpec.describe Authentication::Providers, type: :service do
|
|||
expect(described_class.enabled?(:github)).to be(false)
|
||||
end
|
||||
end
|
||||
|
||||
describe ".authentication_path" do
|
||||
it "returns the correct authentication path for given provider" do
|
||||
expected_path = Rails.application.routes.url_helpers.user_github_omniauth_authorize_path
|
||||
expect(described_class.authentication_path(:github)).to eq(expected_path)
|
||||
end
|
||||
|
||||
it "supports additional parameters" do
|
||||
path = described_class.authentication_path(:github, state: "state")
|
||||
expect(path).to include("state=state")
|
||||
end
|
||||
end
|
||||
|
||||
describe ".sign_in_path" do
|
||||
let(:expected_path) do
|
||||
"/users/auth/github?callback_url=http%3A%2F%2Flocalhost%3A3000%2Fusers%2Fauth%2Fgithub%2Fcallback"
|
||||
end
|
||||
|
||||
it "returns the correct sign in URL for given provider" do
|
||||
expect(described_class.sign_in_path(:github)).to eq(expected_path)
|
||||
end
|
||||
|
||||
it "supports additional parameters" do
|
||||
path = described_class.sign_in_path(:github, state: "state")
|
||||
expect(path).to include("state=state")
|
||||
end
|
||||
|
||||
it "does not override the callback_url parameter" do
|
||||
path = described_class.sign_in_path(:github, callback_url: "https://example.com/callback")
|
||||
expect(path).to eq(expected_path)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -13,7 +13,7 @@ RSpec.describe Broadcasts::WelcomeNotification::Generator, type: :service do
|
|||
let_it_be_readonly(:customize_ux_broadcast) { create(:customize_ux_broadcast) }
|
||||
|
||||
before do
|
||||
mock_auth_hash
|
||||
omniauth_mock_providers_payload
|
||||
allow(Notification).to receive(:send_welcome_notification).and_call_original
|
||||
allow(User).to receive(:mascot_account).and_return(mascot_account)
|
||||
SiteConfig.staff_user_id = mascot_account.id
|
||||
|
|
@ -143,7 +143,7 @@ RSpec.describe Broadcasts::WelcomeNotification::Generator, type: :service do
|
|||
|
||||
describe "#send_feed_customization_notification" do
|
||||
let!(:user) do
|
||||
mock_auth_hash
|
||||
omniauth_mock_providers_payload
|
||||
create(:user, :with_identity, identities: %w[twitter github], created_at: 3.days.ago)
|
||||
end
|
||||
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
require "rails_helper"
|
||||
|
||||
RSpec.describe Users::Delete, type: :service do
|
||||
before { mock_github }
|
||||
before { omniauth_mock_github_payload }
|
||||
|
||||
let(:user) { create(:user, :with_identity, identities: ["github"]) }
|
||||
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
module OmniauthMacros
|
||||
module OmniauthHelpers
|
||||
OMNIAUTH_DEFAULT_FAILURE_HANDLER = OmniAuth.config.on_failure
|
||||
|
||||
INFO = OmniAuth::AuthHash::InfoHash.new(
|
||||
OMNIAUTH_INFO = OmniAuth::AuthHash::InfoHash.new(
|
||||
first_name: "fname",
|
||||
last_name: "lname",
|
||||
location: "location,state,country",
|
||||
|
|
@ -11,8 +11,8 @@ module OmniauthMacros
|
|||
verified: true,
|
||||
)
|
||||
|
||||
EXTRA_INFO = Hashie::Mash.new(
|
||||
raw_info: Hashie::Mash.new(
|
||||
OMNIAUTH_EXTRA_INFO = OmniAuth::AuthHash::InfoHash.new(
|
||||
raw_info: OmniAuth::AuthHash::InfoHash.new(
|
||||
email: "yourname@email.com",
|
||||
first_name: "fname",
|
||||
gender: "female",
|
||||
|
|
@ -32,23 +32,21 @@ module OmniauthMacros
|
|||
),
|
||||
)
|
||||
|
||||
CREDENTIAL = OmniAuth::AuthHash::InfoHash.new(
|
||||
token: "2735246777-jlOnuFlGlvybuwDJfyrIyESLUEgoo6CffyJCQUO",
|
||||
secret: "o0cu6ACtypMQfLyWhme3Vj99uSds7rjr4szuuTiykSYcN",
|
||||
)
|
||||
|
||||
BASIC_INFO = {
|
||||
OMNIAUTH_BASIC_INFO = {
|
||||
uid: "1234567",
|
||||
info: INFO,
|
||||
extra: EXTRA_INFO,
|
||||
credentials: CREDENTIAL
|
||||
info: OMNIAUTH_INFO,
|
||||
extra: OMNIAUTH_EXTRA_INFO,
|
||||
credentials: {
|
||||
token: SecureRandom.hex,
|
||||
secret: SecureRandom.hex
|
||||
}
|
||||
}.freeze
|
||||
|
||||
def mock_auth_with_invalid_credentials(provider)
|
||||
def omniauth_setup_invalid_credentials(provider)
|
||||
OmniAuth.config.mock_auth[provider] = :invalid_credentials
|
||||
end
|
||||
|
||||
def setup_omniauth_error(error)
|
||||
def omniauth_setup_authentication_error(error)
|
||||
# this hack is needed due to a limitation in how OmniAuth handles
|
||||
# failures in mocked/testing environments,
|
||||
# see <https://github.com/omniauth/omniauth/issues/654#issuecomment-610851884>
|
||||
|
|
@ -82,39 +80,46 @@ module OmniauthMacros
|
|||
]
|
||||
end
|
||||
|
||||
def mock_auth_hash
|
||||
mock_twitter
|
||||
mock_github
|
||||
def omniauth_mock_providers_payload
|
||||
Authentication::Providers.available.each do |provider_name|
|
||||
public_send("omniauth_mock_#{provider_name}_payload")
|
||||
end
|
||||
end
|
||||
|
||||
def mock_twitter
|
||||
info = BASIC_INFO[:info].merge(
|
||||
def omniauth_reset_mock
|
||||
Authentication::Providers.available.each do |provider_name|
|
||||
OmniAuth.config.mock_auth[provider_name] = nil
|
||||
end
|
||||
end
|
||||
|
||||
def omniauth_mock_github_payload
|
||||
info = OMNIAUTH_BASIC_INFO[:info].merge(
|
||||
image: "https://dummyimage.com/400x400.jpg",
|
||||
)
|
||||
|
||||
OmniAuth.config.mock_auth[:github] = OmniAuth::AuthHash.new(
|
||||
OMNIAUTH_BASIC_INFO.merge(
|
||||
provider: "github",
|
||||
info: info,
|
||||
),
|
||||
)
|
||||
end
|
||||
|
||||
def omniauth_mock_twitter_payload
|
||||
info = OMNIAUTH_BASIC_INFO[:info].merge(
|
||||
image: "https://dummyimage.com/400x400_normal.jpg",
|
||||
)
|
||||
|
||||
extra = BASIC_INFO[:extra].merge(
|
||||
extra = OMNIAUTH_BASIC_INFO[:extra].merge(
|
||||
access_token: "value",
|
||||
)
|
||||
|
||||
OmniAuth.config.mock_auth[:twitter] = OmniAuth::AuthHash.new(
|
||||
BASIC_INFO.merge(
|
||||
OMNIAUTH_BASIC_INFO.merge(
|
||||
provider: "twitter",
|
||||
info: info,
|
||||
extra: extra,
|
||||
),
|
||||
)
|
||||
end
|
||||
|
||||
def mock_github
|
||||
info = BASIC_INFO[:info].merge(
|
||||
image: "https://dummyimage.com/400x400.jpg",
|
||||
)
|
||||
|
||||
OmniAuth.config.mock_auth[:github] = OmniAuth::AuthHash.new(
|
||||
BASIC_INFO.merge(
|
||||
provider: "github",
|
||||
info: info,
|
||||
),
|
||||
)
|
||||
end
|
||||
end
|
||||
|
|
@ -3,7 +3,7 @@ require "rails_helper"
|
|||
RSpec.describe "Authenticating with GitHub" do
|
||||
let(:sign_in_link) { "Sign In With GitHub" }
|
||||
|
||||
before { mock_github }
|
||||
before { omniauth_mock_github_payload }
|
||||
|
||||
context "when a user is new" do
|
||||
context "when using valid credentials" do
|
||||
|
|
@ -54,13 +54,13 @@ RSpec.describe "Authenticating with GitHub" do
|
|||
end
|
||||
|
||||
before do
|
||||
mock_auth_with_invalid_credentials(:github)
|
||||
omniauth_setup_invalid_credentials(:github)
|
||||
|
||||
allow(DatadogStatsClient).to receive(:increment)
|
||||
end
|
||||
|
||||
after do
|
||||
OmniAuth.config.on_failure = OmniauthMacros.const_get("OMNIAUTH_DEFAULT_FAILURE_HANDLER")
|
||||
OmniAuth.config.on_failure = OmniauthHelpers.const_get("OMNIAUTH_DEFAULT_FAILURE_HANDLER")
|
||||
end
|
||||
|
||||
it "does not create a new user" do
|
||||
|
|
@ -85,7 +85,7 @@ RSpec.describe "Authenticating with GitHub" do
|
|||
"Callback error", "Error reason", "https://example.com/error"
|
||||
)
|
||||
|
||||
setup_omniauth_error(error)
|
||||
omniauth_setup_authentication_error(error)
|
||||
|
||||
visit root_path
|
||||
click_link sign_in_link
|
||||
|
|
@ -101,7 +101,7 @@ RSpec.describe "Authenticating with GitHub" do
|
|||
allow(request).to receive(:code).and_return(401)
|
||||
allow(request).to receive(:message).and_return("unauthorized")
|
||||
error = OAuth::Unauthorized.new(request)
|
||||
setup_omniauth_error(error)
|
||||
omniauth_setup_authentication_error(error)
|
||||
|
||||
visit root_path
|
||||
click_link sign_in_link
|
||||
|
|
@ -114,7 +114,7 @@ RSpec.describe "Authenticating with GitHub" do
|
|||
|
||||
it "notifies Datadog even with no OmniAuth error present" do
|
||||
error = nil
|
||||
setup_omniauth_error(error)
|
||||
omniauth_setup_authentication_error(error)
|
||||
|
||||
visit root_path
|
||||
click_link sign_in_link
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@ require "rails_helper"
|
|||
RSpec.describe "Authenticating with Twitter" do
|
||||
let(:sign_in_link) { "Sign In With Twitter" }
|
||||
|
||||
before { mock_twitter }
|
||||
before { omniauth_mock_twitter_payload }
|
||||
|
||||
context "when a user is new" do
|
||||
context "when using valid credentials" do
|
||||
|
|
@ -50,13 +50,13 @@ RSpec.describe "Authenticating with Twitter" do
|
|||
|
||||
context "when using invalid credentials" do
|
||||
before do
|
||||
mock_auth_with_invalid_credentials(:twitter)
|
||||
omniauth_setup_invalid_credentials(:twitter)
|
||||
|
||||
allow(DatadogStatsClient).to receive(:increment)
|
||||
end
|
||||
|
||||
after do
|
||||
OmniAuth.config.on_failure = OmniauthMacros.const_get("OMNIAUTH_DEFAULT_FAILURE_HANDLER")
|
||||
OmniAuth.config.on_failure = OmniauthHelpers.const_get("OMNIAUTH_DEFAULT_FAILURE_HANDLER")
|
||||
end
|
||||
|
||||
it "does not create a new user" do
|
||||
|
|
@ -81,7 +81,7 @@ RSpec.describe "Authenticating with Twitter" do
|
|||
"Callback error", "Error reason", "https://example.com/error"
|
||||
)
|
||||
|
||||
setup_omniauth_error(error)
|
||||
omniauth_setup_authentication_error(error)
|
||||
|
||||
visit root_path
|
||||
click_link sign_in_link
|
||||
|
|
@ -97,7 +97,7 @@ RSpec.describe "Authenticating with Twitter" do
|
|||
allow(request).to receive(:code).and_return(401)
|
||||
allow(request).to receive(:message).and_return("unauthorized")
|
||||
error = OAuth::Unauthorized.new(request)
|
||||
setup_omniauth_error(error)
|
||||
omniauth_setup_authentication_error(error)
|
||||
|
||||
visit root_path
|
||||
click_link sign_in_link
|
||||
|
|
@ -110,7 +110,7 @@ RSpec.describe "Authenticating with Twitter" do
|
|||
|
||||
it "notifies Datadog even with no OmniAuth error present" do
|
||||
error = nil
|
||||
setup_omniauth_error(error)
|
||||
omniauth_setup_authentication_error(error)
|
||||
|
||||
visit root_path
|
||||
click_link sign_in_link
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue