From 883eb170dec7e4ff04692f2b5a35af584f9404ca Mon Sep 17 00:00:00 2001 From: rhymes Date: Tue, 28 Apr 2020 16:16:28 +0200 Subject: [PATCH] [deploy] Authentication refactoring: add available and enabled providers (#7505) * Add Providers(.available|.enabled|.enabled?) * Add provider_sidebar partial * Add providers_signup_modal partial and Providers.sign_in_url * Use path helpers * Add providers_nav_menu partial * Add providers_registration_form partial * Generalize users/additional_authentication * Refactor sign_in_path and authentication_path * Add .official_name and fix specs * Preload authentication providers correctly and use less Ruby magic * Put require_dependency in the correct place --- app/models/site_config.rb | 5 - app/services/authentication/errors.rb | 3 + app/services/authentication/providers.rb | 69 +++++++++++++- .../authentication/providers/github.rb | 5 + .../authentication/providers/provider.rb | 5 + .../authentication/providers/twitter.rb | 5 + .../articles/_sidebar_additional.html.erb | 7 +- .../registrations/_registration_form.html.erb | 12 +-- app/views/layouts/_nav_menu.html.erb | 30 +++--- app/views/layouts/_signup_modal.html.erb | 19 ++-- .../_providers_nav_menu.html.erb | 10 ++ .../_providers_registration_form.html.erb | 10 ++ .../_providers_sidebar.html.erb | 10 ++ .../_providers_signup_modal.html.erb | 13 +++ .../users/_additional_authentication.html.erb | 31 ++++--- spec/factories/users.rb | 2 +- spec/requests/user/user_settings_spec.rb | 53 +++++++---- .../services/authentication/providers_spec.rb | 93 +++++++++++++++++++ .../user_logs_in_with_github_spec.rb | 10 +- spec/views/users/edit.html.erb_spec.rb | 30 ------ 20 files changed, 306 insertions(+), 116 deletions(-) create mode 100644 app/views/shared/authentication/_providers_nav_menu.html.erb create mode 100644 app/views/shared/authentication/_providers_registration_form.html.erb create mode 100644 app/views/shared/authentication/_providers_sidebar.html.erb create mode 100644 app/views/shared/authentication/_providers_signup_modal.html.erb create mode 100644 spec/services/authentication/providers_spec.rb delete mode 100644 spec/views/users/edit.html.erb_spec.rb diff --git a/app/models/site_config.rb b/app/models/site_config.rb index 3b214abe7..6b4635849 100644 --- a/app/models/site_config.rb +++ b/app/models/site_config.rb @@ -91,9 +91,4 @@ class SiteConfig < RailsSettings::Base # Shop field :shop_url, type: :string, default: "https://shop.dev.to" - - # Helpful methods - def self.auth_allowed?(provider) - authentication_providers.include?(provider) - end end diff --git a/app/services/authentication/errors.rb b/app/services/authentication/errors.rb index 1b7346a2e..2c4311a83 100644 --- a/app/services/authentication/errors.rb +++ b/app/services/authentication/errors.rb @@ -5,5 +5,8 @@ module Authentication class ProviderNotFound < Error end + + class ProviderNotEnabled < Error + end end end diff --git a/app/services/authentication/providers.rb b/app/services/authentication/providers.rb index 83bce303c..50c63e69a 100644 --- a/app/services/authentication/providers.rb +++ b/app/services/authentication/providers.rb @@ -1,11 +1,70 @@ +# We require all authentication modules to make sure providers +# are correctly preloaded both in development and in production and +# ready to be used when needed at runtime +Dir[Rails.root.join("app/services/authentication/**/*.rb")].each do |f| + require_dependency(f) +end + 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 + # Retrieves a provider that is both available and enabled def self.get!(provider_name) - "Authentication::Providers::#{provider_name.titleize}".constantize - rescue NameError => e - raise ::Authentication::Errors::ProviderNotFound, e + name = provider_name.to_s.titleize + + unless Authentication::Providers.const_defined?(name) + raise( + ::Authentication::Errors::ProviderNotFound, + "Provider #{name} is not available!", + ) + end + + unless enabled?(provider_name) + raise( + ::Authentication::Errors::ProviderNotEnabled, + "Provider #{name} is not enabled!", + ) + end + + Authentication::Providers.const_get(name) + end + + # Returns available providers + def self.available + # the magic is done in + Authentication::Providers::Provider.subclasses.map do |subclass| + subclass.name.demodulize.downcase.to_sym + end.sort + end + + # Returns enabled providers + # TODO: [thepracticaldev/oss] ideally this should be "available - disabled" + # we can get there once we have feature flags + def self.enabled + 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 diff --git a/app/services/authentication/providers/github.rb b/app/services/authentication/providers/github.rb index 367407805..6d7fecea6 100644 --- a/app/services/authentication/providers/github.rb +++ b/app/services/authentication/providers/github.rb @@ -2,6 +2,7 @@ module Authentication module Providers # GitHub authentication provider, uses omniauth-github as backend class Github < Provider + OFFICIAL_NAME = "GitHub".freeze CREATED_AT_FIELD = "github_created_at".freeze USERNAME_FIELD = "github_username".freeze @@ -24,6 +25,10 @@ module Authentication } end + def self.official_name + OFFICIAL_NAME + end + protected def cleanup_payload(auth_payload) diff --git a/app/services/authentication/providers/provider.rb b/app/services/authentication/providers/provider.rb index 70e6ba079..90d383074 100644 --- a/app/services/authentication/providers/provider.rb +++ b/app/services/authentication/providers/provider.rb @@ -41,6 +41,11 @@ module Authentication self.class::USERNAME_FIELD end + # Returns the official name of the authentication provider + def self.official_name + raise SubclassResponsibility + end + protected # Remove sensible data from the payload diff --git a/app/services/authentication/providers/twitter.rb b/app/services/authentication/providers/twitter.rb index 0146e6016..a823bfc5c 100644 --- a/app/services/authentication/providers/twitter.rb +++ b/app/services/authentication/providers/twitter.rb @@ -2,6 +2,7 @@ module Authentication module Providers # Twitter authentication provider, uses omniauth-twitter as backend class Twitter < Provider + OFFICIAL_NAME = "Twitter".freeze CREATED_AT_FIELD = "twitter_created_at".freeze USERNAME_FIELD = "twitter_username".freeze @@ -29,6 +30,10 @@ module Authentication } end + def self.official_name + OFFICIAL_NAME + end + protected def cleanup_payload(auth_payload) diff --git a/app/views/articles/_sidebar_additional.html.erb b/app/views/articles/_sidebar_additional.html.erb index d6c1bf651..85d82b494 100644 --- a/app/views/articles/_sidebar_additional.html.erb +++ b/app/views/articles/_sidebar_additional.html.erb @@ -7,12 +7,7 @@ Join <%= community_name %>
- <% if SiteConfig.auth_allowed?("twitter") %> - " class="cta cta-button login-cta-button" data-no-instant>Sign In With Twitter - <% end %> - <% if SiteConfig.auth_allowed?("github") %> - - <% end %> + <%= render partial: "shared/authentication/providers_sidebar" %>
<% end %> diff --git a/app/views/devise/registrations/_registration_form.html.erb b/app/views/devise/registrations/_registration_form.html.erb index 2d5fd11dc..c18871674 100644 --- a/app/views/devise/registrations/_registration_form.html.erb +++ b/app/views/devise/registrations/_registration_form.html.erb @@ -74,16 +74,8 @@

Great to have you

<% end %>
- <% if SiteConfig.auth_allowed?("twitter") %> - " class="sign-up-link" data-no-instant> - <%= inline_svg_tag("twitter-logo.svg", class: "icon-img", aria: true, title: "twitter logo") %> Sign In with Twitter - - <% end %> - <% if SiteConfig.auth_allowed?("github") %> - - <% end %> + <%= render partial: "shared/authentication/providers_registration_form" %> +

We require social login to prevent abuse.

Open Source 😇

diff --git a/app/views/layouts/_nav_menu.html.erb b/app/views/layouts/_nav_menu.html.erb index a7981a749..a2a608bca 100644 --- a/app/views/layouts/_nav_menu.html.erb +++ b/app/views/layouts/_nav_menu.html.erb @@ -3,27 +3,29 @@
- Dashboard - Write a Post - Reading List - Settings - Key Links + Dashboard + Write a Post + Reading List + Settings + Key Links
- Sign Out + Sign Out
<% else %>
- Sign In/Up - <% if SiteConfig.auth_allowed?("twitter") %> - " class="crayons-link crayons-link--block pl-5" data-no-instant>Via Twitter - <% end %> - <% if SiteConfig.auth_allowed?("github") %> - Via GitHub - <% end %> + + Sign In/Up + + + <%= render partial: "shared/authentication/providers_nav_menu" %>
- All about <%= community_name %> + All about <%= community_name %>
<% end %> diff --git a/app/views/layouts/_signup_modal.html.erb b/app/views/layouts/_signup_modal.html.erb index acd931434..1560505d4 100644 --- a/app/views/layouts/_signup_modal.html.erb +++ b/app/views/layouts/_signup_modal.html.erb @@ -6,21 +6,18 @@