Skip to content

Commit 40f04ab

Browse files
lodewigesCopilot
andauthored
Add to option to login with an emailadres (#1196)
* Initial commit of email login feature * Update app/models/sofia_account.rb Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * add case insensitive email lookup in find_by_login * rename helper * add some extra tests * fix test * fix some test * changes tests * reduce the amount of tests * fix tests * add end tokens * fix lint * fix some more tests * make some changes to tests * fix lint * fix tests * introduce fixes * fix the seeding * fix lint * make a exception for rubocop * make the emails simpeler --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent aea52b0 commit 40f04ab

File tree

8 files changed

+160
-14
lines changed

8 files changed

+160
-14
lines changed

app/controllers/sofia_accounts_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ def new_activation_link # rubocop:disable Metrics/AbcSize, Metrics/MethodLength,
126126

127127
def forgot_password # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
128128
generic_message = 'Als dit account bestaat, is er een email verstuurd.'
129-
sofia_account = SofiaAccount.find_by(username: params.require(:username))
129+
sofia_account = SofiaAccount.find_for_login(params.require(:username))
130130
user = sofia_account&.user
131131

132132
if user&.email.present?

app/models/sofia_account.rb

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,4 +32,15 @@ def reset_password_url(activation_token)
3232
default_options = Rails.application.config.action_mailer.default_url_options
3333
URI::Generic.build(default_options.merge(path: "/sofia_accounts/#{id}/reset_password", query: params.to_query)).to_s
3434
end
35+
36+
def self.find_for_login(identifier)
37+
return nil if identifier.blank?
38+
39+
trimmed = identifier.to_s.strip
40+
where('LOWER(username) = LOWER(?)', trimmed).first || User.find_by('LOWER(email) = LOWER(?)', trimmed)&.sofia_account
41+
end
42+
43+
def self.resolve_login_identifier(identifier)
44+
find_for_login(identifier)&.username
45+
end
3546
end

app/views/sofia_accounts/forgot_password_view.html.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
<div class="card-body px-4 py-3 ">
88
<%= simple_form_for :sofia_account, url: forgot_password_sofia_accounts_path, method: :post, wrapper: :vertical_form do |f| %>
99
<p class="text-muted mb-4">Door dit formulier in te vullen ontvangt u een email met een link om een nieuw wachwoord in te stellen.</p>
10-
<%= f.input :username, as: :string, label: "Gebruikersnaam", input_html: {name: 'username', class: 'sofia-account-input'} %>
10+
<%= f.input :username, as: :string, label: "Gebruikersnaam of e-mailadres", input_html: {name: 'username', class: 'sofia-account-input'} %>
1111
<div class="d-grid mt-4">
1212
<%= f.button :submit, 'Link aanvragen', method: :post, class: 'btn btn-primary mt-1' %>
1313
</div>

app/views/sofia_accounts/login.html.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
<h4 class="card-title">Inloggen</h4>
2222
</div>
2323
<div class="card-body px-4 py-3">
24-
<%= f.input :auth_key, as: :string, label: "Gebruikersnaam", input_html: {name: 'auth_key', class: 'sofia-account-input'} %>
24+
<%= f.input :auth_key, as: :string, label: "Gebruikersnaam of e-mailadres", input_html: {name: 'auth_key', class: 'sofia-account-input'} %>
2525
<%= f.input :password, as: :password, label: "Wachtwoord", hint: 'Wachtwoord vergeten?', wrapper: :with_hint_link, input_html: {name: 'password', class: 'sofia-account-input'}, hint_html: {href: forgot_password_view_sofia_accounts_path} %>
2626
<div class="d-grid mt-4">
2727
<button class="btn btn-primary" type="submit" formaction='javascript:authenticate_login();' id="login_submit_button">Inloggen</button>

config/initializers/devise.rb

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@
1818
config.omniauth :amber_oauth2, Rails.application.config.x.amber_client_id,
1919
Rails.application.config.x.amber_client_secret
2020
config.omniauth :identity, model: SofiaAccount, fields: %i[username user_id],
21-
locate_conditions: ->(req) { { SofiaAccount.auth_key => req.params['auth_key'] } },
21+
locate_conditions: lambda { |req|
22+
resolved_username = SofiaAccount.resolve_login_identifier(req.params['auth_key'])
23+
{ SofiaAccount.auth_key => resolved_username || req.params['auth_key'] }
24+
},
2225
on_login: lambda { |e|
2326
SofiaAccountsController.action(:omniauth_redirect_login).call(e)
2427
},

db/seeds.rb

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,10 @@
2424
]
2525

2626
users = dutch_names.map do |name|
27-
FactoryBot.create(:user, name:)
27+
email = "#{name.downcase.gsub(/ van der | de | van /i, ' ').tr(' ', '.')}@example.com"
28+
FactoryBot.create(:user, name:, email:)
2829
end
29-
users << FactoryBot.create(:user, name: 'Benjamin Knopje', birthday: 16.years.ago)
30+
users << FactoryBot.create(:user, name: 'Benjamin Knopje', email: 'benjamin.knopje@example.com', birthday: 16.years.ago)
3031

3132
p 'Seeding activities...'
3233
# Recente activiteiten
@@ -44,16 +45,18 @@
4445
{ title: 'Dies Natalis', days_ago: 90 }
4546
]
4647

47-
historical_activities.each do |hist_act|
48-
start_time = hist_act[:days_ago].days.ago.beginning_of_hour
49-
end_time = (hist_act[:days_ago] - 1).days.ago.beginning_of_hour
48+
historical_activities_list = historical_activities.map do |hist_act|
49+
past_start_time = hist_act[:days_ago].days.ago.beginning_of_hour
50+
past_end_time = (hist_act[:days_ago] - 1).days.ago.beginning_of_hour
51+
52+
# Create activity with future times first to pass validation
5053
activity = FactoryBot.create(:activity,
5154
title: hist_act[:title],
52-
start_time:,
53-
end_time:,
5455
price_list: price_lists.sample,
5556
created_by: users.sample)
57+
5658
activities << activity
59+
{ activity:, past_start_time:, past_end_time: }
5760
end
5861

5962
p 'Seeding orders...'
@@ -88,6 +91,14 @@
8891
end
8992
end
9093

94+
# Lock historical activities at the very end after all orders and credit mutations are created
95+
p 'Locking historical activities...'
96+
historical_activities_list.each do |hist_act|
97+
# rubocop:disable Rails/SkipsModelValidations
98+
hist_act[:activity].update_columns(start_time: hist_act[:past_start_time], end_time: hist_act[:past_end_time])
99+
# rubocop:enable Rails/SkipsModelValidations
100+
end
101+
91102
p 'Seeding invoices'
92103
FactoryBot.create_list(:invoice, 3, :with_rows)
93104

@@ -102,17 +113,17 @@
102113
p 'Seeding Sofia accounts...'
103114
# Use environment variable for password or fallback to default for development
104115

105-
treasurer_user = FactoryBot.create(:user, :sofia_account, name: 'Penningmeester Test')
116+
treasurer_user = FactoryBot.create(:user, :sofia_account, name: 'Penningmeester Test', email: 'penningmeester@example.com')
106117
SofiaAccount.create!(username: 'penningmeester', password: 'password1234', user: treasurer_user)
107118
treasurer_role = Role.create(role_type: :treasurer)
108119
treasurer_user.roles << treasurer_role
109120

110-
main_bartender_user = FactoryBot.create(:user, :sofia_account, name: 'Hoofdtapper Test')
121+
main_bartender_user = FactoryBot.create(:user, :sofia_account, name: 'Hoofdtapper Test', email: 'hoofdtapper@example.com')
111122
SofiaAccount.create!(username: 'hoofdtapper', password: 'password1234', user: main_bartender_user)
112123
main_bartender_role = Role.create(role_type: :main_bartender)
113124
main_bartender_user.roles << main_bartender_role
114125

115-
renting_manager_user = FactoryBot.create(:user, :sofia_account, name: 'Verhuur Test')
126+
renting_manager_user = FactoryBot.create(:user, :sofia_account, name: 'Verhuur Test', email: 'verhuur@example.com')
116127
SofiaAccount.create!(username: 'verhuur', password: 'password1234', user: renting_manager_user)
117128
renting_manager_role = Role.create(role_type: :renting_manager)
118129
renting_manager_user.roles << renting_manager_role

spec/controllers/callbacks_controller/sofia_account_spec.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
end
3232

3333
it 'sends a json response' do
34+
expect(response).to have_http_status(:ok)
3435
expect(response.content_type).to eq 'application/json; charset=utf-8'
3536
expect(response.parsed_body['state']).to eq 'password_prompt'
3637
expect(response.parsed_body['error_message']).to eq 'Inloggen mislukt. De ingevulde gegevens zijn incorrect.'

spec/models/sofia_account_spec.rb

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,4 +96,124 @@
9696
describe 'reset_password_url' do
9797
it { expect(sofia_account.reset_password_url(sofia_account.user.activation_token)).to eq "http://testhost:1337/sofia_accounts/#{sofia_account.id}/reset_password?activation_token=#{sofia_account.user.activation_token}" }
9898
end
99+
100+
describe '.find_for_login' do
101+
let!(:account) { create(:sofia_account, username: 'testuser', password: 'password1234') }
102+
let!(:account_with_email) { create(:sofia_account, password: 'password1234') }
103+
104+
context 'when searching by username' do
105+
it 'finds account by exact username match' do
106+
result = described_class.find_for_login('testuser')
107+
expect(result).to eq(account)
108+
end
109+
110+
it 'finds account by username regardless of case' do
111+
result = described_class.find_for_login('TESTUSER')
112+
expect(result).to eq(account)
113+
end
114+
115+
it 'finds account by username with whitespace trimmed' do
116+
result = described_class.find_for_login(' testuser ')
117+
expect(result).to eq(account)
118+
end
119+
end
120+
121+
context 'when searching by email' do
122+
before do
123+
account_with_email.user.update!(email: 'user@example.com')
124+
end
125+
126+
it 'finds account by exact email match' do
127+
result = described_class.find_for_login('user@example.com')
128+
expect(result).to eq(account_with_email)
129+
end
130+
131+
it 'finds account by email regardless of case' do
132+
result = described_class.find_for_login('USER@EXAMPLE.COM')
133+
expect(result).to eq(account_with_email)
134+
end
135+
136+
it 'finds account by email with whitespace trimmed' do
137+
result = described_class.find_for_login(' user@example.com ')
138+
expect(result).to eq(account_with_email)
139+
end
140+
141+
it 'prefers username match over email match' do
142+
account_with_email.update!(username: 'user@example.com')
143+
result = described_class.find_for_login('user@example.com')
144+
expect(result).to eq(account_with_email)
145+
end
146+
end
147+
148+
context 'when handling nil or blank input' do
149+
it 'returns nil for nil or blank identifier' do
150+
expect(described_class.find_for_login(nil)).to be_nil
151+
expect(described_class.find_for_login('')).to be_nil
152+
expect(described_class.find_for_login(' ')).to be_nil
153+
end
154+
end
155+
156+
context 'when no account matches' do
157+
it 'returns nil for non-existent username' do
158+
result = described_class.find_for_login('nonexistent')
159+
expect(result).to be_nil
160+
end
161+
162+
it 'returns nil for non-existent email' do
163+
result = described_class.find_for_login('nonexistent@example.com')
164+
expect(result).to be_nil
165+
end
166+
end
167+
end
168+
169+
describe '.resolve_login_identifier' do
170+
context 'when identifier resolves to an account' do
171+
let(:email_resolve_account) { create(:sofia_account, username: 'emailuser', password: 'password1234') }
172+
173+
before do
174+
create(:sofia_account, username: 'resolveuser', password: 'password1234')
175+
email_resolve_account.user.update!(email: 'resolve@example.com')
176+
end
177+
178+
it 'returns username when searched by username' do
179+
result = described_class.resolve_login_identifier('resolveuser')
180+
expect(result).to eq('resolveuser')
181+
end
182+
183+
it 'returns username when searched by email' do
184+
result = described_class.resolve_login_identifier('resolve@example.com')
185+
expect(result).to eq(email_resolve_account.username)
186+
end
187+
188+
it 'normalizes case for username lookup' do
189+
result = described_class.resolve_login_identifier('RESOLVEUSER')
190+
expect(result).to eq('resolveuser')
191+
end
192+
193+
it 'normalizes case for email lookup' do
194+
result = described_class.resolve_login_identifier('RESOLVE@EXAMPLE.COM')
195+
expect(result).to eq(email_resolve_account.username)
196+
end
197+
198+
it 'trims whitespace for username lookup' do
199+
result = described_class.resolve_login_identifier(' resolveuser ')
200+
expect(result).to eq('resolveuser')
201+
end
202+
203+
it 'trims whitespace for email lookup' do
204+
result = described_class.resolve_login_identifier(' resolve@example.com ')
205+
expect(result).to eq(email_resolve_account.username)
206+
end
207+
end
208+
209+
context 'when identifier does not resolve to an account' do
210+
it 'returns nil for nil, empty or non-existent identifiers' do
211+
expect(described_class.resolve_login_identifier(nil)).to be_nil
212+
expect(described_class.resolve_login_identifier('')).to be_nil
213+
expect(described_class.resolve_login_identifier(' ')).to be_nil
214+
expect(described_class.resolve_login_identifier('nonexistentuser')).to be_nil
215+
expect(described_class.resolve_login_identifier('nonexistent@example.com')).to be_nil
216+
end
217+
end
218+
end
99219
end

0 commit comments

Comments
 (0)