From 1b3af950d79c067c4abe138df2da38f08cef466a Mon Sep 17 00:00:00 2001 From: LorenzoBellomo Date: Thu, 24 Sep 2026 15:46:23 -0300 Subject: [PATCH 01/13] fix: [TESIS-82] end the open sessions of a deactivated company The JWT of a company that was deactivated stayed valid until it expired, so its users kept operating for up to 24 hours after the company was turned off. is_active was only checked on login, registration and tenant-config. authenticate_user! now refuses a session whose company is not active. The check sits there and not in Devise's active_for_authentication? on purpose: that hook answers 401 to any request carrying the token, including the ones that do not require a session (login, registration, tenant-config). Co-Authored-By: Claude Opus 5.5 --- app/controllers/application_controller.rb | 20 ++++++++++++++++++++ spec/requests/api/v1/auth_spec.rb | 21 +++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index eeb59afc..f5c821d5 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -42,6 +42,26 @@ class MalformedParameterError < StandardError; end def index_action? = action_name == 'index' + # Devise valida el token (firma, vencimiento, denylist); acá se suma lo que el + # token no puede saber: si la empresa de la cuenta sigue activa. El JWT de una + # empresa dada de baja sigue siendo válido hasta vencer, y sin este chequeo + # seguía operando hasta 24 h después de la baja. + # + # Va sobre authenticate_user! y no en active_for_authentication? de Devise a + # propósito: ese hook corta con 401 cualquier request que traiga el token, + # también los que no exigen sesión (login, registro, tenant-config). Acá sólo + # corre donde se pide sesión. + def authenticate_user!(*) + super + return if current_company.is_active? + + render_session_refused('The company of this account is not active') + end + + def render_session_refused(message) + render json: { error: message }, status: :unauthorized + end + def set_current_tenant Current.company_id = current_user&.company_id Current.user = current_user diff --git a/spec/requests/api/v1/auth_spec.rb b/spec/requests/api/v1/auth_spec.rb index 34e27362..602e66da 100644 --- a/spec/requests/api/v1/auth_spec.rb +++ b/spec/requests/api/v1/auth_spec.rb @@ -170,6 +170,27 @@ def cross_tenant = login(slug: sur.slug) end end + # El JWT sigue siendo válido hasta vencer aunque la empresa se dé de baja. La + # baja tiene que cortar también las sesiones que ya estaban abiertas, no sólo + # los logins nuevos. + describe 'a session whose company is deactivated' do + let(:user) { User.create!(email: 'baja@test.com', password: 'password123', company: company) } + let(:auth) do + post '/api/v1/auth/login', params: { email: user.email, password: 'password123' }, headers: tenant_header + { 'Authorization' => "Bearer #{response.parsed_body['token']}" } + end + + it 'stops authenticating the token it already had', :aggregate_failures do + headers = auth + company.update!(is_active: false) + + get '/api/v1/warehouses', headers: headers + + expect(response).to have_http_status(:unauthorized) + expect(response.parsed_body['error']).to eq('The company of this account is not active') + end + end + describe 'DELETE /api/v1/auth/logout' do let(:user) { User.create!(email: 'out@test.com', password: 'password123', company: company) } let(:token) do From 7b116b2a01510d1e130474bce8d0bbdd0031a3e9 Mon Sep 17 00:00:00 2001 From: LorenzoBellomo Date: Thu, 24 Sep 2026 15:52:24 -0300 Subject: [PATCH 02/13] fix: [TESIS-82] turn self-registration into an access request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit POST /auth/register created an account that could log in right away, inside whatever company the X-Tenant-Slug header named. The slug is public (it is the subdomain), so anyone could join any company and read all of its data. Registering is now asking for access, which is what the S02 screen says ("Solicitá acceso al espacio de operación de tu organización"). The account is created with approved = false and cannot log in until someone approves it from the backoffice, where User gains the approved field. The column defaults to true, so existing accounts and the ones created by the backoffice, the seeds or the console keep working. The endpoint also stops telling which emails exist. The email is unique across the whole database, so "Email has already been taken" revealed accounts of any company. It now answers 202 with the same body whether the email was new or not, and it reports invalid input before looking at the email, so a short password cannot be used to tell the two cases apart either. A session whose account loses its approval ends on the next request, like the one of a deactivated company. Co-Authored-By: Claude Opus 5.5 --- app/avo/resources/user.rb | 3 + .../api/v1/auth/registrations_controller.rb | 12 +- app/controllers/application_controller.rb | 18 +-- app/models/user.rb | 4 + app/poros/auth/authenticate_user.rb | 8 +- app/poros/auth/register_user.rb | 48 ++++++-- .../20260924120000_add_approved_to_users.rb | 14 +++ db/schema.rb | 3 +- spec/models/user_spec.rb | 6 + spec/poros/auth/authenticate_user_spec.rb | 9 ++ spec/poros/auth/register_user_spec.rb | 36 +++++- spec/requests/api/v1/auth_spec.rb | 110 ++++++++++++++---- 12 files changed, 227 insertions(+), 44 deletions(-) create mode 100644 db/migrate/20260924120000_add_approved_to_users.rb diff --git a/app/avo/resources/user.rb b/app/avo/resources/user.rb index 1b31c19f..566cdf74 100644 --- a/app/avo/resources/user.rb +++ b/app/avo/resources/user.rb @@ -13,6 +13,9 @@ def fields field :id, as: :id field :email, as: :text, required: true field :company, as: :belongs_to + # El registro público crea la cuenta sin aprobar (Auth::RegisterUser): + # hasta que se tilde acá, no puede loguearse. + field :approved, as: :boolean field :created_at, as: :date_time, only_on: :index end diff --git a/app/controllers/api/v1/auth/registrations_controller.rb b/app/controllers/api/v1/auth/registrations_controller.rb index f28c290e..bc94663b 100644 --- a/app/controllers/api/v1/auth/registrations_controller.rb +++ b/app/controllers/api/v1/auth/registrations_controller.rb @@ -13,14 +13,22 @@ def create company = tenant_company return render_unknown_tenant if company.nil? - user = ::Auth::RegisterUser.new(params: user_params, company: company).call - render json: UserSerializer.render(user), status: :created + ::Auth::RegisterUser.new(params: user_params, company: company).call + render_request_received rescue ActiveRecord::RecordInvalid => e render json: { errors: e.record.errors.full_messages }, status: :unprocessable_content end private + # 202 y el mismo cuerpo, se haya creado la solicitud o no porque el email + # ya tenía cuenta: ver Auth::RegisterUser. Tampoco devuelve la cuenta + # (antes devolvía su id y su company_id): una solicitud pendiente no es + # todavía nada que el que llama pueda usar. + def render_request_received + render json: { status: 'pending_approval' }, status: :accepted + end + # `company_id` ya no se permitea: el tenant sale del slug del request. # Mandarlo en el body no hace nada — no es un error, simplemente se # ignora, como cualquier atributo desconocido. diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index f5c821d5..be176281 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -43,9 +43,10 @@ class MalformedParameterError < StandardError; end def index_action? = action_name == 'index' # Devise valida el token (firma, vencimiento, denylist); acá se suma lo que el - # token no puede saber: si la empresa de la cuenta sigue activa. El JWT de una - # empresa dada de baja sigue siendo válido hasta vencer, y sin este chequeo - # seguía operando hasta 24 h después de la baja. + # token no puede saber porque pudo cambiar después de emitido: si la empresa + # sigue activa y si la cuenta sigue aprobada. El JWT sigue siendo válido hasta + # vencer, y sin este chequeo la sesión seguía operando hasta 24 h después de + # la baja. # # Va sobre authenticate_user! y no en active_for_authentication? de Devise a # propósito: ese hook corta con 401 cualquier request que traiga el token, @@ -53,13 +54,14 @@ def index_action? = action_name == 'index' # corre donde se pide sesión. def authenticate_user!(*) super - return if current_company.is_active? - - render_session_refused('The company of this account is not active') + refusal = session_refusal + render json: { error: refusal }, status: :unauthorized if refusal end - def render_session_refused(message) - render json: { error: message }, status: :unauthorized + def session_refusal + return 'The company of this account is not active' unless current_company.is_active? + + 'This account is pending approval' unless current_user.approved? end def set_current_tenant diff --git a/app/models/user.rb b/app/models/user.rb index 7ebff7e4..69c21a47 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -3,6 +3,10 @@ class User < ApplicationRecord belongs_to :company + # `approved` tiene default true: sólo el registro público crea cuentas sin + # aprobar (Auth::RegisterUser), que no pueden loguearse hasta que alguien las + # apruebe desde el backoffice. + # Include default devise modules. Others available are: # :confirmable, :lockable, :timeoutable, :trackable and :omniauthable devise :database_authenticatable, :registerable, diff --git a/app/poros/auth/authenticate_user.rb b/app/poros/auth/authenticate_user.rb index 32661c83..2f5cf95d 100644 --- a/app/poros/auth/authenticate_user.rb +++ b/app/poros/auth/authenticate_user.rb @@ -10,14 +10,14 @@ def initialize(email:, password:, company:) end # Devuelve nil ante cualquier fallo — tenant no resuelto, email inexistente - # en ese tenant, usuario de otro tenant o password incorrecta. El caller no - # puede distinguir los casos, que es justamente el punto: el 401 tiene que - # ser idéntico para todos. + # en ese tenant, usuario de otro tenant, password incorrecta o cuenta que + # todavía no aprobaron. El caller no puede distinguir los casos, que es + # justamente el punto: el 401 tiene que ser idéntico para todos. def call return nil if @company.nil? user = find_user - return nil unless user&.valid_password?(@password) + return nil unless user&.valid_password?(@password) && user.approved? Warden::JWTAuth::UserEncoder.new.call(user, :user, nil).first end diff --git a/app/poros/auth/register_user.rb b/app/poros/auth/register_user.rb index 48eb4416..fb98d57e 100644 --- a/app/poros/auth/register_user.rb +++ b/app/poros/auth/register_user.rb @@ -8,18 +8,50 @@ def initialize(params:, company:) @company = company end - # La company es la que resolvió el slug del request, nunca la que venga en - # el body: hasta TESIS-120 `company_id` era un parámetro permitido y con eso - # cualquiera podía darse de alta dentro de cualquier tenant. + # Registrarse es pedir acceso (S02: «Solicitá acceso al espacio de operación + # de tu organización»): la cuenta nace sin aprobar y no puede loguearse + # hasta que la aprueben desde el backoffice. Antes el alta daba acceso + # inmediato, y como el slug es público (es el subdominio), cualquiera podía + # entrar a leer los datos de cualquier empresa. # - # El `Current.set` no es decorativo: User incluye CompanyScoped, que en el - # create pisa `company_id` con `Current.company_id` si está seteado. Sin - # esto, un register con un JWT de otro tenant en el header crearía el - # usuario en ese otro tenant en vez de en el del slug. + # Devuelve la cuenta creada, o nil si el email ya tenía una. El controller + # responde lo mismo en los dos casos: el email es único en toda la base, y + # si la respuesta cambiara, el registro diría qué emails existen en esta + # empresa o en cualquier otra. + # + # La company es la que resolvió el slug, nunca la del body (TESIS-120). El + # `Current.set` no es decorativo: CompanyScoped pisa `company_id` con + # `Current.company_id` en el create, y un register con un JWT de otro tenant + # en el header crearía la cuenta en ese otro tenant. def call Current.set(company_id: @company.id) do - User.create!(@params.merge(company: @company)) + user = User.new(@params.merge(company: @company, approved: false)) + next if email_taken?(user) + + user.save! + user end + rescue ActiveRecord::RecordNotUnique + # Dos registros simultáneos del mismo email nuevo pasan los dos la + # validación y el segundo choca contra el índice único. Para el que llama + # es lo mismo que un email que ya tenía cuenta. + nil + end + + private + + # Valida lo que escribió el que llama y dice si el email ya tenía cuenta. + # Los errores de formato (email mal escrito, password corta) se levantan + # siempre, y ANTES de mirar si el email existe: al revés, una password + # inválida respondería distinto según el email estuviera tomado o no, y la + # diferencia volvería a delatarlo. + def email_taken?(user) + user.validate + taken = user.errors.of_kind?(:email, :taken) + user.errors.delete(:email, :taken) + raise ActiveRecord::RecordInvalid, user if user.errors.any? + + taken end end end diff --git a/db/migrate/20260924120000_add_approved_to_users.rb b/db/migrate/20260924120000_add_approved_to_users.rb new file mode 100644 index 00000000..8bdae7d0 --- /dev/null +++ b/db/migrate/20260924120000_add_approved_to_users.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class AddApprovedToUsers < ActiveRecord::Migration[8.1] + # Registrarse pasa a ser pedir acceso (S02: «Solicitá acceso al espacio de + # operación de tu organización»). La cuenta que crea el registro público nace + # sin aprobar y no puede loguearse hasta que la aprueben. + # + # Default true a propósito: las cuentas que ya existen, y las que crean el + # backoffice, los seeds o la consola, quedan habilitadas como hasta ahora. + # Sólo Auth::RegisterUser crea cuentas con false. + def change + add_column :users, :approved, :boolean, default: true, null: false + end +end diff --git a/db/schema.rb b/db/schema.rb index 338cf76c..74dd0156 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_09_21_130000) do +ActiveRecord::Schema[8.1].define(version: 2026_09_24_120000) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" @@ -224,6 +224,7 @@ end create_table "users", force: :cascade do |t| + t.boolean "approved", default: true, null: false t.bigint "company_id", null: false t.datetime "created_at", null: false t.string "email", default: "", null: false diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index b2fe7017..da96f3ce 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -14,6 +14,12 @@ expect(user).not_to be_valid end + # Las cuentas que crean el backoffice, los seeds o la consola quedan + # habilitadas. Sólo el registro público las crea sin aprobar. + it 'is approved unless it is created as an access request' do + expect(user.approved).to be(true) + end + it 'belongs to a company' do expect(described_class.reflect_on_association(:company).macro).to eq(:belongs_to) end diff --git a/spec/poros/auth/authenticate_user_spec.rb b/spec/poros/auth/authenticate_user_spec.rb index 3e359f8f..3f776481 100644 --- a/spec/poros/auth/authenticate_user_spec.rb +++ b/spec/poros/auth/authenticate_user_spec.rb @@ -31,6 +31,15 @@ expect(token).to be_nil end + # Una cuenta del registro público que todavía no aprobaron: credenciales + # correctas, pero no hay token hasta que la aprueben. + it 'returns nil for an account pending approval' do + User.create!(email: 'pend@test.com', password: 'password123', company: company, approved: false) + + token = described_class.new(email: 'pend@test.com', password: 'password123', company: company).call + expect(token).to be_nil + end + # El controller pasa el resultado de resolver el slug, que es nil cuando el # tenant no existe o está inactivo. El PORO no puede confundir eso con un # login global. diff --git a/spec/poros/auth/register_user_spec.rb b/spec/poros/auth/register_user_spec.rb index 76505266..1affb260 100644 --- a/spec/poros/auth/register_user_spec.rb +++ b/spec/poros/auth/register_user_spec.rb @@ -9,13 +9,47 @@ def register(extra_params = {}, into: company) described_class.new(params: params, company: into).call end - it 'creates a user under the given company', :aggregate_failures do + it 'creates the account under the given company', :aggregate_failures do user = register expect(user).to be_persisted expect(user.company).to eq(company) end + # Registrarse es pedir acceso: la cuenta no entra hasta que la aprueben. + it 'creates the account pending approval' do + expect(register.approved).to be(false) + end + + context 'when the email already has an account' do + before { User.create!(email: 'a@test.com', password: 'password123', company: otra) } + + it 'creates nothing and returns nil', :aggregate_failures do + result = nil + + expect { result = register }.not_to change(User, :count) + expect(result).to be_nil + end + + # El error de formato se informa igual que con un email libre, y sin + # mencionar que el email ya existe. + it 'still raises on invalid params, without saying the email is taken' do + expect { register({ password: '123' }) } + .to raise_error(ActiveRecord::RecordInvalid, + 'Validation failed: Password is too short (minimum is 6 characters)') + end + end + + # Dos registros simultáneos del mismo email pasan los dos la validación y el + # segundo choca contra el índice único: es un email que ya tenía cuenta. + it 'treats a duplicate caught by the unique index as an existing email' do + allow(User).to receive(:new).and_wrap_original do |original, *args| + original.call(*args).tap { |user| allow(user).to receive(:save!).and_raise(ActiveRecord::RecordNotUnique) } + end + + expect(register).to be_nil + end + # El company_id del body ya no llega hasta acá (el controller no lo permitea), # pero si llegara tampoco debe ganarle a la company resuelta por slug. it 'ignores a company_id passed in the params', :aggregate_failures do diff --git a/spec/requests/api/v1/auth_spec.rb b/spec/requests/api/v1/auth_spec.rb index 602e66da..811c2a12 100644 --- a/spec/requests/api/v1/auth_spec.rb +++ b/spec/requests/api/v1/auth_spec.rb @@ -21,23 +21,24 @@ def register(params = valid_params, slug: company.slug) [response.status, response.body] end - it 'creates a user', :aggregate_failures do + # Registrarse es pedir acceso (S02): la cuenta queda pendiente y la respuesta + # no devuelve nada que el que llama pueda usar. + it 'creates the account as a request pending approval', :aggregate_failures do expect { register }.to change(User, :count).by(1) - expect(response).to have_http_status(:created) + expect(response).to have_http_status(:accepted) + expect(User.last.approved).to be(false) end - it 'creates the user inside the tenant of the slug' do + it 'answers only that the request is pending' do register - expect(User.last.company).to eq(company) + expect(response.parsed_body).to eq('status' => 'pending_approval') end - it 'returns the user without exposing the password', :aggregate_failures do + it 'creates the account inside the tenant of the slug' do register - body = response.parsed_body - expect(body['email']).to eq('new@test.com') - expect(body.keys).not_to include('password', 'encrypted_password') + expect(User.last.company).to eq(company) end it 'stores the password hashed, never in plain text', :aggregate_failures do @@ -47,11 +48,68 @@ def register(params = valid_params, slug: company.slug) expect(User.last.encrypted_password).not_to eq('password123') end - it 'rejects a duplicate email' do - User.create!(email: 'dup@test.com', password: 'password123', company: company) - register(valid_params.merge(email: 'dup@test.com')) + it 'returns 422 for invalid input' do + expect(register(valid_params.merge(password: '123')).first).to eq(422) + end + + # El agujero que encontró la QA de TESIS-82: el registro daba acceso + # inmediato, y el slug es público (es el subdominio). Cualquiera se daba de + # alta en cualquier empresa y leía sus datos. + context 'when the account was just requested' do + def login_attempt(password: valid_params[:password]) + post '/api/v1/auth/login', params: valid_params.merge(password: password), headers: tenant_header + [response.status, response.body] + end + + before { register } + + it 'cannot log in, and the answer is the one of a wrong password', :aggregate_failures do + pending_login = login_attempt + + expect(pending_login.first).to eq(401) + expect(pending_login).to eq(login_attempt(password: 'wrong')) + end + + it 'logs in once it is approved' do + User.last.update!(approved: true) - expect(response).to have_http_status(:unprocessable_content) + expect(login_attempt.first).to eq(200) + end + end + + # El email es único en toda la base. Si la respuesta cambiara cuando ya tiene + # cuenta, el registro diría qué emails existen, en esta empresa o en otra. + context 'when the email already has an account' do + let(:otra) { Company.create!(name: 'Otra', tax_id: '20-22222222-2', slug: 'otra') } + + before do + User.create!(email: 'ajeno@test.com', password: 'password123', company: otra) + User.create!(email: 'propio@test.com', password: 'password123', company: company) + end + + it 'does not create a second account' do + expect { register(valid_params.merge(email: 'ajeno@test.com')) }.not_to change(User, :count) + end + + it 'answers exactly like for a new email when the account is in another company' do + taken = register(valid_params.merge(email: 'ajeno@test.com')) + + expect(taken).to eq(register(valid_params.merge(email: 'nuevo@test.com'))) + end + + it 'answers exactly like for a new email when the account is in the same company' do + taken = register(valid_params.merge(email: 'propio@test.com')) + + expect(taken).to eq(register(valid_params.merge(email: 'nuevo@test.com'))) + end + + # Un error de formato se informa siempre. Si sólo se informara cuando el + # email está libre, la diferencia lo delataría igual. + it 'reports invalid input without telling whether the email exists' do + taken = register(valid_params.merge(email: 'ajeno@test.com', password: '123')) + + expect(taken).to eq(register(valid_params.merge(email: 'nuevo@test.com', password: '123'))) + end end # El agujero que cierra TESIS-120: hasta acá `company_id` era un parámetro @@ -63,7 +121,7 @@ def register(params = valid_params, slug: company.slug) it 'ignores it and uses the tenant of the slug', :aggregate_failures do register(valid_params.merge(company_id: otra.id)) - expect(response).to have_http_status(:created) + expect(response).to have_http_status(:accepted) expect(User.last.company).to eq(company) expect(otra.users).to be_empty end @@ -170,25 +228,37 @@ def cross_tenant = login(slug: sur.slug) end end - # El JWT sigue siendo válido hasta vencer aunque la empresa se dé de baja. La - # baja tiene que cortar también las sesiones que ya estaban abiertas, no sólo - # los logins nuevos. - describe 'a session whose company is deactivated' do + # El JWT sigue siendo válido hasta vencer aunque la cuenta deje de estar + # habilitada. Dar de baja la empresa, o quitarle la aprobación a la cuenta, + # tiene que cortar también las sesiones que ya estaban abiertas, no sólo los + # logins nuevos. + describe 'a session that stops being allowed' do let(:user) { User.create!(email: 'baja@test.com', password: 'password123', company: company) } let(:auth) do post '/api/v1/auth/login', params: { email: user.email, password: 'password123' }, headers: tenant_header { 'Authorization' => "Bearer #{response.parsed_body['token']}" } end - it 'stops authenticating the token it already had', :aggregate_failures do + # Se abre la sesión, pasa el cambio y recién ahí se usa el token. + def request_after_change headers = auth - company.update!(is_active: false) - + yield get '/api/v1/warehouses', headers: headers + end + + it 'ends when the company is deactivated', :aggregate_failures do + request_after_change { company.update!(is_active: false) } expect(response).to have_http_status(:unauthorized) expect(response.parsed_body['error']).to eq('The company of this account is not active') end + + it 'ends when the account loses its approval', :aggregate_failures do + request_after_change { user.update!(approved: false) } + + expect(response).to have_http_status(:unauthorized) + expect(response.parsed_body['error']).to eq('This account is pending approval') + end end describe 'DELETE /api/v1/auth/logout' do From d2372404c0e8d55611032e33551c0f4ac65f779e Mon Sep 17 00:00:00 2001 From: LorenzoBellomo Date: Thu, 24 Sep 2026 15:53:57 -0300 Subject: [PATCH 03/13] fix: [TESIS-82] find the account of a login whatever the case of the email Devise stores the email downcased and stripped (case_insensitive_keys and strip_whitespace_keys), but it only normalizes on save and inside its own finders. AuthenticateUser looks the account up with its own find_by and passed the email as typed, so someone who registered as Ana@Norte.com got a 401 with the right password when typing it the same way. The frontend does not normalize it either. The email is now normalized the same way before the lookup. Co-Authored-By: Claude Opus 5.5 --- app/poros/auth/authenticate_user.rb | 11 ++++++++++- spec/poros/auth/authenticate_user_spec.rb | 8 ++++++++ spec/requests/api/v1/auth_spec.rb | 6 ++++++ 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/app/poros/auth/authenticate_user.rb b/app/poros/auth/authenticate_user.rb index 2f5cf95d..6c37ac4c 100644 --- a/app/poros/auth/authenticate_user.rb +++ b/app/poros/auth/authenticate_user.rb @@ -29,7 +29,16 @@ def call # tenant de ese token y no por el que se está intentando. El scope acá es el # de la company resuelta por slug, y sólo ese. def find_user - User.unscoped.find_by(company_id: @company.id, email: @email) + User.unscoped.find_by(company_id: @company.id, email: normalized_email) + end + + # Devise guarda el email en minúsculas y sin espacios alrededor + # (case_insensitive_keys y strip_whitespace_keys), pero esa normalización + # sólo corre al guardar y en sus propios finders, y este find_by es nuestro. + # Sin esto, quien se registró como «Ana@Norte.com» no podía entrar tipeando + # el email igual que al registrarse. + def normalized_email + @email.to_s.strip.downcase end end end diff --git a/spec/poros/auth/authenticate_user_spec.rb b/spec/poros/auth/authenticate_user_spec.rb index 3f776481..8cf5188d 100644 --- a/spec/poros/auth/authenticate_user_spec.rb +++ b/spec/poros/auth/authenticate_user_spec.rb @@ -12,6 +12,14 @@ expect(token).to be_present end + # Devise guarda el email normalizado; el que se tipea en el login puede venir + # con otras mayúsculas o con espacios alrededor. + it 'finds the account whatever the case and the surrounding spaces of the email' do + token = described_class.new(email: ' Log@TEST.com ', password: 'password123', + company: company).call + expect(token).to be_present + end + it 'returns nil for a wrong password' do token = described_class.new(email: 'log@test.com', password: 'wrong', company: company).call expect(token).to be_nil diff --git a/spec/requests/api/v1/auth_spec.rb b/spec/requests/api/v1/auth_spec.rb index 811c2a12..2b0c63ef 100644 --- a/spec/requests/api/v1/auth_spec.rb +++ b/spec/requests/api/v1/auth_spec.rb @@ -171,6 +171,12 @@ def login(email: 'log@test.com', password: 'password123', slug: company.slug) expect(payload['company_id']).to eq(company.id) end + # Hallazgo de la QA de TESIS-82: Devise guarda el email en minúsculas, y + # quien lo tipeaba con alguna mayúscula recibía 401 con la password correcta. + it 'accepts the email in another case and with surrounding spaces' do + expect(login(email: ' Log@TEST.com ').first).to eq(200) + end + it 'returns 401 on wrong password' do expect(login(password: 'wrong').first).to eq(401) end From 0353ffa3081d0038fe02a274276b600dc259664e Mon Sep 17 00:00:00 2001 From: LorenzoBellomo Date: Thu, 24 Sep 2026 15:57:35 -0300 Subject: [PATCH 04/13] fix: [TESIS-82] rate limit the login and the registration by IP Passwords could be guessed without limit: after 30 wrong ones in a row, the right one still logged in. Login and registration now allow 10 attempts every 3 minutes per IP and then answer 429 with a Retry-After header. The limit is Rails' rate_limit and not Devise's :lockable on purpose: locking the account would let anyone lock someone else out by mistyping their password. The counter lives in the app cache, Solid Cache in production. The test cache is :null_store, so the rest of the suite never reaches the limit; the new spec gives the counter a real store for each example. Co-Authored-By: Claude Opus 5.5 --- .../api/v1/auth/registrations_controller.rb | 1 + .../api/v1/auth/sessions_controller.rb | 1 + .../concerns/api/v1/auth/attempt_limit.rb | 38 ++++++++ .../api/v1/auth_attempt_limit_spec.rb | 87 +++++++++++++++++++ 4 files changed, 127 insertions(+) create mode 100644 app/controllers/concerns/api/v1/auth/attempt_limit.rb create mode 100644 spec/requests/api/v1/auth_attempt_limit_spec.rb diff --git a/app/controllers/api/v1/auth/registrations_controller.rb b/app/controllers/api/v1/auth/registrations_controller.rb index bc94663b..b7135580 100644 --- a/app/controllers/api/v1/auth/registrations_controller.rb +++ b/app/controllers/api/v1/auth/registrations_controller.rb @@ -5,6 +5,7 @@ module V1 module Auth class RegistrationsController < ApplicationController include TenantFromSlug + include AttemptLimit skip_before_action :authenticate_user! skip_after_action :verify_authorized, :verify_policy_scoped diff --git a/app/controllers/api/v1/auth/sessions_controller.rb b/app/controllers/api/v1/auth/sessions_controller.rb index 60e19a52..67b8c673 100644 --- a/app/controllers/api/v1/auth/sessions_controller.rb +++ b/app/controllers/api/v1/auth/sessions_controller.rb @@ -5,6 +5,7 @@ module V1 module Auth class SessionsController < ApplicationController include TenantFromSlug + include AttemptLimit skip_before_action :authenticate_user!, only: :create skip_after_action :verify_authorized, :verify_policy_scoped diff --git a/app/controllers/concerns/api/v1/auth/attempt_limit.rb b/app/controllers/concerns/api/v1/auth/attempt_limit.rb new file mode 100644 index 00000000..ee1824ff --- /dev/null +++ b/app/controllers/concerns/api/v1/auth/attempt_limit.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module Api + module V1 + module Auth + # Límite de intentos para los endpoints públicos de auth: login y registro. + # + # Sin esto se podían probar contraseñas sin freno: después de 30 incorrectas + # seguidas, la correcta entraba igual. Se limita por IP con el rate_limit de + # Rails y no con el :lockable de Devise, que bloquea la cuenta: con él, + # cualquiera podría dejar afuera a otro usuario tipeando mal su password a + # propósito. + # + # El contador vive en la cache de la app, que en producción es Solid Cache y + # la comparten todos los procesos. En test la cache es :null_store y el + # límite nunca se alcanza, así los specs que loguean muchas veces no chocan + # con él; el spec del límite le da una cache de verdad a propósito. + module AttemptLimit + extend ActiveSupport::Concern + + MAX_ATTEMPTS = 10 + WINDOW = 3.minutes + + included do + rate_limit to: MAX_ATTEMPTS, within: WINDOW, only: :create, + with: :render_too_many_attempts + end + + private + + def render_too_many_attempts + response.set_header('Retry-After', WINDOW.to_i.to_s) + render json: { error: 'Too many attempts, try again later' }, status: :too_many_requests + end + end + end + end +end diff --git a/spec/requests/api/v1/auth_attempt_limit_spec.rb b/spec/requests/api/v1/auth_attempt_limit_spec.rb new file mode 100644 index 00000000..bed49246 --- /dev/null +++ b/spec/requests/api/v1/auth_attempt_limit_spec.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +require 'rails_helper' + +# Límite de intentos de login y registro. Lo pidió la QA de TESIS-82: después +# de 30 passwords incorrectas seguidas, la correcta entraba igual. +# +# En test la cache es :null_store y el límite nunca se alcanza, así los specs +# que loguean muchas veces no chocan con él. Acá el contador usa una cache de +# verdad, sólo durante cada ejemplo. +RSpec.describe 'Auth attempt limit', type: :request do + include ActiveSupport::Testing::TimeHelpers + + let(:company) { Company.create!(name: 'Acme', tax_id: '20-11111111-1', slug: 'acme') } + let(:counter) { ActiveSupport::Cache::MemoryStore.new } + let(:max_attempts) { Api::V1::Auth::AttemptLimit::MAX_ATTEMPTS } + + before do + User.create!(email: 'log@test.com', password: 'password123', company: company) + allow(Api::V1::Auth::SessionsController.cache_store).to receive(:increment) do |*args, **options| + counter.increment(*args, **options) + end + end + + def login(password: 'wrong', ip: '203.0.113.7') + post '/api/v1/auth/login', params: { email: 'log@test.com', password: password }, + headers: { 'X-Tenant-Slug' => company.slug, 'REMOTE_ADDR' => ip } + response + end + + def use_up_attempts(ip: '203.0.113.7') + max_attempts.times { login(ip: ip) } + end + + it 'lets every attempt of the window through' do + (max_attempts - 1).times { login } + + expect(login(password: 'password123')).to have_http_status(:ok) + end + + it 'answers 429 once the attempts run out, even with the right password', :aggregate_failures do + use_up_attempts + login(password: 'password123') + + expect(response).to have_http_status(:too_many_requests) + expect(response.parsed_body['error']).to eq('Too many attempts, try again later') + end + + it 'tells the client when it can try again' do + use_up_attempts + + expect(login.headers['Retry-After']).to eq('180') + end + + it 'counts each IP on its own' do + use_up_attempts(ip: '203.0.113.7') + + expect(login(password: 'password123', ip: '198.51.100.9')).to have_http_status(:ok) + end + + it 'lets the IP in again once the window is over' do + use_up_attempts + + travel(Api::V1::Auth::AttemptLimit::WINDOW + 1.second) do + expect(login(password: 'password123')).to have_http_status(:ok) + end + end + + it 'limits the registration too' do + (max_attempts + 1).times do |n| + post '/api/v1/auth/register', params: { email: "nuevo#{n}@test.com", password: 'password123' }, + headers: { 'X-Tenant-Slug' => company.slug } + end + + expect(response).to have_http_status(:too_many_requests) + end + + # El logout no es un intento de adivinar nada: no cuenta ni se frena. + it 'does not limit the logout' do + token = login(password: 'password123').parsed_body['token'] + use_up_attempts + + delete '/api/v1/auth/logout', headers: { 'Authorization' => "Bearer #{token}", 'REMOTE_ADDR' => '203.0.113.7' } + + expect(response).to have_http_status(:no_content) + end +end From 4b6de72372446830d0c97e133a4de5f5a61ddf18 Mon Sep 17 00:00:00 2001 From: LorenzoBellomo Date: Thu, 24 Sep 2026 15:59:42 -0300 Subject: [PATCH 05/13] fix: [TESIS-82] refuse to configure integrations without the feature The integrations flag of a company was only enforced by the frontend, which hides the section. A company without the feature still configured integrations by calling PUT /integrations directly. The update now answers 403 when the flag is off. The listing stays open: the dashboard's integration nodes widget requests it for every company, and it only shows the global service templates with the company's own status. Co-Authored-By: Claude Opus 5.5 --- .../api/v1/integrations_controller.rb | 14 +++++++++++ app/models/company.rb | 7 ++++++ spec/models/company_spec.rb | 19 ++++++++++++++ spec/requests/api/v1/integrations_spec.rb | 25 ++++++++++++++++++- 4 files changed, 64 insertions(+), 1 deletion(-) diff --git a/app/controllers/api/v1/integrations_controller.rb b/app/controllers/api/v1/integrations_controller.rb index 8b84bd49..515c0569 100644 --- a/app/controllers/api/v1/integrations_controller.rb +++ b/app/controllers/api/v1/integrations_controller.rb @@ -5,6 +5,13 @@ module V1 class IntegrationsController < ApplicationController skip_after_action :verify_authorized, :verify_policy_scoped + # El flag `integrations` de la empresa no se miraba acá: el front oculta la + # sección, pero una empresa sin la feature la configuraba igual llamando al + # endpoint directo. Se corta el alta y la modificación. El listado queda + # abierto porque lo usa el widget de nodos del panel, y sólo muestra las + # plantillas globales con el estado de la propia empresa. + before_action :require_integrations_feature, only: :update + def index integrations = current_company.company_integrations.index_by(&:service_id) render json: IntegrationStatusSerializer.render( @@ -24,6 +31,13 @@ def update private + def require_integrations_feature + return if current_company.feature_enabled?(:integrations) + + render json: { error: 'The integrations feature is not enabled for this company' }, + status: :forbidden + end + def credentials_params raw = params.require(:credentials) unless raw.is_a?(ActionController::Parameters) diff --git a/app/models/company.rb b/app/models/company.rb index 66ca3ddb..e3272bf6 100644 --- a/app/models/company.rb +++ b/app/models/company.rb @@ -33,6 +33,13 @@ def self.find_active_by_slug(slug) active.find_by(slug: normalized) end + # Un flag ausente es un flag apagado, mismo criterio que el front + # (isFeatureEnabled): la empresa que no compró la feature no la tiene + # declarada, y el default nunca puede ser habilitarla. + def feature_enabled?(feature) + features[feature.to_s] == true + end + private def assign_default_slug diff --git a/spec/models/company_spec.rb b/spec/models/company_spec.rb index 1cf0200f..a6323cb2 100644 --- a/spec/models/company_spec.rb +++ b/spec/models/company_spec.rb @@ -41,6 +41,25 @@ expect(company.reload.branding['primary_color']).to eq('#2E7D32') end + + describe '#feature_enabled?' do + it 'is true for a feature turned on' do + company.features = { 'integrations' => true } + + expect(company.feature_enabled?(:integrations)).to be(true) + end + + it 'is false for a feature turned off' do + company.features = { 'integrations' => false } + + expect(company.feature_enabled?(:integrations)).to be(false) + end + + # La empresa que no compró la feature no la tiene declarada. + it 'is false for a feature that is not declared' do + expect(company.feature_enabled?(:integrations)).to be(false) + end + end end describe 'slug' do diff --git a/spec/requests/api/v1/integrations_spec.rb b/spec/requests/api/v1/integrations_spec.rb index 0bd1ae20..e5bb5cb3 100644 --- a/spec/requests/api/v1/integrations_spec.rb +++ b/spec/requests/api/v1/integrations_spec.rb @@ -3,7 +3,9 @@ require 'rails_helper' RSpec.describe 'Integrations API', type: :request do - let(:company) { Company.create!(name: 'Tenant A', tax_id: '30-11111111-1') } + let(:company) do + Company.create!(name: 'Tenant A', tax_id: '30-11111111-1', features: { 'integrations' => true }) + end let(:user) { User.create!(email: 'a@example.com', password: 'password123', company: company) } let(:headers) { auth_headers(user) } let!(:service) do @@ -103,6 +105,27 @@ end end + # La QA de TESIS-82 encontró que el flag sólo lo aplicaba el front: una + # empresa sin la feature configuraba integraciones llamando al endpoint. + context 'when the company does not have the integrations feature' do + before { company.update!(features: { 'integrations' => false }) } + + it 'refuses to configure the integration', :aggregate_failures do + expect do + put "/api/v1/integrations/#{service.id}", params: payload, headers: headers, as: :json + end.not_to change(CompanyIntegration, :count) + expect(response).to have_http_status(:forbidden) + end + + # El widget de nodos del panel lo pide igual, y sólo lista las plantillas + # globales con el estado de la propia empresa. + it 'still lists the services' do + get '/api/v1/integrations', headers: headers + + expect(response).to have_http_status(:ok) + end + end + context 'when another company already configured the same service' do let!(:other_integration) do CompanyIntegration.create!( From 86b5fa705362ab037fc4815f320bd3a3b97ed683 Mon Sep 17 00:00:00 2001 From: LorenzoBellomo Date: Thu, 24 Sep 2026 19:42:32 -0300 Subject: [PATCH 06/13] fix: [TESIS-82] refuse to delete a warehouse that stock transfers reference Deleting the destination of a transfer in transit answered 500. That warehouse has no stock of its own yet (the units are added when the transfer is received) and no order lines, so neither restrict_with_error let it through, and the delete reached the foreign keys of stock_transfers, which are restrict too: ActiveRecord::InvalidForeignKey. Warehouse now declares its outgoing and incoming transfers with restrict_with_error, so the delete answers 409 and the message says the transfers are what block it. Co-Authored-By: Claude Opus 5.5 --- .../api/v1/warehouses_controller.rb | 13 ++++-- app/models/warehouse.rb | 11 +++++ spec/requests/api/v1/warehouses_spec.rb | 42 +++++++++++++++++++ 3 files changed, 63 insertions(+), 3 deletions(-) diff --git a/app/controllers/api/v1/warehouses_controller.rb b/app/controllers/api/v1/warehouses_controller.rb index 0b482fd9..6cd84cf3 100644 --- a/app/controllers/api/v1/warehouses_controller.rb +++ b/app/controllers/api/v1/warehouses_controller.rb @@ -54,12 +54,19 @@ def warehouse_params params.require(:warehouse).permit(:name, :zip_code, :address) end - # Dos cosas bloquean el borrado y el mensaje tiene que decir cuál. Se + # Tres cosas bloquean el borrado y el mensaje tiene que decir cuál. Se # pregunta en el mismo orden en que el modelo las declara, que es el orden # en que `restrict_with_error` corta: si hay stock, el motivo es el stock. def render_conflict(_exception) - reason = @warehouse.stocks.exists? ? 'existing stock' : 'order lines taken from it' - render json: { error: "Cannot delete warehouse with #{reason}" }, status: :conflict + render json: { error: "Cannot delete warehouse with #{blocking_reason}" }, + status: :conflict + end + + def blocking_reason + return 'existing stock' if @warehouse.stocks.exists? + return 'order lines taken from it' if @warehouse.order_items.exists? + + 'stock transfers from or to it' end end end diff --git a/app/models/warehouse.rb b/app/models/warehouse.rb index 82b0bab8..fdd6eef5 100644 --- a/app/models/warehouse.rb +++ b/app/models/warehouse.rb @@ -12,6 +12,17 @@ class Warehouse < ApplicationRecord # devolución sin destino. La FK es restrict por lo mismo; esto la adelanta a # un 409 legible en vez de un InvalidForeignKey. has_many :order_items, dependent: :restrict_with_error + # Tampoco si una transferencia lo tiene como origen o como destino: la FK de + # stock_transfers también es restrict, y sin esto el borrado llegaba a la base + # y respondía 500. El caso real es el destino de una transferencia en + # tránsito: todavía no tiene stock propio, porque las unidades se le suman + # recién al recibirla. + has_many :outgoing_transfers, class_name: 'StockTransfer', inverse_of: :origin_warehouse, + foreign_key: :origin_warehouse_id, + dependent: :restrict_with_error + has_many :incoming_transfers, class_name: 'StockTransfer', inverse_of: :destination_warehouse, + foreign_key: :destination_warehouse_id, + dependent: :restrict_with_error validates :name, presence: true validates :zip_code, presence: true diff --git a/spec/requests/api/v1/warehouses_spec.rb b/spec/requests/api/v1/warehouses_spec.rb index 4944f967..bbbdbdd2 100644 --- a/spec/requests/api/v1/warehouses_spec.rb +++ b/spec/requests/api/v1/warehouses_spec.rb @@ -312,6 +312,38 @@ def stock_queries_while(&) .to eq('Cannot delete warehouse with order lines taken from it') end end + + # Hallazgo de la QA de TESIS-82: la FK de stock_transfers es restrict y el + # modelo no la adelantaba, así que este borrado respondía 500. El destino de + # una transferencia en tránsito no tiene stock propio todavía: las unidades + # se le suman recién al recibirla. + context 'when a stock transfer references it and it has no stock of its own' do + it 'returns 409 and keeps it as the destination of a transfer in transit', :aggregate_failures do + create_transfer_touching_warehouse(as: :destination) + + delete "/api/v1/warehouses/#{warehouse.id}", headers: headers + + expect(response).to have_http_status(:conflict) + expect(Warehouse.find_by(id: warehouse.id)).to be_present + end + + it 'returns 409 as the origin of a transfer too' do + create_transfer_touching_warehouse(as: :origin) + + delete "/api/v1/warehouses/#{warehouse.id}", headers: headers + + expect(response).to have_http_status(:conflict) + end + + it 'says the transfers are what block it' do + create_transfer_touching_warehouse(as: :destination) + + delete "/api/v1/warehouses/#{warehouse.id}", headers: headers + + expect(response.parsed_body['error']) + .to eq('Cannot delete warehouse with stock transfers from or to it') + end + end end def create_warehouse_with_stock @@ -325,4 +357,14 @@ def create_order_line_from_warehouse OrderItem.create!(order: order, product: product, warehouse: warehouse, quantity: 1, unit_price: 100) end + + # Una transferencia en tránsito entre `warehouse` y otro depósito, sin stock en + # `warehouse`. `as:` dice qué papel cumple en ella. + def create_transfer_touching_warehouse(as:) + other = Warehouse.create!(company: company, name: 'Sucursal', zip_code: '1900', address: 'Calle 2') + product = Product.create!(company: company, sku: 'SKU-3', name: 'En viaje') + origin, destination = as == :origin ? [warehouse, other] : [other, warehouse] + StockTransfer.create!(company: company, product: product, origin_warehouse: origin, + destination_warehouse: destination, quantity: 1, dispatched_at: Time.current) + end end From 7b6cb68d2c0641cec3b57d9c4de4d5ac4914576e Mon Sep 17 00:00:00 2001 From: LorenzoBellomo Date: Thu, 24 Sep 2026 19:43:54 -0300 Subject: [PATCH 07/13] fix: [TESIS-82] answer 400 to a warehouse body that is not an object POST and PUT /warehouses with {"warehouse": "x"} answered 500. require returns whatever the key holds, and permit on a String raises NoMethodError. warehouse_params now uses params.expect, which answers 400 for anything that is not an object. The comment it replaces said expect would also answer 400 to a company_id in the body; in Rails 8.1 it drops unpermitted keys silently, and the company_id specs of TESIS-97 still pass. Co-Authored-By: Claude Opus 5.5 --- .../api/v1/warehouses_controller.rb | 13 +++++++----- spec/requests/api/v1/warehouses_spec.rb | 20 +++++++++++++++++++ 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/app/controllers/api/v1/warehouses_controller.rb b/app/controllers/api/v1/warehouses_controller.rb index 6cd84cf3..3eaad3dd 100644 --- a/app/controllers/api/v1/warehouses_controller.rb +++ b/app/controllers/api/v1/warehouses_controller.rb @@ -46,12 +46,15 @@ def set_warehouse authorize @warehouse end + # `expect` y no `require` + `permit`: si `warehouse` llega como String o + # como Array, `require` lo devuelve igual y `permit` revienta con + # NoMethodError, que salía como 500. `expect` responde 400. + # + # Un company_id en el body se sigue ignorando: `expect` descarta en + # silencio las claves que no permite. (El comentario anterior decía que + # respondía 400; en Rails 8.1 no es así, y el spec de company_id lo fija.) def warehouse_params - # permit (no expect) es intencional y load-bearing: expect usa - # on_unpermitted: :raise, así que un body con company_id daría 400 en - # vez de ignorarlo — rompiendo el requisito de la card. - # rubocop:disable-next Rails/StrongParametersExpect - params.require(:warehouse).permit(:name, :zip_code, :address) + params.expect(warehouse: %i[name zip_code address]) end # Tres cosas bloquean el borrado y el mensaje tiene que decir cuál. Se diff --git a/spec/requests/api/v1/warehouses_spec.rb b/spec/requests/api/v1/warehouses_spec.rb index bbbdbdd2..6e9594a2 100644 --- a/spec/requests/api/v1/warehouses_spec.rb +++ b/spec/requests/api/v1/warehouses_spec.rb @@ -202,6 +202,18 @@ def stock_queries_while(&) post '/api/v1/warehouses', params: {}, headers: headers, as: :json expect(response).to have_http_status(:bad_request) end + + # Hallazgo de la QA de TESIS-82: `require` devolvía lo que llegara en la + # clave y `permit` reventaba sobre un String, con un 500. + it 'returns 400 when warehouse is not an object' do + post '/api/v1/warehouses', params: { warehouse: 'Central' }, headers: headers, as: :json + expect(response).to have_http_status(:bad_request) + end + + it 'returns 400 when warehouse is a list' do + post '/api/v1/warehouses', params: { warehouse: [warehouse_attrs] }, headers: headers, as: :json + expect(response).to have_http_status(:bad_request) + end end describe 'PUT /api/v1/warehouses/:id' do @@ -232,6 +244,14 @@ def stock_queries_while(&) expect(response).to have_http_status(:not_found) end + it 'returns 400 and changes nothing when warehouse is not an object', :aggregate_failures do + put "/api/v1/warehouses/#{warehouse.id}", params: { warehouse: 'Actualizado' }, + headers: headers, as: :json + + expect(response).to have_http_status(:bad_request) + expect(warehouse.reload.name).to eq('Central') + end + it 'updates via PATCH as well', :aggregate_failures do patch "/api/v1/warehouses/#{warehouse.id}", params: { warehouse: { name: 'Actualizado' } }, From 87ea8f90f7306165b2cc4b1ce298a9279beaa88e Mon Sep 17 00:00:00 2001 From: LorenzoBellomo Date: Thu, 24 Sep 2026 19:44:26 -0300 Subject: [PATCH 08/13] docs: [TESIS-82] record the auth decisions of the module QA in ADR-002 Registration as an access request, the per-request check of the company and the approval, the attempt limit per IP and the email normalization of the login, with the alternatives that were discarded and what the deployed environment needs for the attempt limit to count. Co-Authored-By: Claude Opus 5.5 --- docs/adr/ADR-002-authentication.md | 68 ++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/docs/adr/ADR-002-authentication.md b/docs/adr/ADR-002-authentication.md index 64ed64aa..4b2918f0 100644 --- a/docs/adr/ADR-002-authentication.md +++ b/docs/adr/ADR-002-authentication.md @@ -84,3 +84,71 @@ una pestaña. Hay un spec dedicado a ese caso. puramente stateless: es el precio de poder revocar - ⚠️ La tabla es global y **no** lleva `CompanyScoped`. Filtrar por empresa dejaría pasar un token revocado desde otro contexto de tenant + + +--- + +## Actualización — QA del módulo de auth (2026-09-24, TESIS-82) + +La validación del módulo encontró cuatro huecos en cómo se entra y cómo se +sigue adentro. Esta sección registra cómo se cerró cada uno. + +### El registro es una solicitud de acceso + +`POST /auth/register` creaba una cuenta que podía loguearse en el acto, dentro +de la empresa que nombrara el header `X-Tenant-Slug`. El slug es público (es el +subdominio), así que cualquiera podía darse de alta en cualquier empresa y leer +todos sus datos. TESIS-120 ya había sacado `company_id` del body, pero el slug +que lo reemplazó también lo elige quien llama. + +Registrarse pasa a ser pedir acceso, que es lo que dice la pantalla S02 +(«Solicitá acceso al espacio de operación de tu organización»): + +- `users.approved`, booleano con **default `true`**. Las cuentas que ya existían + y las que crean el backoffice, los seeds o la consola nacen habilitadas; sólo + `Auth::RegisterUser` crea cuentas en `false`. +- Una cuenta sin aprobar no obtiene token: el login le responde el mismo 401 que + a una password incorrecta. Se aprueba desde el backoffice (campo `approved` del + recurso User). +- El endpoint responde **202 con el mismo cuerpo** haya creado la solicitud o no. + El email es único en toda la base, y el viejo 422 «Email has already been + taken» decía qué emails tenían cuenta en cualquier empresa. Los errores de + formato se siguen informando, pero antes de mirar si el email existe: si no, + una password corta respondería distinto según el email estuviera tomado. + +Se descartó hacer el email único por empresa: resolvía la enumeración pero +pedía reemplazar la validación de Devise y una migración de índices, y el 202 +indistinguible ya la cierra. + +### La sesión se revisa en cada request, no sólo al loguearse + +El JWT sigue siendo válido hasta vencer aunque cambie algo que el token no +puede saber. Una empresa dada de baja seguía operando hasta 24 h con los tokens +que ya tenía. `ApplicationController#authenticate_user!` revisa ahora, después +de Devise, que la empresa siga activa y la cuenta aprobada, y responde 401 si +no. + +Va sobre `authenticate_user!` y no en `active_for_authentication?` a propósito: +el hook de Devise corta con 401 cualquier request que traiga el token, también +los que no exigen sesión (login, registro, tenant-config). + +### Límite de intentos por IP + +No había freno: después de 30 passwords incorrectas seguidas, la correcta +entraba. Login y registro aceptan ahora 10 intentos cada 3 minutos por IP +(`Api::V1::Auth::AttemptLimit`, sobre el `rate_limit` de Rails) y después +responden 429 con `Retry-After`. + +- Se prefirió al `:lockable` de Devise porque bloquear la cuenta deja que + cualquiera deje afuera a otro usuario tipeando mal su password a propósito. +- El contador vive en la cache de la app: Solid Cache en producción, compartida + por todos los procesos. ⚠️ El entorno desplegado necesita la base de cache + (`proyecto_api_production_cache`), y `request.remote_ip` tiene que ser la IP + del cliente y no la del proxy: si no, todos comparten un solo contador. + +### El email del login se normaliza + +Devise guarda el email en minúsculas y sin espacios, pero sólo normaliza al +guardar y en sus propios finders. `Auth::AuthenticateUser` busca con su propio +`find_by`, así que quien se registró como «Ana@Norte.com» recibía 401 con la +password correcta. Ahora normaliza igual antes de buscar. From a53b91be17df2856193052e48a4c98861d31aa24 Mon Sep 17 00:00:00 2001 From: LorenzoBellomo Date: Fri, 25 Sep 2026 09:46:07 -0300 Subject: [PATCH 09/13] fix: [TESIS-82] let a suspended session log out and revoke its token The session check answered 401 to DELETE /auth/logout when the company was inactive or the account unapproved, so the token never reached the denylist and worked again if the company was reactivated within its 24 h. Co-Authored-By: Claude Opus 5.5 --- .../api/v1/auth/sessions_controller.rb | 10 +++++++ spec/requests/api/v1/auth_spec.rb | 27 +++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/app/controllers/api/v1/auth/sessions_controller.rb b/app/controllers/api/v1/auth/sessions_controller.rb index 67b8c673..c0473f00 100644 --- a/app/controllers/api/v1/auth/sessions_controller.rb +++ b/app/controllers/api/v1/auth/sessions_controller.rb @@ -40,6 +40,16 @@ def destroy head :no_content end + + private + + # El logout pasa aunque la empresa esté dada de baja o la cuenta haya + # perdido la aprobación. Es lo único que quien quedó afuera todavía quiere + # hacer, y si se le respondiera 401 el token nunca entraría a la denylist: + # con la empresa reactivada o la cuenta aprobada de nuevo dentro de sus + # 24 h, cualquier copia de ese token volvería a servir. El token sí tiene + # que ser válido: eso lo sigue exigiendo Devise. + def session_refusal = nil end end end diff --git a/spec/requests/api/v1/auth_spec.rb b/spec/requests/api/v1/auth_spec.rb index 2b0c63ef..bcf5c3f0 100644 --- a/spec/requests/api/v1/auth_spec.rb +++ b/spec/requests/api/v1/auth_spec.rb @@ -265,6 +265,33 @@ def request_after_change expect(response).to have_http_status(:unauthorized) expect(response.parsed_body['error']).to eq('This account is pending approval') end + + # El logout es lo único que quien quedó afuera todavía puede querer hacer. Si + # se le respondiera 401, el token no entraría a la denylist y volvería a + # servir al revertirse el cambio dentro de sus 24 h. + def logout_and_revert(change, revert) + headers = auth + change.call + delete '/api/v1/auth/logout', headers: headers + status = response.status + revert.call + get '/api/v1/warehouses', headers: headers + [status, response.status] + end + + it 'can still log out while the company is inactive, for good' do + statuses = logout_and_revert(-> { company.update!(is_active: false) }, + -> { company.update!(is_active: true) }) + + expect(statuses).to eq([204, 401]) + end + + it 'can still log out while the account is pending approval, for good' do + statuses = logout_and_revert(-> { user.update!(approved: false) }, + -> { user.update!(approved: true) }) + + expect(statuses).to eq([204, 401]) + end end describe 'DELETE /api/v1/auth/logout' do From f596747297b12565a8c0a1420b15e2e0d357470c Mon Sep 17 00:00:00 2001 From: LorenzoBellomo Date: Fri, 25 Sep 2026 09:46:08 -0300 Subject: [PATCH 10/13] fix: [TESIS-82] count only the failed logins against the attempt limit rate_limit counted every POST /auth/login, so the eleventh correct login from the same IP (operators behind one NAT, the demo) got 429. The login now counts only the 401s; the registration keeps counting every request. Co-Authored-By: Claude Opus 5.5 --- .../api/v1/auth/registrations_controller.rb | 2 + .../api/v1/auth/sessions_controller.rb | 2 + .../concerns/api/v1/auth/attempt_limit.rb | 46 +++++++++++++++---- .../api/v1/auth_attempt_limit_spec.rb | 15 ++++-- 4 files changed, 54 insertions(+), 11 deletions(-) diff --git a/app/controllers/api/v1/auth/registrations_controller.rb b/app/controllers/api/v1/auth/registrations_controller.rb index b7135580..de1e95e7 100644 --- a/app/controllers/api/v1/auth/registrations_controller.rb +++ b/app/controllers/api/v1/auth/registrations_controller.rb @@ -7,6 +7,8 @@ class RegistrationsController < ApplicationController include TenantFromSlug include AttemptLimit + rate_limit to: MAX_ATTEMPTS, within: WINDOW, only: :create, + with: :render_too_many_attempts skip_before_action :authenticate_user! skip_after_action :verify_authorized, :verify_policy_scoped diff --git a/app/controllers/api/v1/auth/sessions_controller.rb b/app/controllers/api/v1/auth/sessions_controller.rb index c0473f00..b0ee97c2 100644 --- a/app/controllers/api/v1/auth/sessions_controller.rb +++ b/app/controllers/api/v1/auth/sessions_controller.rb @@ -7,6 +7,7 @@ class SessionsController < ApplicationController include TenantFromSlug include AttemptLimit + before_action :refuse_exhausted_attempts, only: :create skip_before_action :authenticate_user!, only: :create skip_after_action :verify_authorized, :verify_policy_scoped @@ -25,6 +26,7 @@ def create if token render json: { token: token }, status: :ok else + count_failed_attempt render json: { error: 'Invalid email or password' }, status: :unauthorized end end diff --git a/app/controllers/concerns/api/v1/auth/attempt_limit.rb b/app/controllers/concerns/api/v1/auth/attempt_limit.rb index ee1824ff..58a3ee75 100644 --- a/app/controllers/concerns/api/v1/auth/attempt_limit.rb +++ b/app/controllers/concerns/api/v1/auth/attempt_limit.rb @@ -6,10 +6,18 @@ module Auth # Límite de intentos para los endpoints públicos de auth: login y registro. # # Sin esto se podían probar contraseñas sin freno: después de 30 incorrectas - # seguidas, la correcta entraba igual. Se limita por IP con el rate_limit de - # Rails y no con el :lockable de Devise, que bloquea la cuenta: con él, - # cualquiera podría dejar afuera a otro usuario tipeando mal su password a - # propósito. + # seguidas, la correcta entraba igual. Se limita por IP y no con el + # :lockable de Devise, que bloquea la cuenta: con él, cualquiera podría + # dejar afuera a otro usuario tipeando mal su password a propósito. + # + # Cada endpoint cuenta distinto: + # - Login: sólo los intentos fallidos (before_action + # :refuse_exhausted_attempts + count_failed_attempt en la rama del 401). + # El que entra no está adivinando nada, y contar los exitosos dejaba + # afuera al undécimo operario de un depósito detrás de un mismo NAT + # aunque pusiera bien su password. + # - Registro: todos, con el rate_limit de Rails. Cada pedido crea una + # solicitud, y no hay un «pedido exitoso» que se repita en el uso normal. # # El contador vive en la cache de la app, que en producción es Solid Cache y # la comparten todos los procesos. En test la cache es :null_store y el @@ -21,12 +29,34 @@ module AttemptLimit MAX_ATTEMPTS = 10 WINDOW = 3.minutes - included do - rate_limit to: MAX_ATTEMPTS, within: WINDOW, only: :create, - with: :render_too_many_attempts + private + + # Una vez agotados se rechaza sin mirar la password, también la correcta: + # si se la evaluara, el que prueba contraseñas seguiría probando y la + # correcta le daría el token igual. + def refuse_exhausted_attempts + render_too_many_attempts if failed_attempts >= MAX_ATTEMPTS end - private + # La ventana es fija: el increment de la cache conserva el vencimiento + # que tomó la entrada con el primer fallo. + def count_failed_attempt + attempt_store.increment(failed_attempts_key, 1, expires_in: WINDOW) + end + + # `raw: true` porque el valor lo escribió increment, que en algunos stores + # guarda el entero sin serializar. + def failed_attempts + attempt_store.read(failed_attempts_key, raw: true).to_i + end + + def failed_attempts_key + "auth-failed-attempts:#{request.remote_ip}" + end + + def attempt_store + self.class.cache_store + end def render_too_many_attempts response.set_header('Retry-After', WINDOW.to_i.to_s) diff --git a/spec/requests/api/v1/auth_attempt_limit_spec.rb b/spec/requests/api/v1/auth_attempt_limit_spec.rb index bed49246..ab1d4c88 100644 --- a/spec/requests/api/v1/auth_attempt_limit_spec.rb +++ b/spec/requests/api/v1/auth_attempt_limit_spec.rb @@ -17,9 +17,9 @@ before do User.create!(email: 'log@test.com', password: 'password123', company: company) - allow(Api::V1::Auth::SessionsController.cache_store).to receive(:increment) do |*args, **options| - counter.increment(*args, **options) - end + store = Api::V1::Auth::SessionsController.cache_store + allow(store).to receive(:increment) { |*args, **options| counter.increment(*args, **options) } + allow(store).to receive(:read) { |*args, **options| counter.read(*args, **options) } end def login(password: 'wrong', ip: '203.0.113.7') @@ -38,6 +38,15 @@ def use_up_attempts(ip: '203.0.113.7') expect(login(password: 'password123')).to have_http_status(:ok) end + # Lo que se frena es al que prueba contraseñas, no al que entra: un depósito + # con varios operarios detrás del mismo NAT loguea muchas veces al empezar el + # turno. + it 'does not count the logins that succeed' do + (max_attempts + 2).times { login(password: 'password123') } + + expect(response).to have_http_status(:ok) + end + it 'answers 429 once the attempts run out, even with the right password', :aggregate_failures do use_up_attempts login(password: 'password123') From a3b024d47837f6071f4c393681283eba9f1882ed Mon Sep 17 00:00:00 2001 From: LorenzoBellomo Date: Fri, 25 Sep 2026 09:46:10 -0300 Subject: [PATCH 11/13] fix: [TESIS-82] run bcrypt on logins without an account An unknown email or tenant skipped bcrypt and answered the 401 much faster than a wrong password, so the response time enumerated users. Compare against a throwaway digest with the same cost instead. Co-Authored-By: Claude Opus 5.5 --- app/poros/auth/authenticate_user.rb | 24 ++++++++++++++++++++--- spec/poros/auth/authenticate_user_spec.rb | 18 +++++++++++++++++ 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/app/poros/auth/authenticate_user.rb b/app/poros/auth/authenticate_user.rb index 6c37ac4c..eb8951ed 100644 --- a/app/poros/auth/authenticate_user.rb +++ b/app/poros/auth/authenticate_user.rb @@ -14,21 +14,39 @@ def initialize(email:, password:, company:) # todavía no aprobaron. El caller no puede distinguir los casos, que es # justamente el punto: el 401 tiene que ser idéntico para todos. def call - return nil if @company.nil? - user = find_user - return nil unless user&.valid_password?(@password) && user.approved? + return nil unless password_matches?(user) && user.approved? Warden::JWTAuth::UserEncoder.new.call(user, :user, nil).first end + # Digest descartable contra el que se compara cuando no hay cuenta. Se arma + # con el mismo Devise::Encryptor (y por lo tanto el mismo costo) que las + # passwords reales. + def self.dummy_digest + @dummy_digest ||= Devise::Encryptor.digest(User, SecureRandom.hex(32)) + end + private + # Se corre bcrypt aunque no haya cuenta que comparar. Si no, el 401 de un + # email inexistente (o de un tenant no resuelto) volvía mucho antes que el + # de una password incorrecta, y el tiempo de respuesta decía qué emails + # existen aunque el cuerpo fuera idéntico. + def password_matches?(user) + return user.valid_password?(@password) if user + + Devise::Encryptor.compare(User, self.class.dummy_digest, @password) + false + end + # `unscoped` explícito: User incluye CompanyScoped, y si el request de login # llegara con un JWT viejo en el header, el default scope filtraría por el # tenant de ese token y no por el que se está intentando. El scope acá es el # de la company resuelta por slug, y sólo ese. def find_user + return nil if @company.nil? + User.unscoped.find_by(company_id: @company.id, email: normalized_email) end diff --git a/spec/poros/auth/authenticate_user_spec.rb b/spec/poros/auth/authenticate_user_spec.rb index 8cf5188d..1f9da194 100644 --- a/spec/poros/auth/authenticate_user_spec.rb +++ b/spec/poros/auth/authenticate_user_spec.rb @@ -56,6 +56,24 @@ expect(token).to be_nil end + # Sin cuenta que comparar se corre bcrypt igual: si no, el tiempo de respuesta + # diría qué emails existen aunque el resultado sea el mismo nil. + it 'still runs bcrypt when the email does not exist' do + allow(Devise::Encryptor).to receive(:compare).and_call_original + + described_class.new(email: 'ghost@test.com', password: 'password123', company: company).call + + expect(Devise::Encryptor).to have_received(:compare).with(User, described_class.dummy_digest, 'password123') + end + + it 'still runs bcrypt when the tenant could not be resolved' do + allow(Devise::Encryptor).to receive(:compare).and_call_original + + described_class.new(email: 'log@test.com', password: 'password123', company: nil).call + + expect(Devise::Encryptor).to have_received(:compare).once + end + # User incluye CompanyScoped: si el default scope se colara acá, un Current # heredado de otro request decidiría el tenant en vez del slug. it 'ignores a leftover Current.company_id and honours the given company' do From a200b13592d6de646b51e00e605dad3f324235ad Mon Sep 17 00:00:00 2001 From: LorenzoBellomo Date: Fri, 25 Sep 2026 09:46:10 -0300 Subject: [PATCH 12/13] refactor: [TESIS-82] authorize the integrations update with Pundit Move the integrations feature check from a before_action to CompanyIntegrationPolicy#update?, so verify_authorized enforces it. Co-Authored-By: Claude Opus 5.5 --- .../api/v1/integrations_controller.rb | 21 +++------- app/policies/company_integration_policy.rb | 11 +++++ .../company_integration_policy_spec.rb | 42 +++++++++++++++++++ 3 files changed, 59 insertions(+), 15 deletions(-) create mode 100644 app/policies/company_integration_policy.rb create mode 100644 spec/policies/company_integration_policy_spec.rb diff --git a/app/controllers/api/v1/integrations_controller.rb b/app/controllers/api/v1/integrations_controller.rb index 515c0569..790b6c50 100644 --- a/app/controllers/api/v1/integrations_controller.rb +++ b/app/controllers/api/v1/integrations_controller.rb @@ -3,14 +3,11 @@ module Api module V1 class IntegrationsController < ApplicationController - skip_after_action :verify_authorized, :verify_policy_scoped - - # El flag `integrations` de la empresa no se miraba acá: el front oculta la - # sección, pero una empresa sin la feature la configuraba igual llamando al - # endpoint directo. Se corta el alta y la modificación. El listado queda - # abierto porque lo usa el widget de nodos del panel, y sólo muestra las - # plantillas globales con el estado de la propia empresa. - before_action :require_integrations_feature, only: :update + # El listado no pasa por Pundit: lo usa el widget de nodos del panel + # aunque la empresa no tenga la feature `integrations`, y sólo muestra las + # plantillas globales con el estado de la propia empresa. El alta y la + # modificación sí: ver CompanyIntegrationPolicy. + skip_after_action :verify_policy_scoped def index integrations = current_company.company_integrations.index_by(&:service_id) @@ -20,6 +17,7 @@ def index end def update + authorize CompanyIntegration integration = Integrations::UpsertIntegration.new( company: current_company, service_id: params[:service_id], @@ -31,13 +29,6 @@ def update private - def require_integrations_feature - return if current_company.feature_enabled?(:integrations) - - render json: { error: 'The integrations feature is not enabled for this company' }, - status: :forbidden - end - def credentials_params raw = params.require(:credentials) unless raw.is_a?(ActionController::Parameters) diff --git a/app/policies/company_integration_policy.rb b/app/policies/company_integration_policy.rb new file mode 100644 index 00000000..a658233e --- /dev/null +++ b/app/policies/company_integration_policy.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class CompanyIntegrationPolicy < ApplicationPolicy + # El flag `integrations` de la empresa sólo lo aplicaba el front: una empresa + # sin la feature configuraba integraciones llamando al endpoint directo (QA de + # TESIS-82). Se compara con `== true` en Company#feature_enabled?, igual que + # el front, así los dos deciden lo mismo. + def update? + user.present? && user.company.feature_enabled?(:integrations) + end +end diff --git a/spec/policies/company_integration_policy_spec.rb b/spec/policies/company_integration_policy_spec.rb new file mode 100644 index 00000000..b1281f2e --- /dev/null +++ b/spec/policies/company_integration_policy_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe CompanyIntegrationPolicy, type: :policy do + subject(:policy) { described_class.new(user, CompanyIntegration) } + + let(:company) do + Company.create!(name: 'Tenant A', tax_id: '30-11111111-1', features: { 'integrations' => true }) + end + let(:user) { User.create!(email: 'a@example.com', password: 'password123', company: company) } + + it 'lets a company with the integrations feature configure them' do + expect(policy.update?).to be(true) + end + + context 'when the company does not have the feature' do + before { company.update!(features: { 'integrations' => false }) } + + it 'denies configuring them' do + expect(policy.update?).to be(false) + end + end + + # Igual que el front (`features?.[feature] === true`): un "true" string es + # apagado en los dos lados. + context 'when the flag is not a real boolean' do + before { company.update!(features: { 'integrations' => 'true' }) } + + it 'denies configuring them' do + expect(policy.update?).to be(false) + end + end + + context 'without a user' do + let(:user) { nil } + + it 'denies configuring them' do + expect(policy.update?).to be(false) + end + end +end From 46b9b2922fb0d7f3689498cde5b9704c82f81c7a Mon Sep 17 00:00:00 2001 From: LorenzoBellomo Date: Fri, 25 Sep 2026 09:46:10 -0300 Subject: [PATCH 13/13] docs: [TESIS-82] record the review decisions in ADR-002 Co-Authored-By: Claude Opus 5.5 --- docs/adr/ADR-002-authentication.md | 33 ++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/docs/adr/ADR-002-authentication.md b/docs/adr/ADR-002-authentication.md index 4b2918f0..9f89a295 100644 --- a/docs/adr/ADR-002-authentication.md +++ b/docs/adr/ADR-002-authentication.md @@ -132,12 +132,29 @@ Va sobre `authenticate_user!` y no en `active_for_authentication?` a propósito: el hook de Devise corta con 401 cualquier request que traiga el token, también los que no exigen sesión (login, registro, tenant-config). +El logout es la excepción: `DELETE /auth/logout` sólo exige un token válido, y +revoca aunque la empresa esté inactiva o la cuenta sin aprobar. Si respondiera +401, el token nunca entraría a la denylist y, como el corte es reversible, +cualquier copia de él volvería a servir al reactivarse la empresa o aprobarse +de nuevo la cuenta dentro de sus 24 h. + +El corte es una suspensión, no una baja: reactivar la empresa devuelve el +acceso a los tokens que siguen vivos y que nadie cerró. Revocar todos los tokens +de la empresa al desactivarla se descartó por ahora: la denylist guarda `jti` +emitidos, no sesiones abiertas, y no hay de dónde sacar los que no se cerraron. + ### Límite de intentos por IP No había freno: después de 30 passwords incorrectas seguidas, la correcta entraba. Login y registro aceptan ahora 10 intentos cada 3 minutos por IP -(`Api::V1::Auth::AttemptLimit`, sobre el `rate_limit` de Rails) y después -responden 429 con `Retry-After`. +(`Api::V1::Auth::AttemptLimit`) y después responden 429 con `Retry-After`. + +- En el login cuentan **sólo los intentos fallidos**. Contar todos (el + `rate_limit` de Rails cuenta requests) dejaba afuera al undécimo operario que + entra al turno detrás del mismo NAT, con la password correcta. Agotados los + intentos se rechaza sin evaluar la password, también la correcta. +- En el registro cuentan todos, con el `rate_limit` de Rails: cada pedido crea + una solicitud y no hay un uso normal que lo repita. - Se prefirió al `:lockable` de Devise porque bloquear la cuenta deja que cualquiera deje afuera a otro usuario tipeando mal su password a propósito. @@ -152,3 +169,15 @@ Devise guarda el email en minúsculas y sin espacios, pero sólo normaliza al guardar y en sus propios finders. `Auth::AuthenticateUser` busca con su propio `find_by`, así que quien se registró como «Ana@Norte.com» recibía 401 con la password correcta. Ahora normaliza igual antes de buscar. + +### El tiempo del 401 no delata qué emails existen + +Con un email inexistente (o un tenant no resuelto) no había password contra la +cual correr bcrypt, y el 401 volvía mucho antes que el de una password +incorrecta: el cuerpo era idéntico, pero el tiempo de respuesta enumeraba +usuarios. `Auth::AuthenticateUser` compara ahora contra un digest descartable, +armado con el mismo `Devise::Encryptor` y el mismo costo, cuando no encuentra la +cuenta. + +- ⚠️ En `Auth::RegisterUser` queda una diferencia más chica: el camino del email + ya tomado se saltea el INSERT. Se deja como limitación conocida.