From 32b681de9f0a74f76f0813423110b22eb278be0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Sun, 30 Apr 2023 17:17:50 +0200 Subject: [PATCH] feat: restrict aliases to events, disable login with alias (#80) * fix: use right key for participants * feat: restrict aliases to events, disable login with alias --- app/interactors/authenticate_credentials.rb | 4 +++- app/interactors/create_token.rb | 5 ++++- app/models/token.rb | 12 +++--------- app/views/questions/ballot/_secret.html.erb | 4 ++-- test/controllers/sessions_controller_test.rb | 12 ------------ test/interactors/create_token_test.rb | 1 - 6 files changed, 12 insertions(+), 26 deletions(-) diff --git a/app/interactors/authenticate_credentials.rb b/app/interactors/authenticate_credentials.rb index 5839339..7098f53 100644 --- a/app/interactors/authenticate_credentials.rb +++ b/app/interactors/authenticate_credentials.rb @@ -23,8 +23,10 @@ def authenticate_user fail!(error: Errors::AccessDenied.new(identifier_type:)) if identity.disabled? end + # TODO: fix aliased tokens, because they cause a security vulnerability. def authenticate_token - self.identity = Token.from_value(identifier) + self.identity = Token.from_hash(identifier) + fail!(error: Errors::AccessDenied.new(identifier_type:)) if identity.nil? fail!(error: Errors::AccessDenied.new(identifier_type:)) if identity.disabled? end diff --git a/app/interactors/create_token.rb b/app/interactors/create_token.rb index 269b6dd..917f848 100644 --- a/app/interactors/create_token.rb +++ b/app/interactors/create_token.rb @@ -56,7 +56,10 @@ def find_or_initialize_token end def find_token - Token.from_value(cleaned_identifier) if identifier.present? + token = Token.from_hash(cleaned_identifier) if identifier.present? + return token if token.present? + + Token.from_alias(cleaned_identifier, event:) if identifier.present? && aliased rescue ActiveRecord::RecordNotFound nil end diff --git a/app/models/token.rb b/app/models/token.rb index eac1b98..8d7f508 100644 --- a/app/models/token.rb +++ b/app/models/token.rb @@ -21,13 +21,6 @@ class Token < ApplicationRecord after_initialize :init - def self.from_value(value) - token = Token.from_hash(value) - token = Token.from_alias(value) if token.nil? - - token - end - def self.from_hash(hash) hash = sanitize(hash) @@ -35,10 +28,11 @@ def self.from_hash(hash) Token.find(ids.first) unless ids.empty? end - def self.from_alias(value) + def self.from_alias(value, event: nil) value = sanitize(value) + return Token.find_by!(alias: value) if event.blank? - Token.find_by!(alias: value) + Token.find_by!(alias: value, event: event) end def self.sanitize(value) diff --git a/app/views/questions/ballot/_secret.html.erb b/app/views/questions/ballot/_secret.html.erb index ec96ab2..874fc19 100644 --- a/app/views/questions/ballot/_secret.html.erb +++ b/app/views/questions/ballot/_secret.html.erb @@ -27,7 +27,7 @@ <%= t('stats.census') %> - <%= t('stats.casted_votes') %> + <%= t('stats.participants') %> @@ -37,7 +37,7 @@ <% if Rails.configuration.x.asembleo.open_registration %> <%= User.voter.enabled.count %> <% else %> - <%= @consultation.tokens.voter.count %> + <%= @consultation.tokens.enabled.voter.count %> <% end %> diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index f17456e..e94ac2c 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -19,18 +19,6 @@ class SessionsControllerTest < ActionDispatch::IntegrationTest assert_equal principal.id, session[:identity_id] end - test 'should create session with aliased token' do - aliased_token = Faker::PhoneNumber.cell_phone - @params = { session: { identifier: aliased_token } } - - principal.update!(alias: Token.sanitize(aliased_token)) - - subject - - assert_response :redirect - assert_equal principal.id, session[:identity_id] - end - test 'should fail on deleted token' do principal.destroy! subject diff --git a/test/interactors/create_token_test.rb b/test/interactors/create_token_test.rb index 5d5e1ed..67dd724 100644 --- a/test/interactors/create_token_test.rb +++ b/test/interactors/create_token_test.rb @@ -21,7 +21,6 @@ class CreateTokenTest < ActionDispatch::IntegrationTest test 'should deliver magic link' do @params[:identifier] = 'voter@example.com' - @params[:aliased] = true @params[:send_magic_link] = true subject