diff --git a/.rubocop.yml b/.rubocop.yml index 9b59277..abcd1de 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 b4a43e3..4033beb 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -40,12 +40,12 @@ Metrics/AbcSize: # Configuration parameters: CountComments, CountAsOne, ExcludedMethods, IgnoredMethods. # IgnoredMethods: refine Metrics/BlockLength: - Max: 27 + Max: 28 # Offense count: 1 # Configuration parameters: CountComments, CountAsOne. Metrics/ClassLength: - Max: 117 + Max: 140 # Offense count: 1 # Configuration parameters: IgnoredMethods. diff --git a/CHANGELOG.md b/CHANGELOG.md index ad90600..4bc62a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +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 diff --git a/Gemfile b/Gemfile index 11f3f6c..c026a79 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 010dfb5..e6a238c 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/version.rb b/lib/omniauth-identity/version.rb index b931ae8..141fd26 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 diff --git a/lib/omniauth/identity/model.rb b/lib/omniauth/identity/model.rb index 3730cc0..ec18166 100644 --- a/lib/omniauth/identity/model.rb +++ b/lib/omniauth/identity/model.rb @@ -4,23 +4,31 @@ 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 + 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 + extend Gem::Deprecate # Authenticate a user with the given key and password. # @@ -43,6 +51,53 @@ def auth_key(method = false) @auth_key || 'email' end + + # 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. + # @since 3.0.5 + 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 + + # Persists a new Identity object to the ORM. + # Default raises an error. Override as needed per ORM. + # + # @abstract + # @return [Model] An instance of the identity model class. + # @since 3.0.5 + 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. + # @since 3.0.5 + def persisted? + raise NotImplementedError unless defined?(super) + + super end # Returns self if the provided password is correct, false @@ -55,22 +110,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 +152,21 @@ def auth_key=(value) raise NotImplementedError end end + + # 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 f58fdc5..9431095 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 406fc12..ea2c857 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 390c2a1..8979b02 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 b1638d1..7f65881 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 diff --git a/lib/omniauth/strategies/identity.rb b/lib/omniauth/strategies/identity.rb index 1808455..bf5362a 100644 --- a/lib/omniauth/strategies/identity.rb +++ b/lib/omniauth/strategies/identity.rb @@ -6,8 +6,8 @@ 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] # Primary Feature Switches: @@ -65,29 +65,23 @@ 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') 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 + deprecated_registration(attributes) end end @@ -142,15 +136,53 @@ 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 + + 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