Skip to content

Commit aef4c6e

Browse files
pbolingpboling
authored
Merge pull request #109 from omniauth/validation-regression
fix: options[:on_validation] regression
2 parents 38fa977 + afd8081 commit aef4c6e

File tree

3 files changed

+141
-27
lines changed

3 files changed

+141
-27
lines changed

‎.rubocop_todo.yml

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# This configuration was generated by
22
# `rubocop --auto-gen-config`
3-
# on 2021-02-14 08:34:14 UTC using RuboCop version 1.9.1.
3+
# on 2021-03-21 02:34:29 UTC using RuboCop version 1.11.0.
44
# The point is for the user to remove these configuration records
55
# one by one as the offenses are removed from the code base.
66
# Note that changes in the inspected code, or installation of new
@@ -34,7 +34,7 @@ Lint/NestedMethodDefinition:
3434
# Offense count: 1
3535
# Configuration parameters: IgnoredMethods, CountRepeatedAttributes.
3636
Metrics/AbcSize:
37-
Max: 42
37+
Max: 30
3838

3939
# Offense count: 1
4040
# Configuration parameters: CountComments, CountAsOne, ExcludedMethods, IgnoredMethods.
@@ -45,22 +45,17 @@ Metrics/BlockLength:
4545
# Offense count: 1
4646
# Configuration parameters: CountComments, CountAsOne.
4747
Metrics/ClassLength:
48-
Max: 140
49-
50-
# Offense count: 1
51-
# Configuration parameters: IgnoredMethods.
52-
Metrics/CyclomaticComplexity:
53-
Max: 9
48+
Max: 145
5449

5550
# Offense count: 7
5651
# Configuration parameters: CountComments, CountAsOne, ExcludedMethods, IgnoredMethods.
5752
Metrics/MethodLength:
58-
Max: 21
53+
Max: 20
5954

6055
# Offense count: 2
6156
# Configuration parameters: IgnoredMethods.
6257
Metrics/PerceivedComplexity:
63-
Max: 11
58+
Max: 9
6459

6560
# Offense count: 1
6661
Naming/AccessorMethodName:
@@ -131,14 +126,14 @@ RSpec/LeakyConstantDeclaration:
131126
RSpec/MessageSpies:
132127
EnforcedStyle: receive
133128

134-
# Offense count: 8
129+
# Offense count: 9
135130
RSpec/MultipleExpectations:
136131
Max: 4
137132

138-
# Offense count: 3
133+
# Offense count: 14
139134
# Configuration parameters: AllowSubject.
140135
RSpec/MultipleMemoizedHelpers:
141-
Max: 6
136+
Max: 8
142137

143138
# Offense count: 44
144139
# Configuration parameters: IgnoreSharedExamples.
@@ -148,9 +143,9 @@ RSpec/NamedSubject:
148143
- 'spec/omniauth/identity/models/couch_potato_spec.rb'
149144
- 'spec/omniauth/identity/models/mongoid_spec.rb'
150145

151-
# Offense count: 6
146+
# Offense count: 12
152147
RSpec/NestedGroups:
153-
Max: 4
148+
Max: 5
154149

155150
# Offense count: 4
156151
RSpec/StubbedMock:

‎lib/omniauth/strategies/identity.rb

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,11 @@ class Identity
2020
option :on_registration, nil # See #registration_phase
2121
option :on_failed_registration, nil # See #registration_phase
2222
option :locate_conditions, ->(req) { { model.auth_key => req['auth_key'] } }
23+
option :create_identity_link_text, 'Create an Identity'
24+
option :registration_failure_message, 'One or more fields were invalid'
25+
option :validation_failure_message, 'Validation failed'
26+
option :title, 'Identity Verification' # Title for Login Form
27+
option :registration_form_title, 'Register Identity' # Title for Registration Form
2328

2429
def request_phase
2530
if options[:on_login]
@@ -71,14 +76,14 @@ def registration_phase
7176
if model.respond_to?(:column_names) && model.column_names.include?('provider')
7277
attributes.reverse_merge!(provider: 'identity')
7378
end
79+
@identity = model.new(attributes)
7480
if saving_instead_of_creating?
75-
@identity = model.new(attributes)
7681
env['omniauth.identity'] = @identity
7782
if !validating? || valid?
7883
@identity.save
7984
registration_result
8085
else
81-
registration_failure('Validation failed')
86+
registration_failure(options[:validation_failure_message])
8287
end
8388
else
8489
deprecated_registration(attributes)
@@ -114,19 +119,19 @@ def model
114119

115120
def build_omniauth_login_form
116121
OmniAuth::Form.build(
117-
title: (options[:title] || 'Identity Verification'),
122+
title: options[:title],
118123
url: callback_path
119124
) do |f|
120125
f.text_field 'Login', 'auth_key'
121126
f.password_field 'Password', 'password'
122127
if options[:enable_registration]
123-
f.html "<p align='center'><a href='#{registration_path}'>Create an Identity</a></p>"
128+
f.html "<p align='center'><a href='#{registration_path}'>#{options[:create_identity_link_text]}</a></p>"
124129
end
125130
end
126131
end
127132

128133
def build_omniauth_registration_form(validation_message)
129-
OmniAuth::Form.build(title: 'Register Identity') do |f|
134+
OmniAuth::Form.build(title: options[:registration_form_title]) do |f|
130135
f.html "<p style='color:red'>#{validation_message}</p>" if validation_message
131136
options[:fields].each do |field|
132137
f.text_field field.to_s.capitalize, field.to_s
@@ -137,14 +142,14 @@ def build_omniauth_registration_form(validation_message)
137142
end
138143

139144
def saving_instead_of_creating?
140-
model.respond_to?(:save) && model.respond_to?(:persisted?)
145+
@identity.respond_to?(:save) && @identity.respond_to?(:persisted?)
141146
end
142147

143148
# Validates the model before it is persisted
144149
#
145150
# @return [truthy or falsey] :on_validation option is truthy or falsey
146151
def validating?
147-
options[:on_validation]
152+
!!options[:on_validation]
148153
end
149154

150155
# Validates the model before it is persisted
@@ -169,7 +174,7 @@ def registration_result
169174
env['PATH_INFO'] = callback_path
170175
callback_phase
171176
else
172-
registration_failure('One or more fields were invalid')
177+
registration_failure(options[:registration_failure_message])
173178
end
174179
end
175180

‎spec/omniauth/strategies/identity_spec.rb

Lines changed: 118 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,9 @@
33
RSpec.describe OmniAuth::Strategies::Identity do
44
attr_accessor :app
55

6-
let(:auth_hash) { last_response.headers['env']['omniauth.auth'] }
7-
let(:identity_hash) { last_response.headers['env']['omniauth.identity'] }
6+
let(:env_hash) { last_response.headers['env'] }
7+
let(:auth_hash) { env_hash['omniauth.auth'] }
8+
let(:identity_hash) { env_hash['omniauth.identity'] }
89
let(:identity_options) { {} }
910
let(:anon_ar) do
1011
AnonymousActiveRecord.generate(
@@ -193,7 +194,7 @@ def set_app!(identity_options = {})
193194
end
194195
end
195196

196-
context 'with successful creation' do
197+
context 'with good identity' do
197198
let(:properties) do
198199
{
199200
name: 'Awesome Dude',
@@ -209,9 +210,66 @@ def set_app!(identity_options = {})
209210
expect(auth_hash['uid']).to match(/\d+/)
210211
expect(auth_hash['provider']).to eq('identity')
211212
end
213+
214+
context 'with on_validation proc' do
215+
let(:identity_options) do
216+
{ model: anon_ar, on_validation: on_validation_proc }
217+
end
218+
let(:on_validation_proc) do
219+
lambda { |_env|
220+
false
221+
}
222+
end
223+
224+
context 'when validation fails' do
225+
it 'does not set the env hash' do
226+
post '/auth/identity/register', properties
227+
expect(env_hash).to eq(nil)
228+
end
229+
230+
it 'renders registration form' do
231+
post '/auth/identity/register', properties
232+
expect(last_response.body).to be_include(described_class.default_options[:registration_form_title])
233+
end
234+
235+
it 'displays validation failure message' do
236+
post '/auth/identity/register', properties
237+
expect(last_response.body).to be_include(described_class.default_options[:validation_failure_message])
238+
end
239+
end
240+
241+
context 'when validation succeeds' do
242+
let(:on_validation_proc) do
243+
lambda { |_env|
244+
true
245+
}
246+
end
247+
248+
it 'sets the auth hash' do
249+
post '/auth/identity/register', properties
250+
expect(auth_hash['uid']).to match(/\d+/)
251+
expect(auth_hash['provider']).to eq('identity')
252+
end
253+
254+
it 'does not render registration form' do
255+
post '/auth/identity/register', properties
256+
expect(last_response.body).not_to be_include(described_class.default_options[:registration_form_title])
257+
end
258+
259+
it 'does not display validation failure message' do
260+
post '/auth/identity/register', properties
261+
expect(last_response.body).not_to be_include(described_class.default_options[:validation_failure_message])
262+
end
263+
264+
it 'does not display registration failure message' do
265+
post '/auth/identity/register', properties
266+
expect(last_response.body).not_to be_include(described_class.default_options[:registration_failure_message])
267+
end
268+
end
269+
end
212270
end
213271

214-
context 'with invalid identity' do
272+
context 'with bad identity' do
215273
let(:properties) do
216274
{
217275
name: 'Awesome Dude',
@@ -249,6 +307,62 @@ def set_app!(identity_options = {})
249307
expect(last_response.body).not_to be_include('One or more fields were invalid')
250308
end
251309
end
310+
311+
context 'with on_validation proc' do
312+
let(:identity_options) do
313+
{ model: anon_ar, on_validation: on_validation_proc }
314+
end
315+
let(:on_validation_proc) do
316+
lambda { |_env|
317+
false
318+
}
319+
end
320+
321+
context 'when validation fails' do
322+
it 'does not set the env hash' do
323+
post '/auth/identity/register', properties
324+
expect(env_hash).to eq(nil)
325+
end
326+
327+
it 'renders registration form' do
328+
post '/auth/identity/register', properties
329+
expect(last_response.body).to be_include(described_class.default_options[:registration_form_title])
330+
end
331+
332+
it 'displays validation failure message' do
333+
post '/auth/identity/register', properties
334+
expect(last_response.body).to be_include(described_class.default_options[:validation_failure_message])
335+
end
336+
end
337+
338+
context 'when validation succeeds' do
339+
let(:on_validation_proc) do
340+
lambda { |_env|
341+
true
342+
}
343+
end
344+
345+
it 'does not set the env hash' do
346+
post '/auth/identity/register', properties
347+
expect(env_hash).to eq(nil)
348+
end
349+
350+
it 'renders registration form' do
351+
post '/auth/identity/register', properties
352+
expect(last_response.body).to be_include(described_class.default_options[:registration_form_title])
353+
end
354+
355+
it 'does not display validation failure message' do
356+
post '/auth/identity/register', properties
357+
expect(last_response.body).not_to be_include(described_class.default_options[:validation_failure_message])
358+
end
359+
360+
it 'display registration failure message' do
361+
post '/auth/identity/register', properties
362+
expect(last_response.body).to be_include(described_class.default_options[:registration_failure_message])
363+
end
364+
end
365+
end
252366
end
253367
end
254368
end

0 commit comments

Comments
 (0)