diff --git a/Gemfile b/Gemfile index 5db7c75b1..be795f50b 100644 --- a/Gemfile +++ b/Gemfile @@ -63,6 +63,7 @@ gem "omniauth", "~> 2.0" # A generalized Rack framework for multiple-provider au gem "omniauth-apple", "~> 1.0" # OmniAuth strategy for Sign In with Apple gem "omniauth-facebook", "~> 9.0" # OmniAuth strategy for Facebook gem "omniauth-github", "~> 2.0" # OmniAuth strategy for GitHub +gem "omniauth-google-oauth2", "~> 1.0" gem "omniauth-rails_csrf_protection", "~> 1.0" # Provides CSRF protection on OmniAuth request endpoint on Rails application. gem "omniauth-twitter", "~> 1.4" # OmniAuth strategy for Twitter gem "parallel", "~> 1.21" # Run any kind of code in parallel processes diff --git a/Gemfile.lock b/Gemfile.lock index 23a9dce6c..0a40c2027 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -523,6 +523,11 @@ GEM omniauth-github (2.0.0) omniauth (~> 2.0) omniauth-oauth2 (~> 1.7.1) + omniauth-google-oauth2 (1.0.0) + jwt (>= 2.0) + oauth2 (~> 1.1) + omniauth (~> 2.0) + omniauth-oauth2 (~> 1.7.1) omniauth-oauth (1.2.0) oauth omniauth (>= 1.0, < 3) @@ -966,6 +971,7 @@ DEPENDENCIES omniauth-apple (~> 1.0) omniauth-facebook (~> 9.0) omniauth-github (~> 2.0) + omniauth-google-oauth2 (~> 1.0) omniauth-rails_csrf_protection (~> 1.0) omniauth-twitter (~> 1.4) parallel (~> 1.21) diff --git a/app/assets/images/google_oauth2.svg b/app/assets/images/google_oauth2.svg new file mode 100644 index 000000000..eec3cc840 --- /dev/null +++ b/app/assets/images/google_oauth2.svg @@ -0,0 +1,3 @@ + + + diff --git a/app/assets/stylesheets/components/buttons.scss b/app/assets/stylesheets/components/buttons.scss index 73f53803c..3de9f4d60 100644 --- a/app/assets/stylesheets/components/buttons.scss +++ b/app/assets/stylesheets/components/buttons.scss @@ -24,6 +24,10 @@ --brand-email-bg: #24292e; --brand-email-color: #fff; --brand-email-bg-hover: #000; + + --brand-google_oauth2-bg: #1da1f2; + --brand-google_oauth2-color: #fff; + --brand-google_oauth2-bg-hover: #0096f2; } // Basic styling @@ -325,6 +329,17 @@ label[class*='crayons-btn']:focus-within // label for input[type="file"] made to --color-inverted-hover: var(--brand-email-color); } +.crayons-btn--brand-google_oauth2 { + --bg: var(--brand-google_oauth2-bg); + --bg-hover: var(--brand-google_oauth2-bg-hover); + --color: var(--brand-google_oauth2-color); + --color-hover: var(--brand-google_oauth2-color); + --bg-inverted: var(--brand-google_oauth2-bg); + --bg-inverted-hover: var(--brand-google_oauth2-bg-hover); + --color-inverted: var(--brand-google_oauth2-color); + --color-inverted-hover: var(--brand-google_oauth2-color); +} + // Icon alone .crayons-btn--icon, .crayons-btn--icon-rounded { diff --git a/app/helpers/authentication_helper.rb b/app/helpers/authentication_helper.rb index a7728d405..49d6ab77b 100644 --- a/app/helpers/authentication_helper.rb +++ b/app/helpers/authentication_helper.rb @@ -4,9 +4,7 @@ module AuthenticationHelper end def authentication_available_providers - Authentication::Providers.available.map do |provider_name| - Authentication::Providers.const_get(provider_name.to_s.titleize) - end + Authentication::Providers.available_providers end def authentication_enabled_providers diff --git a/app/lib/constants/settings/authentication.rb b/app/lib/constants/settings/authentication.rb index 1d14231b8..3503bd83a 100644 --- a/app/lib/constants/settings/authentication.rb +++ b/app/lib/constants/settings/authentication.rb @@ -59,6 +59,14 @@ module Constants description: "The \"Client Secret\" portion of the GitHub Oauth Apps portal", placeholder: "" }, + google_oauth2_key: { + description: "The \"Client ID\" portion of the OAuth 2.0 page on the Google Cloud Platform portal", + placeholder: "" + }, + google_oauth2_secret: { + description: "The \"Client Secret\" portion of the OAuth 2.0 page on the Google Cloud Platform portal", + placeholder: "" + }, invite_only_mode: { description: "Only users invited by email can join this community.", placeholder: "" diff --git a/app/models/settings/authentication.rb b/app/models/settings/authentication.rb index 5726d669f..46d5eff17 100644 --- a/app/models/settings/authentication.rb +++ b/app/models/settings/authentication.rb @@ -21,6 +21,8 @@ module Settings setting :forem_secret, type: :string setting :github_key, type: :string, default: ApplicationConfig["GITHUB_KEY"] setting :github_secret, type: :string, default: ApplicationConfig["GITHUB_SECRET"] + setting :google_oauth2_key, type: :string + setting :google_oauth2_secret, type: :string setting :invite_only_mode, type: :boolean, default: false setting :providers, type: :array, default: %w[] setting :require_captcha_for_email_password_registration, type: :boolean, default: false diff --git a/app/services/authentication/providers.rb b/app/services/authentication/providers.rb index 082a6409a..2faeaab2b 100644 --- a/app/services/authentication/providers.rb +++ b/app/services/authentication/providers.rb @@ -11,7 +11,7 @@ module Authentication module Providers # Retrieves a provider that is both available and enabled def self.get!(provider_name) - name = provider_name.to_s.titleize + name = provider_name.to_s.camelize unless available?(provider_name) raise( @@ -32,12 +32,16 @@ module Authentication def self.available Authentication::Providers::Provider.subclasses.map do |subclass| - subclass.name.demodulize.downcase.to_sym + subclass.name.demodulize.underscore.downcase.to_sym end.sort end + def self.available_providers + Authentication::Providers::Provider.subclasses.sort_by(&:name) + end + def self.available?(provider_name) - Authentication::Providers.const_defined?(provider_name.to_s.titleize) + Authentication::Providers.const_defined?(provider_name.to_s.camelize) end # Returns enabled providers diff --git a/app/services/authentication/providers/google_oauth2.rb b/app/services/authentication/providers/google_oauth2.rb new file mode 100644 index 000000000..9060378ba --- /dev/null +++ b/app/services/authentication/providers/google_oauth2.rb @@ -0,0 +1,55 @@ +module Authentication + module Providers + # Google OAuth 2.0 authentication provider, uses omniauth-google-oauth2 as backend + class GoogleOauth2 < Provider + OFFICIAL_NAME = "Google".freeze + SETTINGS_URL = "https://console.cloud.google.com/apis/credentials".freeze + + def new_user_data + { + name: info.name, + email: info.email || "", + remote_profile_image_url: Users::SafeRemoteProfileImageUrl.call(@info.image), + google_oauth2_username: user_nickname + } + end + + def existing_user_data + { + google_oauth2_username: info.name + } + end + + # We're overriding this method because Google doesn't have a concept nickname or username. + # Instead: we'll construct one based on the user's name with some randomization thrown in based + # on uid, which is guaranteed to be present and unique from Google. + def user_nickname + [ + info.name.sub(" ", "_"), + Digest::SHA512.hexdigest(auth_payload.uid), + ].join("_")[0...25] + end + + def self.official_name + OFFICIAL_NAME + end + + def self.settings_url + SETTINGS_URL + end + + def self.sign_in_path(**kwargs) + ::Authentication::Paths.sign_in_path( + "google_oauth2", + **kwargs, + ) + end + + protected + + def cleanup_payload(auth_payload) + auth_payload + end + end + end +end diff --git a/app/services/authentication/providers/provider.rb b/app/services/authentication/providers/provider.rb index 42760dd10..470aa80c1 100644 --- a/app/services/authentication/providers/provider.rb +++ b/app/services/authentication/providers/provider.rb @@ -34,7 +34,7 @@ module Authentication end def self.provider_name - name.demodulize.downcase.to_sym + name.demodulize.underscore.to_sym end def self.user_username_field diff --git a/app/workers/metrics/record_daily_notifications_worker.rb b/app/workers/metrics/record_daily_notifications_worker.rb index 4dc5d9e79..4fa1e0b2a 100644 --- a/app/workers/metrics/record_daily_notifications_worker.rb +++ b/app/workers/metrics/record_daily_notifications_worker.rb @@ -9,6 +9,7 @@ module Metrics welcome_notification_customize_feed welcome_notification_twitter_connect welcome_notification_github_connect + welcome_notification_google_connect welcome_notification_customize_experience welcome_notification_discuss_and_ask welcome_notification_download_app diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 37c1d91ab..f1d2cc146 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -18,6 +18,11 @@ GITHUB_OMNIAUTH_SETUP = lambda do |env| env["omniauth.strategy"].options[:scope] = "user:email" end +GOOGLE_OAUTH2_OMNIAUTH_SETUP = lambda do |env| + env["omniauth.strategy"].options[:client_id] = Settings::Authentication.google_oauth2_key + env["omniauth.strategy"].options[:client_secret] = Settings::Authentication.google_oauth2_secret +end + FACEBOOK_OMNIAUTH_SETUP = lambda do |env| env["omniauth.strategy"].options[:client_id] = Settings::Authentication.facebook_key env["omniauth.strategy"].options[:client_secret] = Settings::Authentication.facebook_secret @@ -321,6 +326,7 @@ Devise.setup do |config| # Fun fact, unless Twitter is last, it doesn't work for some reason. config.omniauth :facebook, setup: FACEBOOK_OMNIAUTH_SETUP config.omniauth :github, setup: GITHUB_OMNIAUTH_SETUP + config.omniauth :google_oauth2, setup: GOOGLE_OAUTH2_OMNIAUTH_SETUP config.omniauth :apple, setup: APPLE_OMNIAUTH_SETUP config.omniauth :forem, setup: FOREM_OMNIAUTH_SETUP, strategy_class: OmniAuth::Strategies::Forem config.omniauth :twitter, setup: TWITTER_OMNIAUTH_SETUP diff --git a/db/migrate/20211220170907_add_google_o_auth_fields.rb b/db/migrate/20211220170907_add_google_o_auth_fields.rb new file mode 100644 index 000000000..9a64b7e54 --- /dev/null +++ b/db/migrate/20211220170907_add_google_o_auth_fields.rb @@ -0,0 +1,6 @@ +class AddGoogleOAuthFields < ActiveRecord::Migration[6.1] + def change + add_column :users, :google_oauth2_username, :string + add_column :users, :google_oauth2_created_at, :datetime + end +end diff --git a/db/migrate/20211220171024_add_google_o_auth_fields_index.rb b/db/migrate/20211220171024_add_google_o_auth_fields_index.rb new file mode 100644 index 000000000..c15349c2b --- /dev/null +++ b/db/migrate/20211220171024_add_google_o_auth_fields_index.rb @@ -0,0 +1,7 @@ +class AddGoogleOAuthFieldsIndex < ActiveRecord::Migration[6.1] + disable_ddl_transaction! + + def change + add_index :users, :google_oauth2_username, algorithm: :concurrently + end +end diff --git a/db/schema.rb b/db/schema.rb index 87526bf7d..f1c616557 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1209,6 +1209,8 @@ ActiveRecord::Schema.define(version: 2021_12_22_040359) do t.string "forem_username" t.datetime "github_repos_updated_at", default: "2017-01-01 05:00:00" t.string "github_username" + t.datetime "google_oauth2_created_at" + t.string "google_oauth2_username" t.datetime "invitation_accepted_at" t.datetime "invitation_created_at" t.integer "invitation_limit" @@ -1270,6 +1272,7 @@ ActiveRecord::Schema.define(version: 2021_12_22_040359) do t.index ["facebook_username"], name: "index_users_on_facebook_username" t.index ["feed_fetched_at"], name: "index_users_on_feed_fetched_at" t.index ["github_username"], name: "index_users_on_github_username", unique: true + t.index ["google_oauth2_username"], name: "index_users_on_google_oauth2_username" t.index ["invitation_token"], name: "index_users_on_invitation_token", unique: true t.index ["invitations_count"], name: "index_users_on_invitations_count" t.index ["invited_by_id"], name: "index_users_on_invited_by_id" diff --git a/db/seeds.rb b/db/seeds.rb index 820b37e32..af335e5de 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -369,6 +369,8 @@ seeder.create_if_none(Broadcast) do "Consider connecting it.", github_connect: "You're on a roll! 🎉 Do you have a GitHub account? " \ "Consider connecting it so you can pin any of your repos to your profile.", + google_oauth2_connect: "You're on a roll! 🎉 Do you have a Google account? " \ + "Consider connecting it.", apple_connect: "You're on a roll! 🎉 Do you have an Apple account? " \ "Consider connecting it.", customize_feed: diff --git a/lib/data_update_scripts/20220107215659_insert_google_ouauth2_connect_broadcast_message.rb b/lib/data_update_scripts/20220107215659_insert_google_ouauth2_connect_broadcast_message.rb new file mode 100644 index 000000000..a449d908b --- /dev/null +++ b/lib/data_update_scripts/20220107215659_insert_google_ouauth2_connect_broadcast_message.rb @@ -0,0 +1,17 @@ +module DataUpdateScripts + class InsertGoogleOuauth2ConnectBroadcastMessage + def run + return if Broadcast.find_by(title: "Welcome Notification: google_oauth2_connect") + + message = "You're on a roll! 🎉 Do you have a Google account? " \ + "Consider connecting it." + + Broadcast.create!( + title: "Welcome Notification: google_oauth2_connect", + processed_html: message, + type_of: "Welcome", + active: true, + ) + end + end +end diff --git a/spec/factories/broadcasts.rb b/spec/factories/broadcasts.rb index eaa5b3299..be4c80b93 100644 --- a/spec/factories/broadcasts.rb +++ b/spec/factories/broadcasts.rb @@ -39,6 +39,15 @@ FactoryBot.define do end end + factory :google_oauth2_connect_broadcast do + title { "Welcome Notification: google_oauth2_connect" } + type_of { "Welcome" } + processed_html do + "You're on a roll! 🎉 Do you have a Google account? Consider " \ + "connecting it." + end + end + factory :facebook_connect_broadcast do title { "Welcome Notification: facebook_connect" } type_of { "Welcome" } diff --git a/spec/helpers/authentication_helper_spec.rb b/spec/helpers/authentication_helper_spec.rb index c92a79ee5..69a1788fe 100644 --- a/spec/helpers/authentication_helper_spec.rb +++ b/spec/helpers/authentication_helper_spec.rb @@ -34,7 +34,7 @@ RSpec.describe AuthenticationHelper, type: :helper do allow(Authentication::Providers).to receive(:enabled).and_return(providers) allow(user).to receive(:identities).and_return(user.identities.where(provider: providers)) - expect(helper.signed_up_with(user)).to match(/GitHub and Twitter/) + expect(helper.signed_up_with(user)).to match(/Google and Twitter/) expect(helper.signed_up_with(user)).to match(/use any of those/) end diff --git a/spec/lib/data_update_scripts/insert_google_ouauth2_connect_broadcast_message_spec.rb b/spec/lib/data_update_scripts/insert_google_ouauth2_connect_broadcast_message_spec.rb new file mode 100644 index 000000000..d6f677721 --- /dev/null +++ b/spec/lib/data_update_scripts/insert_google_ouauth2_connect_broadcast_message_spec.rb @@ -0,0 +1,19 @@ +require "rails_helper" +require Rails.root.join( + "lib/data_update_scripts/20220107215659_insert_google_ouauth2_connect_broadcast_message.rb", +) + +describe DataUpdateScripts::InsertGoogleOuauth2ConnectBroadcastMessage do + it "works without a Broadcast" do + expect do + described_class.new.run + end.to change(Broadcast, :count).by(1) + end + + it "works when a Broadcast already exists" do + create(:google_oauth2_connect_broadcast) + expect do + described_class.new.run + end.not_to change(Broadcast, :count) + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 76dd00371..cd5dc94b9 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -453,7 +453,7 @@ RSpec.describe User, type: :model do case provider_name when :apple expect(new_user.username).to match(/valid_username_\w+/) - when :facebook + when :facebook, :google_oauth2 expect(new_user.username).to match(/fname_lname_\S*\z/) else expect(new_user.username).to eq("valid_username") @@ -472,7 +472,7 @@ RSpec.describe User, type: :model do case provider_name when :apple expect(new_user.username).to match(/invalidusername_\w+/) - when :facebook + when :facebook, :google_oauth2 expect(new_user.username).to match(/fname_lname_\S*\z/) else expect(new_user.username).to eq("invalidusername") diff --git a/spec/services/authentication/providers_spec.rb b/spec/services/authentication/providers_spec.rb index 370548989..0420c98c8 100644 --- a/spec/services/authentication/providers_spec.rb +++ b/spec/services/authentication/providers_spec.rb @@ -24,15 +24,35 @@ RSpec.describe Authentication::Providers, type: :service do ) expect(is_subclass_of).to be(true) end + + it "handles spaces in provider names" do + allow(Settings::Authentication).to receive(:providers).and_return(%w[google_oauth2]) + + expect do + described_class.get!(:google_oauth2) + end.not_to raise_error + end end describe ".available" do it "lists the available providers" do - available_providers = %i[apple facebook forem github twitter] + available_providers = %i[apple facebook forem github google_oauth2 twitter] expect(described_class.available).to eq(available_providers) end end + describe ".availble_providers" do + it "lists the available providers" do + available_providers = [Authentication::Providers::Forem, + Authentication::Providers::Facebook, + Authentication::Providers::Apple, + Authentication::Providers::Twitter, + Authentication::Providers::GoogleOauth2, + Authentication::Providers::Github].sort_by(&:name) + expect(described_class.available_providers).to eq(available_providers) + end + end + describe ".enabled" do context "when one of the available providers is disabled" do it "only lists those that remain enabled" do diff --git a/spec/services/broadcasts/welcome_notification/generator_spec.rb b/spec/services/broadcasts/welcome_notification/generator_spec.rb index 429831dc0..b87a5022b 100644 --- a/spec/services/broadcasts/welcome_notification/generator_spec.rb +++ b/spec/services/broadcasts/welcome_notification/generator_spec.rb @@ -11,6 +11,7 @@ RSpec.describe Broadcasts::WelcomeNotification::Generator, type: :service do let!(:facebook_connect_broadcast) { create(:facebook_connect_broadcast) } let!(:apple_connect_broadcast) { create(:apple_connect_broadcast) } let!(:forem_connect_broadcast) { create(:forem_connect_broadcast) } + let!(:google_oauth2_connect_broadcast) { create(:google_oauth2_connect_broadcast) } let!(:customize_feed_broadcast) { create(:customize_feed_broadcast) } let!(:discuss_and_ask_broadcast) { create(:discuss_and_ask_broadcast) } let!(:customize_ux_broadcast) { create(:customize_ux_broadcast) } @@ -70,6 +71,7 @@ RSpec.describe Broadcasts::WelcomeNotification::Generator, type: :service do twitter_connect_broadcast, apple_connect_broadcast, forem_connect_broadcast, + google_oauth2_connect_broadcast, ].include?(user.notifications.last.notifiable) expect(not_github).to be(true) diff --git a/spec/support/omniauth_helpers.rb b/spec/support/omniauth_helpers.rb index 878e98286..06cf759c5 100644 --- a/spec/support/omniauth_helpers.rb +++ b/spec/support/omniauth_helpers.rb @@ -135,6 +135,55 @@ module OmniauthHelpers }, ).freeze + OMNIAUTH_PAYLOAD_GOOGLE_OAUTH2 = OmniAuth::AuthHash::InfoHash.new( + { + "provider" => "google_oauth2", + "uid" => "100000000000000000000", + "info" => { + "name" => "fname lname", + "email" => "john@example.com", + "first_name" => "fname", + "last_name" => "lname", + "image" => "https://dummyimage.com/400x400.jpg", + "urls" => { + "google" => "https://plus.google.com/+JohnSmith" + } + }, + "credentials" => { + "token" => "TOKEN", + "refresh_token" => "REFRESH_TOKEN", + "expires_at" => 1_496_120_719, + "expires" => true + }, + "extra" => { + "id_token" => "ID_TOKEN", + "id_info" => { + "azp" => "APP_ID", + "aud" => "APP_ID", + "sub" => "100000000000000000000", + "email" => "john@example.com", + "email_verified" => true, + "at_hash" => "HK6E_P6Dh8Y93mRNtsDB1Q", + "iss" => "accounts.google.com", + "iat" => 1_496_117_119, + "exp" => 1_496_120_719 + }, + "raw_info" => { + "sub" => "100000000000000000000", + "name" => "fname lname", + "given_name" => "fname", + "family_name" => "lname", + "profile" => "https://plus.google.com/+JohnSmith", + "picture" => "https://dummyimage.com/400x400.jpg", + "email" => "john@example.com", + "email_verified" => "true", + "locale" => "en", + "hd" => "company.com" + } + } + }, + ).freeze + def omniauth_setup_invalid_credentials(provider) OmniAuth.config.mock_auth[provider] = :invalid_credentials end @@ -198,6 +247,10 @@ module OmniauthHelpers OmniAuth.config.mock_auth[:apple] = OMNIAUTH_PAYLOAD_APPLE.dup end + def omniauth_mock_google_oauth2_payload + OmniAuth.config.mock_auth[:google_oauth2] = OMNIAUTH_PAYLOAD_GOOGLE_OAUTH2.dup + end + def omniauth_mock_github_payload info = OMNIAUTH_BASIC_INFO[:info].merge( image: "https://dummyimage.com/400x400.jpg", diff --git a/spec/system/authentication/creator_config_edit_spec.rb b/spec/system/authentication/creator_config_edit_spec.rb index 10cfbda49..13a2fef31 100644 --- a/spec/system/authentication/creator_config_edit_spec.rb +++ b/spec/system/authentication/creator_config_edit_spec.rb @@ -16,8 +16,8 @@ RSpec.describe "Creator config edit", type: :system, js: true do click_on("Show info", match: :first) end - Authentication::Providers.available.each do |provider| - element = find(".config-authentication__item--label", text: /#{provider}/i) + Authentication::Providers.available_providers.each do |provider| + element = find(".config-authentication__item--label", text: /#{provider.official_name}/i) expect(element).not_to be_nil end end diff --git a/vendor/cache/omniauth-google-oauth2-1.0.0.gem b/vendor/cache/omniauth-google-oauth2-1.0.0.gem new file mode 100644 index 000000000..dba15dc31 Binary files /dev/null and b/vendor/cache/omniauth-google-oauth2-1.0.0.gem differ