From 167e869e8da820bf61c6cebed944ee532b303c26 Mon Sep 17 00:00:00 2001 From: Peter Boling Date: Fri, 19 Mar 2021 20:08:06 -0600 Subject: [PATCH 1/6] Define API for classes utilizing OmniAuth::Identity::Model --- .rubocop.yml | 7 ++ .rubocop_todo.yml | 2 +- CHANGELOG.md | 2 + Gemfile | 1 + README.md | 18 ++-- lib/omniauth/identity/model.rb | 105 ++++++++++++++----- lib/omniauth/identity/models/couch_potato.rb | 1 + lib/omniauth/identity/models/mongoid.rb | 1 + lib/omniauth/identity/models/no_brainer.rb | 1 + lib/omniauth/identity/models/sequel.rb | 11 ++ 10 files changed, 114 insertions(+), 35 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 9b59277f..abcd1de9 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -7,6 +7,7 @@ require: - 'rubocop-performance' - 'rubocop-rake' - 'rubocop-rspec' + - 'rubocop-sequel' AllCops: NewCops: enable @@ -25,3 +26,9 @@ Metrics/BlockLength: - shared_examples_for - namespace - draw + +Sequel/SaveChanges: + Enabled: false + +Lint/UselessMethodDefinition: + Enabled: false diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index b4a43e3e..8b17edf2 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -40,7 +40,7 @@ Metrics/AbcSize: # Configuration parameters: CountComments, CountAsOne, ExcludedMethods, IgnoredMethods. # IgnoredMethods: refine Metrics/BlockLength: - Max: 27 + Max: 28 # Offense count: 1 # Configuration parameters: CountComments, CountAsOne. diff --git a/CHANGELOG.md b/CHANGELOG.md index ad906000..d7b0a063 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +- Fix breaking changes introduced by [#86's](https://github.com/omniauth/omniauth-identity/pull/86) introduction of `:on_validation` + ## [3.0.4] - 2021-02-14 ### Added diff --git a/Gemfile b/Gemfile index 11f3f6c1..c026a799 100644 --- a/Gemfile +++ b/Gemfile @@ -39,6 +39,7 @@ group :development, :test do gem 'rubocop-performance', platform: :mri gem 'rubocop-rake', platform: :mri gem 'rubocop-rspec', platform: :mri + gem 'rubocop-sequel', platform: :mri gem 'simplecov', '~> 0.21', platform: :mri end diff --git a/README.md b/README.md index 010dfb54..e6a238c5 100644 --- a/README.md +++ b/README.md @@ -249,21 +249,21 @@ always break things! From the code - here are the options we have for you, a couple of which are documented above, and the rest are documented... in the specs we hope!? ``` - option :fields, %i[name email] +option :fields, %i[name email] # Primary Feature Switches: - option :enable_registration, true # See #other_phase and #request_phase - option :enable_login, true # See #other_phase +option :enable_registration, true # See #other_phase and #request_phase +option :enable_login, true # See #other_phase # Customization Options: - option :on_login, nil # See #request_phase - option :on_validation, nil # See #registration_phase - option :on_registration, nil # See #registration_phase - option :on_failed_registration, nil # See #registration_phase - option :locate_conditions, ->(req) { { model.auth_key => req['auth_key'] } } +option :on_login, nil # See #request_phase +option :on_validation, nil # See #registration_phase +option :on_registration, nil # See #registration_phase +option :on_failed_registration, nil # See #registration_phase +option :locate_conditions, ->(req) { { model.auth_key => req['auth_key'] } } ``` -Please contribute some documentation if you have the gumption! The maintainer's time is limited, and sometimes the authors of PRs with new options don't update the _this_ readme. 😭 +Please contribute some documentation if you have the gumption! The maintainer's time is limited, and sometimes the authors of PRs with new options don't update the _this_ readme. 😭 ## License diff --git a/lib/omniauth/identity/model.rb b/lib/omniauth/identity/model.rb index 3730cc0c..e0cd1db7 100644 --- a/lib/omniauth/identity/model.rb +++ b/lib/omniauth/identity/model.rb @@ -8,20 +8,13 @@ module Identity # abstract must be implemented in the including class for things to # work properly. module Model + SCHEMA_ATTRIBUTES = %w[name email nickname first_name last_name location description image phone].freeze + def self.included(base) base.extend ClassMethods end module ClassMethods - # Locate an identity given its unique login key. - # - # @abstract - # @param [String] key The unique login key. - # @return [Model] An instance of the identity model class. - def locate(key) - raise NotImplementedError - end - # Authenticate a user with the given key and password. # # @param [String] key The unique login key provided for a given identity. @@ -43,6 +36,67 @@ def auth_key(method = false) @auth_key || 'email' end + + ### Singleton API for classes utilizing the OmniAuth::Identity::Model + # + # * create(*args) + # * locate(key) + + # Persists a new Identity object to the ORM. + # Defaults to calling super. Override as needed per ORM. + # + # @abstract + # @param [Hash] args Attributes of the new instance. + # @return [Model] An instance of the identity model class. + def create(*args) + raise NotImplementedError unless defined?(super) + + super + end + + # Locate an identity given its unique login key. + # + # @abstract + # @param [String] key The unique login key. + # @return [Model] An instance of the identity model class. + def locate(key) + raise NotImplementedError + end + # + ### END Singleton API for classes utilizing the OmniAuth::Identity::Model + end + + ### Instance API for classes utilizing the OmniAuth::Identity::Model + # + # * save + # * persisted? + # * authenticate(password) + # * uid + # * auth_key - Normally defined by the macro `auth_key` + # * auth_key=(value) - Normally defined by the macro `auth_key` + # + + # Persists a new Identity object to the ORM. + # Default raises an error. Override as needed per ORM. + # + # @since 3.0.5 + # @abstract + # @return [Model] An instance of the identity model class. + def save + raise NotImplementedError unless defined?(super) + + super + end + + # Checks if the Identity object is persisted in the ORM. + # Defaults to calling super. Override as needed per ORM. + # + # @abstract + # @return [true or false] true if object exists, false if not. + def persisted? + raise NotImplementedError unless defined?(super) + + super end # Returns self if the provided password is correct, false @@ -55,22 +109,6 @@ def authenticate(password) raise NotImplementedError end - SCHEMA_ATTRIBUTES = %w[name email nickname first_name last_name location description image phone].freeze - # A hash of as much of the standard OmniAuth schema as is stored - # in this particular model. By default, this will call instance - # methods for each of the attributes it needs in turn, ignoring - # any for which `#respond_to?` is `false`. - # - # If `first_name`, `nickname`, and/or `last_name` is provided but - # `name` is not, it will be automatically calculated. - # - # @return [Hash] A string-keyed hash of user information. - def info - SCHEMA_ATTRIBUTES.each_with_object({}) do |attribute, hash| - hash[attribute] = send(attribute) if respond_to?(attribute) - end - end - # An identifying string that must be globally unique to the # application. Defaults to stringifying the `id` method. # @@ -113,6 +151,23 @@ def auth_key=(value) raise NotImplementedError end end + # + ### END Instance API for classes utilizing the OmniAuth::Identity::Model + + # A hash of as much of the standard OmniAuth schema as is stored + # in this particular model. By default, this will call instance + # methods for each of the attributes it needs in turn, ignoring + # any for which `#respond_to?` is `false`. + # + # If `first_name`, `nickname`, and/or `last_name` is provided but + # `name` is not, it will be automatically calculated. + # + # @return [Hash] A string-keyed hash of user information. + def info + SCHEMA_ATTRIBUTES.each_with_object({}) do |attribute, hash| + hash[attribute] = send(attribute) if respond_to?(attribute) + end + end end end end diff --git a/lib/omniauth/identity/models/couch_potato.rb b/lib/omniauth/identity/models/couch_potato.rb index f58fdc58..94310955 100644 --- a/lib/omniauth/identity/models/couch_potato.rb +++ b/lib/omniauth/identity/models/couch_potato.rb @@ -6,6 +6,7 @@ module OmniAuth module Identity module Models # can not be named CouchPotato since there is a class with that name + # NOTE: CouchPotato is based on ActiveModel. module CouchPotatoModule def self.included(base) base.class_eval do diff --git a/lib/omniauth/identity/models/mongoid.rb b/lib/omniauth/identity/models/mongoid.rb index 406fc12d..ea2c8577 100644 --- a/lib/omniauth/identity/models/mongoid.rb +++ b/lib/omniauth/identity/models/mongoid.rb @@ -5,6 +5,7 @@ module OmniAuth module Identity module Models + # NOTE: Mongoid is based on ActiveModel. module Mongoid def self.included(base) base.class_eval do diff --git a/lib/omniauth/identity/models/no_brainer.rb b/lib/omniauth/identity/models/no_brainer.rb index 390c2a13..8979b024 100644 --- a/lib/omniauth/identity/models/no_brainer.rb +++ b/lib/omniauth/identity/models/no_brainer.rb @@ -6,6 +6,7 @@ module OmniAuth module Identity module Models # http://nobrainer.io/ an ORM for RethinkDB + # NOTE: NoBrainer is based on ActiveModel. module NoBrainer def self.included(base) base.class_eval do diff --git a/lib/omniauth/identity/models/sequel.rb b/lib/omniauth/identity/models/sequel.rb index b1638d1b..7f65881c 100644 --- a/lib/omniauth/identity/models/sequel.rb +++ b/lib/omniauth/identity/models/sequel.rb @@ -6,6 +6,9 @@ module OmniAuth module Identity module Models # http://sequel.jeremyevans.net/ an SQL ORM + # NOTE: Sequel is *not* based on ActiveModel, but supports the API we need, except for `persisted`: + # * create + # * save, but save is deprecated in favor of `save_changes` module Sequel def self.included(base) base.class_eval do @@ -29,6 +32,14 @@ def self.auth_key=(key) def self.locate(search_hash) where(search_hash).first end + + def persisted? + exists? + end + + def save + save_changes + end end end end From 3a46174563cc94695cf5dd13bc70ec50091abeac Mon Sep 17 00:00:00 2001 From: Peter Boling Date: Fri, 19 Mar 2021 21:23:16 -0600 Subject: [PATCH 2/6] Improve logic --- lib/omniauth/identity/model.rb | 41 +++++++++---------- lib/omniauth/strategies/identity.rb | 61 ++++++++++++++++++++--------- 2 files changed, 61 insertions(+), 41 deletions(-) diff --git a/lib/omniauth/identity/model.rb b/lib/omniauth/identity/model.rb index e0cd1db7..bb4f6993 100644 --- a/lib/omniauth/identity/model.rb +++ b/lib/omniauth/identity/model.rb @@ -4,9 +4,22 @@ module OmniAuth module Identity # This module provides an includable interface for implementing the # necessary API for OmniAuth Identity to properly locate identities - # and provide all necessary information. All methods marked as - # abstract must be implemented in the including class for things to - # work properly. + # and provide all necessary information. + # + # All methods marked as abstract must be implemented in the + # including class for things to work properly. + # + ### Singleton API + # + # * locate(key) + # * create(*args) - Deprecated in v3.0.5; Will be removed in v4.0 + # + ### Instance API + # + # * save + # * persisted? + # * authenticate(password) + # module Model SCHEMA_ATTRIBUTES = %w[name email nickname first_name last_name location description image phone].freeze @@ -15,6 +28,8 @@ def self.included(base) end module ClassMethods + extend Gem::Deprecate + # Authenticate a user with the given key and password. # # @param [String] key The unique login key provided for a given identity. @@ -37,14 +52,10 @@ def auth_key(method = false) @auth_key || 'email' end - ### Singleton API for classes utilizing the OmniAuth::Identity::Model - # - # * create(*args) - # * locate(key) - # Persists a new Identity object to the ORM. # Defaults to calling super. Override as needed per ORM. # + # @deprecated v4.0 will begin using {#new} with {#save} instead. # @abstract # @param [Hash] args Attributes of the new instance. # @return [Model] An instance of the identity model class. @@ -62,20 +73,8 @@ def create(*args) def locate(key) raise NotImplementedError end - # - ### END Singleton API for classes utilizing the OmniAuth::Identity::Model end - ### Instance API for classes utilizing the OmniAuth::Identity::Model - # - # * save - # * persisted? - # * authenticate(password) - # * uid - # * auth_key - Normally defined by the macro `auth_key` - # * auth_key=(value) - Normally defined by the macro `auth_key` - # - # Persists a new Identity object to the ORM. # Default raises an error. Override as needed per ORM. # @@ -151,8 +150,6 @@ def auth_key=(value) raise NotImplementedError end end - # - ### END Instance API for classes utilizing the OmniAuth::Identity::Model # A hash of as much of the standard OmniAuth schema as is stored # in this particular model. By default, this will call instance diff --git a/lib/omniauth/strategies/identity.rb b/lib/omniauth/strategies/identity.rb index 18084553..11f345da 100644 --- a/lib/omniauth/strategies/identity.rb +++ b/lib/omniauth/strategies/identity.rb @@ -7,7 +7,6 @@ module Strategies # use for external OmniAuth providers. class Identity include OmniAuth::Strategy - option :fields, %i[name email] # Primary Feature Switches: @@ -71,23 +70,20 @@ def registration_phase if model.respond_to?(:column_names) && model.column_names.include?('provider') attributes.reverse_merge!(provider: 'identity') end - @identity = model.new(attributes) - - # on_validation may run a Captcha or other validation mechanism - # Must return true when validation passes, false otherwise - if options[:on_validation] && !options[:on_validation].call(env: env) - if options[:on_failed_registration] - env['omniauth.identity'] = @identity - options[:on_failed_registration].call(env) + if saving_instead_of_creating? + @identity = model.new(attributes) + env['omniauth.identity'] = @identity + if !validating? || valid? + @identity.save + registration_result else - validation_message = 'Validation failed' - registration_form(validation_message) + registration_failure('Validation failed') end - elsif @identity.save && @identity.persisted? - env['PATH_INFO'] = callback_path - callback_phase else - show_custom_options_or_default + warn "[DEPRECATION] Please define '#{model.class}#save'. Behavior based on '#{model.class}.create' will be removed in omniauth-identity v4.0. See lib/omniauth/identity/model.rb" + @identity = model.create(attributes) + env['omniauth.identity'] = @identity + registration_result end end @@ -142,13 +138,40 @@ def build_omniauth_registration_form(validation_message) end end - def show_custom_options_or_default + def saving_instead_of_creating? + model.respond_to?(:save) && model.respond_to?(:persisted?) + end + + # Validates the model before it is persisted + # + # @return [truthy or falsey] :on_validation option is truthy or falsey + def validating? + options[:on_validation] + end + + # Validates the model before it is persisted + # + # @return [true or false] result of :on_validation call + def valid? + # on_validation may run a Captcha or other validation mechanism + # Must return true when validation passes, false otherwise + !!options[:on_validation].call(env: env) + end + + def registration_failure(message) if options[:on_failed_registration] - env['omniauth.identity'] = @identity options[:on_failed_registration].call(env) else - validation_message = 'One or more fields were invalid' - registration_form(validation_message) + registration_form(message) + end + end + + def registration_result + if @identity.persisted? + env['PATH_INFO'] = callback_path + callback_phase + else + registration_failure('One or more fields were invalid') end end end From 39a8d4bad9cab7180f6eda4303fd97e34b0991dd Mon Sep 17 00:00:00 2001 From: Peter Boling Date: Fri, 19 Mar 2021 21:29:34 -0600 Subject: [PATCH 3/6] Linting and refactor --- .rubocop_todo.yml | 2 +- lib/omniauth/strategies/identity.rb | 19 ++++++++++++++----- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 8b17edf2..4033beb9 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -45,7 +45,7 @@ Metrics/BlockLength: # Offense count: 1 # Configuration parameters: CountComments, CountAsOne. Metrics/ClassLength: - Max: 117 + Max: 140 # Offense count: 1 # Configuration parameters: IgnoredMethods. diff --git a/lib/omniauth/strategies/identity.rb b/lib/omniauth/strategies/identity.rb index 11f345da..bf5362ac 100644 --- a/lib/omniauth/strategies/identity.rb +++ b/lib/omniauth/strategies/identity.rb @@ -6,6 +6,7 @@ module Strategies # user authentication using the same process flow that you # use for external OmniAuth providers. class Identity + DEFAULT_REGISTRATION_FIELDS = %i[password password_confirmation].freeze include OmniAuth::Strategy option :fields, %i[name email] @@ -64,7 +65,7 @@ def registration_form(validation_message = nil) end def registration_phase - attributes = (options[:fields] + %i[password password_confirmation]).each_with_object({}) do |k, h| + attributes = (options[:fields] + DEFAULT_REGISTRATION_FIELDS).each_with_object({}) do |k, h| h[k] = request[k.to_s] end if model.respond_to?(:column_names) && model.column_names.include?('provider') @@ -80,10 +81,7 @@ def registration_phase registration_failure('Validation failed') end else - warn "[DEPRECATION] Please define '#{model.class}#save'. Behavior based on '#{model.class}.create' will be removed in omniauth-identity v4.0. See lib/omniauth/identity/model.rb" - @identity = model.create(attributes) - env['omniauth.identity'] = @identity - registration_result + deprecated_registration(attributes) end end @@ -174,6 +172,17 @@ def registration_result registration_failure('One or more fields were invalid') end end + + def deprecated_registration(attributes) + warn <<~CREATEDEP + [DEPRECATION] Please define '#{model.class}#save'. + Behavior based on '#{model.class}.create' will be removed in omniauth-identity v4.0. + See lib/omniauth/identity/model.rb + CREATEDEP + @identity = model.create(attributes) + env['omniauth.identity'] = @identity + registration_result + end end end end From 70c83f28d97e51c0da99f0843766caa0a5a239c2 Mon Sep 17 00:00:00 2001 From: Peter Boling Date: Fri, 19 Mar 2021 21:37:00 -0600 Subject: [PATCH 4/6] Documentation @since tags --- lib/omniauth/identity/model.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/omniauth/identity/model.rb b/lib/omniauth/identity/model.rb index bb4f6993..ec181664 100644 --- a/lib/omniauth/identity/model.rb +++ b/lib/omniauth/identity/model.rb @@ -59,6 +59,7 @@ def auth_key(method = false) # @abstract # @param [Hash] args Attributes of the new instance. # @return [Model] An instance of the identity model class. + # @since 3.0.5 def create(*args) raise NotImplementedError unless defined?(super) @@ -78,9 +79,9 @@ def locate(key) # Persists a new Identity object to the ORM. # Default raises an error. Override as needed per ORM. # - # @since 3.0.5 # @abstract # @return [Model] An instance of the identity model class. + # @since 3.0.5 def save raise NotImplementedError unless defined?(super) @@ -92,6 +93,7 @@ def save # # @abstract # @return [true or false] true if object exists, false if not. + # @since 3.0.5 def persisted? raise NotImplementedError unless defined?(super) From 5e246de682c602bfb190f1f51f41d7016de24686 Mon Sep 17 00:00:00 2001 From: Peter Boling Date: Fri, 19 Mar 2021 21:41:41 -0600 Subject: [PATCH 5/6] Documentation release 3.0.5 --- CHANGELOG.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d7b0a063..4bc62a8b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,8 +8,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [3.0.5] - 2021-03-19 + +### Fixed + - Fix breaking changes introduced by [#86's](https://github.com/omniauth/omniauth-identity/pull/86) introduction of `:on_validation` +### Added + +- Define `#save`, `#persisted?` and `::create` on `Omniauth::Identity::Model` +- Add `@since` YARD tags to interface methods +- Refactor `Omniauth::Strategies::Identity.registration_phase` to support `Omniauth::Identity::Model`-inheriting classes that do not define `#save`. + - This support will be dropped in v4.0. + ## [3.0.4] - 2021-02-14 ### Added From a972ef9b08013a7a8eda644b3b2cec773a98ae36 Mon Sep 17 00:00:00 2001 From: Peter Boling Date: Fri, 19 Mar 2021 21:41:53 -0600 Subject: [PATCH 6/6] Bump omniauth-identity to 3.0.5 --- lib/omniauth-identity/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/omniauth-identity/version.rb b/lib/omniauth-identity/version.rb index b931ae84..141fd26b 100644 --- a/lib/omniauth-identity/version.rb +++ b/lib/omniauth-identity/version.rb @@ -2,6 +2,6 @@ module OmniAuth module Identity - VERSION = '3.0.4' + VERSION = '3.0.5' end end