maverick/docbrown
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

[deploy] Authentication refactoring - step 1 (#7259)

Browse source 
* Add Authentication providers

* Making strides on refactoring

* New usr from Twitter

* Existing Twitter user

* Existing user Github

* Test skip_confirmation!

* Refactor a bit and add simplecov groups

* Test user already logged in

* Rename to Authentication::Authenticator

* Refactor to a common class

* Add existing user test for twitter login

* Refactor global handler

* Fix spec and comment Twitter provider

* Update comments and strategy TODO

* Move providers loading code to Authentication::Providers

* add SubclassResponsibility error

* Use delegation
This commit is contained in:
rhymes 2020-04-15 18:13:10 +02:00 • committed by GitHub
parent b054b06cd9
commit d6e47a03c7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
29 changed files with 1082 additions and 402 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
.rubocop.yml
View file

@ -382,6 +382,8 @@ RSpec/DescribeClass:
Description: Check that the first argument to the top level describe is a constant.
Enabled: true
StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/DescribeClass
Exclude:
- 'spec/system/**/*'
RSpec/ExampleLength:
Description: Checks for long examples.

7
app/controllers/omniauth_callbacks_controller.rb
View file

@ -35,9 +35,14 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
private
# TODO: [thepracticaldev/oss] test all of this
def callback_for(provider)
cta_variant = request.env["omniauth.params"]["state"].to_s
@user = AuthorizationService.new(request.env["omniauth.auth"], current_user, cta_variant).get_user
@user = Authentication::Authenticator.call(
request.env["omniauth.auth"],
current_user: current_user,
cta_variant: cta_variant,
)
if persisted_and_valid?
# delete legacy session based cookie once the user has logged in again

4
app/errors/subclass_responsibility.rb Normal file
View file

@ -0,0 +1,4 @@
# raised to signal that the implementation of a method is responsibility
# of the subclass
class SubclassResponsibility < StandardError
end

21
app/models/identity.rb
View file

@ -1,14 +1,31 @@
class Identity < ApplicationRecord
belongs_to :user
has_many :backup_data, as: :instance, class_name: "BackupData", dependent: :destroy
validates :uid, :provider, presence: true
validates :uid, uniqueness: { scope: :provider }, if: proc { |identity| identity.uid_changed? || identity.provider_changed? }
validates :user_id, uniqueness: { scope: :provider }, if: proc { |identity| identity.user_id_changed? || identity.provider_changed? }
# TODO: [thepracticaldev/oss] put the providers somewhere else
validates :provider, inclusion: { in: %w[github twitter] }
# TODO: [thepracticaldev/oss] should this be transitioned to JSON?
serialize :auth_data_dump
def self.find_for_oauth(auth)
find_or_create_by(uid: auth.uid, provider: auth.provider)
# Builds an identity from OmniAuth's authentication payload
def self.build_from_omniauth(provider)
payload = provider.payload
identity = find_or_initialize_by(
provider: payload.provider,
uid: payload.uid,
)
identity.assign_attributes(
token: payload.credentials.token,
secret: payload.credentials.secret,
auth_data_dump: payload,
)
identity
end
end

130
app/services/authentication/authenticator.rb Normal file
View file

@ -0,0 +1,130 @@
module Authentication
# TODO: [thepracticaldev/oss] use strategy pattern for the three cases
# described below.
# Make the decision early which one of the 3 cases we're dealing with
# and then call either NewUserStrategy, UpdateUserStrategy or
# LoggedInUserStrategy. I think the resulting three classes would be much
# easier to understand and they can still share methods by inheriting
# from a basic AuthStrategy.
# Authenticator will perform one of these tree operations:
# 1. create a new user and match it to its authentication identity
# 2. update an existing user and align it to its authentication identity
# 3. return the current user if a user is given (already logged in scenario)
class Authenticator
# auth_payload is the payload schema, see https://github.com/omniauth/omniauth/wiki/Auth-Hash-Schema
def initialize(auth_payload, current_user: nil, cta_variant: nil)
@provider = load_authentication_provider(auth_payload)
@current_user = current_user
@cta_variant = cta_variant
end
def self.call(*args)
new(*args).call
end
def call
identity = Identity.build_from_omniauth(provider)
return current_user if current_user_identity_exists?
user = proper_user(identity)
user = if user.nil?
build_user
else
update_user(user)
end
save_identity(identity, user)
user.skip_confirmation!
flag_spam_user(user) if account_less_than_a_week_old?(user, identity)
user.save!
user
end
private
attr_reader :provider, :current_user, :cta_variant
# Loads the proper authentication provider from the available ones
def load_authentication_provider(auth_payload)
provider_class = Authentication::Providers.get!(auth_payload.provider)
provider_class.new(auth_payload)
end
def current_user_identity_exists?
current_user&.identities&.exists?(provider: provider.name)
end
def proper_user(identity)
if current_user
current_user
elsif identity.user
identity.user
elsif provider.user_email.present?
User.find_by(email: provider.user_email)
end
end
def build_user
existing_user = User.where(
provider.user_username_field => provider.user_nickname,
).take
return existing_user if existing_user
User.new.tap do |user|
user.assign_attributes(provider.new_user_data)
user.assign_attributes(default_user_fields)
user.set_remember_fields
end
end
def default_user_fields
{
password: Devise.friendly_token(20),
signup_cta_variant: cta_variant,
saw_onboarding: false,
editor_version: :v2
}
end
def update_user(user)
user.tap do |model|
user.assign_attributes(provider.existing_user_data)
update_profile_updated_at(model)
model.set_remember_fields
end
end
def update_profile_updated_at(user)
field_name = "#{provider.user_username_field}_changed?"
user.profile_updated_at = Time.current if user.public_send(field_name)
end
def save_identity(identity, user)
identity.user = user if identity.user_id.blank?
identity.save!
end
def account_less_than_a_week_old?(user, logged_in_identity)
provider_created_at = user.public_send(provider.user_created_at_field)
user_identity_age = provider_created_at ||
Time.zone.parse(logged_in_identity.auth_data_dump.extra.raw_info.created_at)
# last one is a fallback in case both are nil
range = 1.week.ago.beginning_of_day..Time.current
range.cover?(user_identity_age)
end
def flag_spam_user(user)
Slack::Messengers::PotentialSpammer.call(user: user)
end
end
end

9
app/services/authentication/errors.rb Normal file
View file

@ -0,0 +1,9 @@
module Authentication
module Errors
class Error < StandardError
end
class ProviderNotFound < Error
end
end
end

11
app/services/authentication/providers.rb Normal file
View file

@ -0,0 +1,11 @@
module Authentication
module Providers
# TODO: [thepracticaldev/oss] raise exception if provider is available but not enabled for this app
# TODO: [thepracticaldev/oss] add available providers, enabled providers
def self.get!(provider_name)
"Authentication::Providers::#{provider_name.titleize}".constantize
rescue NameError => e
raise ::Authentication::Errors::ProviderNotFound, e
end
end
end

34
app/services/authentication/providers/github.rb Normal file
View file

@ -0,0 +1,34 @@
module Authentication
module Providers
# GitHub authentication provider, uses omniauth-github as backend
class Github < Provider
CREATED_AT_FIELD = "github_created_at".freeze
USERNAME_FIELD = "github_username".freeze
def new_user_data
name = raw_info.name.presence || info.name
{
email: info.email.to_s,
github_created_at: raw_info.created_at,
github_username: info.nickname,
name: name,
remote_profile_image_url: info.image.to_s
}
end
def existing_user_data
{
github_created_at: raw_info.created_at,
github_username: info.nickname
}
end
protected
def cleanup_payload(auth_payload)
auth_payload
end
end
end
end

54
app/services/authentication/providers/provider.rb Normal file
View file

@ -0,0 +1,54 @@
module Authentication
module Providers
# Authentication provider
class Provider
# name of the field to store the upstream identity's creation date in
CREATED_AT_FIELD = "".freeze
# name of the field to store the upstream identity's username in
USERNAME_FIELD = "".freeze
delegate :email, :nickname, to: :info, prefix: :user
def initialize(auth_payload)
@auth_payload = cleanup_payload(auth_payload.dup)
@info = auth_payload.info
@raw_info = auth_payload.extra.raw_info
end
# Extract data for a brand new user
def new_user_data
raise SubclassResponsibility
end
# Extract data for an existing user
def existing_user_data
raise SubclassResponsibility
end
def name
auth_payload.provider
end
def payload
auth_payload
end
def user_created_at_field
self.class::CREATED_AT_FIELD
end
def user_username_field
self.class::USERNAME_FIELD
end
protected
# Remove sensible data from the payload
def cleanup_payload(_auth_payload)
raise SubclassResponsibility
end
attr_reader :auth_payload, :info, :raw_info
end
end
end

43
app/services/authentication/providers/twitter.rb Normal file
View file

@ -0,0 +1,43 @@
module Authentication
module Providers
# Twitter authentication provider, uses omniauth-twitter as backend
class Twitter < Provider
CREATED_AT_FIELD = "twitter_created_at".freeze
USERNAME_FIELD = "twitter_username".freeze
def new_user_data
name = raw_info.name.presence || info.name
remote_profile_image_url = info.image.to_s.gsub("_normal", "")
{
email: info.email.to_s,
name: name,
remote_profile_image_url: remote_profile_image_url,
twitter_created_at: raw_info.created_at,
twitter_followers_count: raw_info.followers_count.to_i,
twitter_following_count: raw_info.friends_count.to_i,
twitter_username: info.nickname
}
end
def existing_user_data
{
twitter_created_at: raw_info.created_at,
twitter_followers_count: raw_info.followers_count.to_i,
twitter_following_count: raw_info.friends_count.to_i,
twitter_username: info.nickname
}
end
protected
def cleanup_payload(auth_payload)
auth_payload.tap do |auth|
# Twitter sends the server side access token keys in the payload
# for each authentication. We definitely do not want to store those
auth.extra.delete("access_token")
end
end
end
end
end

115
app/services/authorization_service.rb
View file

@ -1,115 +0,0 @@
class AuthorizationService
attr_accessor :auth, :signed_in_resource, :cta_variant
def initialize(auth, signed_in_resource = nil, cta_variant = nil)
@auth = auth
@signed_in_resource = signed_in_resource
@cta_variant = cta_variant
end
def get_user
identity = build_identity
return signed_in_resource if user_identity_exists(identity)
user = proper_user(identity)
user = if user.nil?
build_user(identity)
else
update_user(user)
end
set_identity(identity, user)
user.skip_confirmation!
flag_spam_user(user) if account_less_than_a_week_old?(user, identity)
user
end
def add_social_identity_data(user)
return unless auth&.provider && auth&.extra && auth.extra.raw_info
if auth.provider == "twitter"
user.twitter_created_at = auth.extra.raw_info.created_at
user.twitter_followers_count = auth.extra.raw_info.followers_count.to_i
user.twitter_following_count = auth.extra.raw_info.friends_count.to_i
else
user.github_created_at = auth.extra.raw_info.created_at
end
end
def build_identity
identity = Identity.find_for_oauth(auth)
identity.token = auth.credentials.token
identity.secret = auth.credentials.secret
auth["extra"].delete("access_token") if auth["extra"]["access_token"]
identity.auth_data_dump = auth
identity.save
identity
end
def build_user(identity)
user = User.where("#{identity.provider}_username" => auth.info.nickname).first
if user.nil?
user = User.new(
name: auth.extra.raw_info.name,
remote_profile_image_url: (auth.info.image || "").gsub("_normal", ""),
github_username: (auth.info.nickname if auth.provider == "github"),
signup_cta_variant: cta_variant,
email: auth.info.email || "",
twitter_username: (auth.info.nickname if auth.provider == "twitter"),
password: Devise.friendly_token[0, 20],
)
user.name = auth.info.nickname if user.name.blank?
user.skip_confirmation!
user.set_remember_fields
add_social_identity_data(user)
user.saw_onboarding = false
user.editor_version = "v2"
user.save!
end
user
end
def update_user(user)
user.set_remember_fields
user.github_username = auth.info.nickname if auth.provider == "github" && auth.info.nickname != user.github_username
user.twitter_username = auth.info.nickname if auth.provider == "twitter" && auth.info.nickname != user.twitter_username
add_social_identity_data(user)
user.profile_updated_at = Time.current if user.twitter_username_changed? || user.github_username_changed?
user.save
user
end
def proper_user(identity)
if signed_in_resource
signed_in_resource
elsif identity.user
identity.user
elsif auth.info.email.present?
User.find_by(email: auth.info.email)
end
end
def set_identity(identity, user)
return if identity.user_id.present?
identity.user = user
identity.save!
end
def user_identity_exists(identity)
signed_in_resource &&
Identity.where(provider: identity.provider, user_id: signed_in_resource.id).any?
end
def account_less_than_a_week_old?(user, logged_in_identity)
user_identity_age = user.github_created_at ||
user.twitter_created_at ||
Time.zone.parse(logged_in_identity.auth_data_dump.extra.raw_info.created_at)
# last one is a fallback in case both are nil
range = 1.week.ago.beginning_of_day..Time.current
range.cover?(user_identity_age)
end
def flag_spam_user(user)
Slack::Messengers::PotentialSpammer.call(user: user)
end
end

6
spec/factories/users.rb
View file

@ -27,7 +27,11 @@ FactoryBot.define do
after(:create) do |user, options|
options.identities.each do |provider|
create(:identity, user: user, provider: provider)
auth = OmniAuth.config.mock_auth.fetch(provider.to_sym)
create(
:identity,
user: user, provider: provider, uid: auth.uid, auth_data_dump: auth,
)
end
end
end

1
spec/labor/badge_rewarder_spec.rb
View file

@ -113,6 +113,7 @@ RSpec.describe BadgeRewarder, type: :labor do
end
before do
mock_github
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")

2
spec/models/github_repo_spec.rb
View file

@ -4,6 +4,8 @@ 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 }
it { is_expected.to validate_presence_of(:name) }
it { is_expected.to validate_presence_of(:url) }
it { is_expected.to validate_uniqueness_of(:url) }

83
spec/models/identity_spec.rb
View file

@ -16,12 +16,83 @@ RSpec.describe Identity, type: :model do
it { is_expected.to validate_inclusion_of(:provider).in_array(%w[github twitter]) }
it { is_expected.to serialize(:auth_data_dump) }
describe ".find_for_oauth" do
it "works" do
allow(described_class).to receive(:find_or_create_by)
auth = { uid: 0, provider: "github" }
described_class.find_for_oauth(instance_double("request", auth))
expect(described_class).to have_received(:find_or_create_by).with(auth)
describe ".build_build_from_omniauth" do
let(:user) { create(:user) }
before do
mock_auth_hash
end
context "with Github payload" do
let(:auth_payload) { OmniAuth.config.mock_auth[:github] }
let(:provider) { Authentication::Providers::Github.new(auth_payload) }
it "initializes a new identity from the auth payload" do
identity = described_class.build_from_omniauth(provider)
expect(identity.new_record?).to be(true)
expect(identity.provider).to eq("github")
expect(identity.uid).to eq(auth_payload.uid)
expect(identity.token).to eq(auth_payload.credentials.token)
expect(identity.secret).to eq(auth_payload.credentials.secret)
expect(identity.auth_data_dump).to eq(provider.payload)
end
it "finds an existing identity" do
payload = provider.payload
existing_identity = described_class.create!(
user: user,
provider: payload.provider,
uid: payload.uid,
token: payload.credentials.token,
secret: payload.credentials.secret,
auth_data_dump: payload,
)
identity = described_class.build_from_omniauth(provider)
expect(identity).to eq(existing_identity)
end
end
context "with Twitter payload" do
let(:auth_payload) { OmniAuth.config.mock_auth[:twitter] }
let(:provider) { Authentication::Providers::Twitter.new(auth_payload) }
it "initializes a new identity from the auth payload" do
identity = described_class.build_from_omniauth(provider)
expect(identity.new_record?).to be(true)
expect(identity.provider).to eq("twitter")
expect(identity.uid).to eq(auth_payload.uid)
expect(identity.token).to eq(auth_payload.credentials.token)
expect(identity.secret).to eq(auth_payload.credentials.secret)
expect(identity.auth_data_dump).to eq(provider.payload)
end
it "finds an existing identity" do
payload = provider.payload
existing_identity = described_class.create!(
user: user,
provider: payload.provider,
uid: payload.uid,
token: payload.credentials.token,
secret: payload.credentials.secret,
auth_data_dump: payload,
)
identity = described_class.build_from_omniauth(provider)
expect(identity).to eq(existing_identity)
end
it "does not store the access token in auth_data_dump" do
expect(auth_payload.extra.access_token).not_to be_nil
identity = described_class.build_from_omniauth(provider)
expect(identity.auth_data_dump.extra.access_token).to be_nil
end
end
end
end

7
spec/models/user_spec.rb
View file

@ -2,8 +2,11 @@ require "rails_helper"
def user_from_authorization_service(service_name, signed_in_resource, cta_variant)
auth = OmniAuth.config.mock_auth[service_name]
service = AuthorizationService.new(auth, signed_in_resource, cta_variant)
service.get_user
Authentication::Authenticator.call(
auth,
current_user: signed_in_resource,
cta_variant: cta_variant,
)
end
RSpec.describe User, type: :model do

3
spec/rails_helper.rb
View file

@ -124,6 +124,9 @@ RSpec.configure do |config|
stub_request(:any, /api.mailchimp.com/).
to_return(status: 200, body: "", headers: {})
stub_request(:any, /dummyimage.com/).
to_return(status: 200, body: "", headers: {})
end
OmniAuth.config.test_mode = true

1
spec/requests/github_repos_spec.rb
View file

@ -19,6 +19,7 @@ RSpec.describe "GithubRepos", type: :request do
end
before do
mock_github
allow(Octokit::Client).to receive(:new).and_return(my_octokit_client)
allow(my_octokit_client).to receive(:repositories) { stubbed_github_repos }
end

5
spec/requests/internal/users_spec.rb
View file

@ -1,7 +1,10 @@
require "rails_helper"
RSpec.describe "internal/users", type: :request do
let!(:user) { create(:user, :with_identity, identities: ["github"]) }
let!(:user) do
mock_github
create(:user, :with_identity, identities: ["github"])
end
let(:admin) { create(:user, :super_admin) }
before do

5
spec/requests/user/user_settings_spec.rb
View file

@ -265,7 +265,10 @@ RSpec.describe "UserSettings", type: :request do
context "when user has two identities" do
let(:user) { create(:user, :with_identity, identities: %w[github twitter]) }
before { sign_in user }
before do
mock_auth_hash
sign_in user
end
it "brings the identity count to 1" do
delete "/users/remove_association", params: { provider: "twitter" }

354
spec/services/authentication/authenticator_spec.rb Normal file
View file

@ -0,0 +1,354 @@
require "rails_helper"
RSpec.describe Authentication::Authenticator, type: :service do
before { mock_auth_hash }
context "when authenticating through an unknown provider" do
it "raises ProviderNotFound" do
auth_payload = OmniAuth.config.mock_auth[:github].merge(provider: "okta")
expect { described_class.call(auth_payload) }.to raise_error(
Authentication::Errors::ProviderNotFound,
)
end
end
context "when authenticating through Github" do
let!(:auth_payload) { OmniAuth.config.mock_auth[:github] }
let!(:service) { described_class.new(auth_payload) }
describe "new user" do
it "creates a new user" do
expect do
service.call
end.to change(User, :count).by(1)
end
it "creates a new identity" do
expect do
service.call
end.to change(Identity, :count).by(1)
end
it "extracts the proper data from the auth payload" do
user = service.call
info = auth_payload.info
raw_info = auth_payload.extra.raw_info
expect(user.email).to eq(info.email)
expect(user.name).to eq(raw_info.name)
expect(user.remote_profile_image_url).to eq(info.image)
expect(user.github_created_at.to_i).to eq(Time.zone.parse(raw_info.created_at).to_i)
expect(user.github_username).to eq(info.nickname)
end
it "sets default fields" do
user = service.call
expect(user.password).to be_present
expect(user.signup_cta_variant).to be_nil
expect(user.saw_onboarding).to be(false)
expect(user.editor_version).to eq("v2")
end
it "sets the correct sign up cta variant" do
user = described_class.call(auth_payload, cta_variant: "awesome")
expect(user.signup_cta_variant).to eq("awesome")
end
it "sets remember_me for the new user" do
user = service.call
expect(user.remember_me).to be(true)
expect(user.remember_token).to be_present
expect(user.remember_created_at).to be_present
end
it "sets confirmed_at" do
user = service.call
expect(user.confirmed_at).to be_present
end
it "queues a slack message to be sent for a user whose identity is brand new" do
auth_payload.extra.raw_info.created_at = 1.minute.ago.rfc3339
sidekiq_assert_enqueued_with(job: SlackBotPingWorker) do
described_class.call(auth_payload)
end
end
end
describe "existing user" do
let(:user) { create(:user, :with_identity, identities: [:github]) }
before do
auth_payload.info.email = user.email
end
it "doesn't create a new user" do
expect do
service.call
end.not_to change(User, :count)
end
it "creates a new identity if the user doesn't have one" do
user = create(:user)
auth_payload.info.email = user.email
auth_payload.uid = "#{user.email}-#{rand(10_000)}"
expect do
described_class.call(auth_payload)
end.to change(Identity, :count).by(1)
end
it "does not create a new identity if the user has one" do
expect do
service.call
end.not_to change(Identity, :count)
end
it "sets remember_me for the existing user" do
user.update_columns(remember_token: nil, remember_created_at: nil)
service.call
user.reload
expect(user.remember_me).to be(true)
expect(user.remember_token).to be_present
expect(user.remember_created_at).to be_present
end
it "updates confirmed_at with the current UTC time" do
original_confirmed_at = user.confirmed_at
Timecop.travel(1.minute.from_now) do
service.call
end
user.reload
expect(
user.confirmed_at.utc.to_i > original_confirmed_at.utc.to_i,
).to be(true)
end
it "updates the username when it is changed on the provider" do
new_username = "new_username#{rand(1000)}"
auth_payload.info.nickname = new_username
user = described_class.call(auth_payload)
expect(user.github_username).to eq(new_username)
end
it "updates profile_updated_at when the username is changed" do
original_profile_updated_at = user.profile_updated_at
new_username = "new_username#{rand(1000)}"
auth_payload.info.nickname = new_username
Timecop.travel(1.minute.from_now) do
described_class.call(auth_payload)
end
user.reload
expect(
user.profile_updated_at.to_i > original_profile_updated_at.to_i,
).to be(true)
end
end
describe "user already logged in" do
it "returns the current user if the identity exists" do
user = create(:user, :with_identity, identities: [:github])
expect(described_class.call(auth_payload, current_user: user)).to eq(user)
end
it "creates the identity if for any reason it does not exist" do
user = create(:user)
expect do
described_class.call(auth_payload, current_user: user)
end.to change(Identity, :count).by(1)
end
end
end
context "when authenticating through Twitter" do
let!(:auth_payload) { OmniAuth.config.mock_auth[:twitter] }
let!(:service) { described_class.new(auth_payload) }
describe "new user" do
it "creates a new user" do
expect do
service.call
end.to change(User, :count).by(1)
end
it "creates a new identity" do
expect do
service.call
end.to change(Identity, :count).by(1)
end
it "extracts the proper data from the auth payload" do
user = service.call
info = auth_payload.info
raw_info = auth_payload.extra.raw_info
expect(user.email).to eq(info.email)
expect(user.name).to eq(raw_info.name)
expect(user.remote_profile_image_url).to eq(info.image.to_s.gsub("_normal", ""))
expect(user.twitter_created_at.to_i).to eq(Time.zone.parse(raw_info.created_at).to_i)
expect(user.twitter_followers_count).to eq(raw_info.followers_count.to_i)
expect(user.twitter_following_count).to eq(raw_info.friends_count.to_i)
expect(user.twitter_username).to eq(info.nickname)
end
it "sets default fields" do
user = service.call
expect(user.password).to be_present
expect(user.signup_cta_variant).to be_nil
expect(user.saw_onboarding).to be(false)
expect(user.editor_version).to eq("v2")
end
it "sets the correct sign up cta variant" do
user = described_class.call(auth_payload, cta_variant: "awesome")
expect(user.signup_cta_variant).to eq("awesome")
end
it "sets remember_me for the new user" do
user = service.call
expect(user.remember_me).to be(true)
expect(user.remember_token).to be_present
expect(user.remember_created_at).to be_present
end
it "sets confirmed_at" do
user = service.call
expect(user.confirmed_at).to be_present
end
it "queues a slack message to be sent for a user whose identity is brand new" do
auth_payload.extra.raw_info.created_at = 1.minute.ago.rfc3339
sidekiq_assert_enqueued_with(job: SlackBotPingWorker) do
described_class.call(auth_payload)
end
end
end
describe "existing user" do
let(:user) { create(:user, :with_identity, identities: [:twitter]) }
before do
auth_payload.info.email = user.email
end
it "doesn't create a new user" do
expect do
service.call
end.not_to change(User, :count)
end
it "creates a new identity if the user doesn't have one" do
user = create(:user)
auth_payload.info.email = user.email
auth_payload.uid = "#{user.email}-#{rand(10_000)}"
expect do
described_class.call(auth_payload)
end.to change(Identity, :count).by(1)
end
it "does not create a new identity if the user has one" do
expect do
service.call
end.not_to change(Identity, :count)
end
it "updates the proper data from the auth payload" do
# simulate changing twitter data
auth_payload.extra.raw_info.followers_count = rand(100).to_s
auth_payload.extra.raw_info.friends_count = rand(100).to_s
user = described_class.call(auth_payload)
raw_info = auth_payload.extra.raw_info
expect(user.twitter_created_at.to_i).to eq(Time.zone.parse(raw_info.created_at).to_i)
expect(user.twitter_followers_count).to eq(raw_info.followers_count.to_i)
expect(user.twitter_following_count).to eq(raw_info.friends_count.to_i)
end
it "sets remember_me for the existing user" do
user.update_columns(remember_token: nil, remember_created_at: nil)
service.call
user.reload
expect(user.remember_me).to be(true)
expect(user.remember_token).to be_present
expect(user.remember_created_at).to be_present
end
it "updates confirmed_at with the current UTC time" do
original_confirmed_at = user.confirmed_at
Timecop.travel(1.minute.from_now) do
service.call
end
user.reload
expect(
user.confirmed_at.utc.to_i > original_confirmed_at.utc.to_i,
).to be(true)
end
it "updates the username when it is changed on the provider" do
new_username = "new_username#{rand(1000)}"
auth_payload.info.nickname = new_username
user = described_class.call(auth_payload)
expect(user.twitter_username).to eq(new_username)
end
it "updates profile_updated_at when the username is changed" do
original_profile_updated_at = user.profile_updated_at
new_username = "new_username#{rand(1000)}"
auth_payload.info.nickname = new_username
Timecop.travel(1.minute.from_now) do
described_class.call(auth_payload)
end
user.reload
expect(
user.profile_updated_at.to_i > original_profile_updated_at.to_i,
).to be(true)
end
end
describe "user already logged in" do
it "returns the current user if the identity exists" do
user = create(:user, :with_identity, identities: [:twitter])
expect(described_class.call(auth_payload, current_user: user)).to eq(user)
end
it "creates the identity if for any reason it does not exist" do
user = create(:user)
expect do
described_class.call(auth_payload, current_user: user)
end.to change(Identity, :count).by(1)
end
end
end
end

77
spec/services/authorization_service_spec.rb
View file

@ -1,77 +0,0 @@
require "rails_helper"
RSpec.describe AuthorizationService, type: :service do
before { mock_auth_hash }
describe "new user" do
let(:auth) { OmniAuth.config.mock_auth[:github] }
let!(:service) { described_class.new(auth) }
it "creates a new user" do
expect do
service.get_user
end.to change(User, :count).by(1)
end
it "sets remember_me for the new user" do
user = service.get_user
user.reload
expect(user.remember_me).to be_truthy
expect(user.remember_token).to be_truthy
expect(user.remember_created_at).to be_truthy
end
it "queues a slack message to be sent for a user whose identity is brand new" do
service.auth.extra.raw_info.created_at = 1.minute.ago.rfc3339
sidekiq_assert_enqueued_with(job: SlackBotPingWorker) do
service.get_user
end
end
end
describe "existing user" do
let(:auth) { OmniAuth.config.mock_auth[:twitter] }
let(:user) { create(:user) }
before { OmniAuth.config.mock_auth[:twitter].info.email = user.email }
it "doesn't create a duplicate user" do
service = described_class.new(auth)
expect do
service.get_user
end.not_to change(User, :count)
end
it "sets remember_me for the existing user" do
user.update_columns(remember_token: nil, remember_created_at: nil)
service = described_class.new(auth)
service.get_user
user.reload
expect(user.remember_me).to be_truthy
expect(user.remember_token).to be_truthy
expect(user.remember_created_at).to be_truthy
end
context "when the user has a new Twitter username" do
it "updates their username properly" do
new_username = "new_username#{rand(1000)}"
auth.info.nickname = new_username
service = described_class.new(auth)
service.get_user
user.reload
expect(user.twitter_username).to eq new_username
end
it "touches the profile_updated_at timestamp" do
original_profile_updated_at = user.profile_updated_at
new_username = "new_username#{rand(1000)}"
auth.info.nickname = new_username
service = described_class.new(auth)
service.get_user
user.reload
expect(user.profile_updated_at).to be > original_profile_updated_at
end
end
end
end

8
spec/services/broadcasts/welcome_notification/generator_spec.rb
View file

@ -13,6 +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
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
@ -141,10 +142,13 @@ RSpec.describe Broadcasts::WelcomeNotification::Generator, type: :service do
end
describe "#send_feed_customization_notification" do
let!(:user) { create(:user, :with_identity, identities: %w[twitter github], created_at: 3.days.ago) }
let!(:user) do
mock_auth_hash
create(:user, :with_identity, identities: %w[twitter github], created_at: 3.days.ago)
end
it "does not send a notification to a newly-created user" do
user.update!(created_at: Time.zone.now)
user.update!(created_at: Time.current)
sidekiq_perform_enqueued_jobs { described_class.new(user.id).send(:send_feed_customization_notification) }
expect(Notification).not_to have_received(:send_welcome_notification)
end

2
spec/services/users/delete_spec.rb
View file

@ -1,6 +1,8 @@
require "rails_helper"
RSpec.describe Users::Delete, type: :service do
before { mock_github }
let(:user) { create(:user, :with_identity, identities: ["github"]) }
it "deletes user" do

17
spec/spec_helper.rb
View file

@ -1,4 +1,21 @@
require "simplecov"
SimpleCov.start do
# TODO: [thepracticaldev/oss] enable this when we upgrade to SimpleCov 0.18
# enable_coverage :branch
add_group "Decorators", "app/decorators"
add_group "Errors", "app/errors"
add_group "Labor", "app/labor"
add_group "Liquid tags", "app/liquid_tags"
add_group "Policies", "app/policies"
add_group "Queries", "app/queries"
add_group "Sanitizers", "app/sanitizers"
add_group "Serializers", "app/serializers"
add_group "Services", "app/services"
add_group "Uploaders", "app/uploaders"
add_group "View objects", "app/view_objects"
end
require "zonebie/rspec"
# This file was generated by the `rails generate rspec:install` command. Conventionally, all

102
spec/support/omniauth_macros.rb
View file

@ -1,4 +1,6 @@
module OmniauthMacros
OMNIAUTH_DEFAULT_FAILURE_HANDLER = OmniAuth.config.on_failure
INFO = OmniAuth::AuthHash::InfoHash.new(
first_name: "fname",
last_name: "lname",
@ -7,27 +9,28 @@ module OmniauthMacros
nickname: "fname.lname",
email: "yourname@email.com",
verified: true,
# info.image = "http://graph.facebook.com/123456/picture?type=square&#8221"
)
EXTRA_INFO = Hashie::Mash.new(raw_info: Hashie::Mash.new(
email: "yourname@email.com",
first_name: "fname",
gender: "female",
id: "123456",
last_name: "lname",
link: "http://www.facebook.com/url&#8221",
lang: "fr",
locale: "en_US",
name: "fname lname",
timezone: 5.5,
updated_time: "2012-06-08T13:09:47+0000",
username: "fname.lname",
verified: true,
followers_count: 100,
friends_count: 1000,
created_at: "2017-06-08T13:09:47+0000",
))
EXTRA_INFO = Hashie::Mash.new(
raw_info: Hashie::Mash.new(
email: "yourname@email.com",
first_name: "fname",
gender: "female",
id: "123456",
last_name: "lname",
link: "http://www.facebook.com/url&#8221",
lang: "fr",
locale: "en_US",
name: "fname lname",
timezone: 5.5,
updated_time: "2012-06-08T13:09:47+0000",
username: "fname.lname",
verified: true,
followers_count: 100,
friends_count: 1000,
created_at: "2017-06-08T13:09:47+0000",
),
)
CREDENTIAL = OmniAuth::AuthHash::InfoHash.new(
token: "2735246777-jlOnuFlGlvybuwDJfyrIyESLUEgoo6CffyJCQUO",
@ -41,20 +44,77 @@ module OmniauthMacros
credentials: CREDENTIAL
}.freeze
def mock_auth_with_invalid_credentials(provider)
OmniAuth.config.mock_auth[provider] = :invalid_credentials
end
def setup_omniauth_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>
# for more details
local_failure_handler = lambda do |env|
env["omniauth.error"] = error
env
end
# here we compose the two handlers into a single function,
# the result will be global_failure_handler(local_failure_handler(env))
failure_handler = local_failure_handler >> OMNIAUTH_DEFAULT_FAILURE_HANDLER
OmniAuth.config.on_failure = failure_handler
end
def omniauth_failure_args(error, provider, params)
class_name = error.present? ? error.class.name : ""
[
tags: [
"class:#{class_name}",
"message:#{error&.message}",
"reason:#{error.try(:error_reason)}",
"type:#{error.try(:error)}",
"uri:#{error.try(:error_uri)}",
"provider:#{provider}",
"origin:",
"params:#{params}",
],
]
end
def mock_auth_hash
mock_twitter
mock_github
end
def mock_twitter
info = BASIC_INFO[:info].merge(
image: "https://dummyimage.com/400x400_normal.jpg",
)
extra = BASIC_INFO[:extra].merge(
access_token: "value",
)
OmniAuth.config.mock_auth[:twitter] = OmniAuth::AuthHash.new(
BASIC_INFO.merge(provider: "twitter"),
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"),
BASIC_INFO.merge(
provider: "github",
info: info,
),
)
end
end

104
spec/system/authentication/user_logs_in_with_github_spec.rb Normal file
View file

@ -0,0 +1,104 @@
require "rails_helper"
RSpec.describe "Authenticating with GitHub" do
let(:sign_in_link) { "Sign In With GitHub" }
before { mock_github }
context "when a user is new" do
context "when using valid credentials" do
it "logs in and redirects to the onboarding" do
visit root_path
click_link sign_in_link
expect(page.current_path).to include("/onboarding")
expect(page.html).to include("onboarding-container")
end
end
context "when using invalid credentials" do
before do
mock_auth_with_invalid_credentials(:github)
allow(DatadogStatsClient).to receive(:increment)
end
after do
OmniAuth.config.on_failure = OmniauthMacros.const_get("OMNIAUTH_DEFAULT_FAILURE_HANDLER")
end
it "does not log in" do
visit root_path
click_link sign_in_link
expect(page).to have_current_path("/users/sign_in")
expect(page).to have_link("Sign In/Up")
expect(page).to have_link("Via GitHub")
expect(page).to have_link("All about #{ApplicationConfig['COMMUNITY_NAME']}")
end
it "notifies Datadog about a callback error" do
error = OmniAuth::Strategies::OAuth2::CallbackError.new(
"Callback error", "Error reason", "https://example.com/error"
)
setup_omniauth_error(error)
visit root_path
click_link sign_in_link
args = omniauth_failure_args(error, "github", '{"state"=>"navbar_basic"}')
expect(DatadogStatsClient).to have_received(:increment).with(
"omniauth.failure", *args
)
end
it "notifies Datadog about an OAuth unauthorized error" do
request = double
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)
visit root_path
click_link sign_in_link
args = omniauth_failure_args(error, "github", '{"state"=>"navbar_basic"}')
expect(DatadogStatsClient).to have_received(:increment).with(
"omniauth.failure", *args
)
end
it "notifies Datadog even with no OmniAuth error present" do
error = nil
setup_omniauth_error(error)
visit root_path
click_link sign_in_link
args = omniauth_failure_args(error, "github", '{"state"=>"navbar_basic"}')
expect(DatadogStatsClient).to have_received(:increment).with(
"omniauth.failure", *args
)
end
end
end
context "when a user already exists" do
let!(:auth_payload) { OmniAuth.config.mock_auth[:github] }
let(:user) { create(:user, :with_identity, identities: [:github]) }
before do
auth_payload.info.email = user.email
sign_in user
end
context "when using valid credentials" do
it "logs in and redirects to the onboarding" do
visit "/users/auth/github"
expect(page).to have_current_path("/dashboard?signin=true")
end
end
end
end

104
spec/system/authentication/user_logs_in_with_twitter_spec.rb Normal file
View file

@ -0,0 +1,104 @@
require "rails_helper"
RSpec.describe "Authenticating with Twitter" do
let(:sign_in_link) { "Sign In With Twitter" }
before { mock_twitter }
context "when a user is new" do
context "when using valid credentials" do
it "logs in and redirects to the onboarding" do
visit root_path
click_link sign_in_link
expect(page).to have_current_path("/onboarding?referrer=none")
expect(page.html).to include("onboarding-container")
end
end
context "when using invalid credentials" do
before do
mock_auth_with_invalid_credentials(:twitter)
allow(DatadogStatsClient).to receive(:increment)
end
after do
OmniAuth.config.on_failure = OmniauthMacros.const_get("OMNIAUTH_DEFAULT_FAILURE_HANDLER")
end
it "does not log in" do
visit root_path
click_link sign_in_link
expect(page).to have_current_path("/users/sign_in")
expect(page).to have_link("Sign In/Up")
expect(page).to have_link("Via Twitter")
expect(page).to have_link("All about #{ApplicationConfig['COMMUNITY_NAME']}")
end
it "notifies Datadog about a callback error" do
error = OmniAuth::Strategies::OAuth2::CallbackError.new(
"Callback error", "Error reason", "https://example.com/error"
)
setup_omniauth_error(error)
visit root_path
click_link sign_in_link
args = omniauth_failure_args(error, "twitter", "{}")
expect(DatadogStatsClient).to have_received(:increment).with(
"omniauth.failure", *args
)
end
it "notifies Datadog about an OAuth unauthorized error" do
request = double
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)
visit root_path
click_link sign_in_link
args = omniauth_failure_args(error, "twitter", "{}")
expect(DatadogStatsClient).to have_received(:increment).with(
"omniauth.failure", *args
)
end
it "notifies Datadog even with no OmniAuth error present" do
error = nil
setup_omniauth_error(error)
visit root_path
click_link sign_in_link
args = omniauth_failure_args(error, "twitter", "{}")
expect(DatadogStatsClient).to have_received(:increment).with(
"omniauth.failure", *args
)
end
end
end
context "when a user already exists" do
let!(:auth_payload) { OmniAuth.config.mock_auth[:twitter] }
let(:user) { create(:user, :with_identity, identities: [:twitter]) }
before do
auth_payload.info.email = user.email
sign_in user
end
context "when using valid credentials" do
it "logs in and redirects to the onboarding" do
visit "/users/auth/twitter"
expect(page).to have_current_path("/dashboard?signin=true")
end
end
end
end

173
spec/system/user_logs_in_with_twitter_spec.rb
View file

@ -1,173 +0,0 @@
require "rails_helper"
global_failure_handler = OmniAuth.config.on_failure
def user_grants_authorization_on_twitter(twitter_callback_hash)
OmniAuth.config.add_mock(:twitter, twitter_callback_hash)
end
def user_does_not_grant_authorization_on_twitter
OmniAuth.config.mock_auth[:twitter] = :invalid_credentials
end
def setup_omniauth_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>
# for more details
global_failure_handler = OmniAuth.config.on_failure
local_failure_handler = lambda do |env|
env["omniauth.error"] = error
env
end
# here we compose the two handlers into a single function,
# the result will be global_failure_handler(local_failure_handler(env))
failure_handler = local_failure_handler >> global_failure_handler
OmniAuth.config.on_failure = failure_handler
end
def get_failure_args(error)
class_name = error.present? ? error.class.name : ""
[
tags: [
"class:#{class_name}",
"message:#{error&.message}",
"reason:#{error.try(:error_reason)}",
"type:#{error.try(:error)}",
"uri:#{error.try(:error_uri)}",
"provider:twitter",
"origin:",
"params:{}",
],
]
end
RSpec.describe "Authenticating with Twitter" do
let(:twitter_callback_hash) do
{
provider: "twitter",
uid: "111111",
credentials: {
token: "222222",
secret: "333333"
},
extra: {
access_token: "",
raw_info: {
name: "Bruce Wayne",
created_at: "Thu Jul 4 00:00:00 +0000 2013" # This is mandatory
}
},
info: {
nickname: "batman",
name: "Bruce Wayne",
email: "batman@batcave.com"
}
}
end
context "when a user is new" do
let(:twitter_link) { "Sign In With Twitter" }
context "when using valid credentials" do
before do
user_grants_authorization_on_twitter(twitter_callback_hash)
end
it "logs in and redirects to the onboarding" do
visit root_path
click_link twitter_link
expect(page).to have_current_path("/onboarding?referrer=none")
expect(page.html).to include("onboarding-container")
end
end
context "when using invalid credentials" do
let(:callback_failure) do
[
"omniauth.failure",
tags: [
"class:",
"message:",
"reason:",
"type:",
"uri:",
"provider:twitter",
"origin:",
"params:{}",
],
]
end
before do
user_does_not_grant_authorization_on_twitter
allow(DatadogStatsClient).to receive(:increment)
end
after do
OmniAuth.config.on_failure = global_failure_handler
end
it "does not log in" do
visit root_path
click_link twitter_link
expect(page).to have_current_path("/users/sign_in")
expect(page).to have_link("Sign In/Up")
expect(page).to have_link("Via Twitter")
expect(page).to have_link("Via GitHub")
expect(page).to have_link("All about #{ApplicationConfig['COMMUNITY_NAME']}")
end
it "notifies Datadog about a callback error" do
error = OmniAuth::Strategies::OAuth2::CallbackError.new(
"Callback error", "Error reason", "https://example.com/error"
)
setup_omniauth_error(error)
visit root_path
click_link twitter_link
args = get_failure_args(error)
expect(DatadogStatsClient).to have_received(:increment).with(
"omniauth.failure", *args
)
end
it "notifies Datadog about an OAuth unauthorized error" do
request = double
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)
visit root_path
click_link twitter_link
args = get_failure_args(error)
expect(DatadogStatsClient).to have_received(:increment).with(
"omniauth.failure", *args
)
end
it "notifies Datadog even with no OmniAuth error present" do
error = nil
setup_omniauth_error(error)
visit root_path
click_link twitter_link
args = get_failure_args(error)
expect(DatadogStatsClient).to have_received(:increment).with(
"omniauth.failure", *args
)
end
end
end
end