fix: [TESIS-82] close the auth and multi-tenancy gaps found by the module QA

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
 

app/controllers/api/v1/auth/registrations_controller.rb

@@ -5,22 +5,33 @@ module V1
     module Auth
       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
 
         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.

app/controllers/api/v1/auth/sessions_controller.rb

@@ -5,7 +5,9 @@ module V1
     module Auth
       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
 
@@ -24,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
@@ -39,6 +42,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

app/controllers/api/v1/integrations_controller.rb

@@ -3,7 +3,11 @@
 module Api
   module V1
     class IntegrationsController < ApplicationController
-      skip_after_action :verify_authorized, :verify_policy_scoped
+      # 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)
@@ -13,6 +17,7 @@ def index
       end
 
       def update
+        authorize CompanyIntegration
         integration = Integrations::UpsertIntegration.new(
           company: current_company,
           service_id: params[:service_id],

app/controllers/api/v1/warehouses_controller.rb

@@ -46,20 +46,30 @@ 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
 
-      # 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

app/controllers/application_controller.rb

@@ -42,6 +42,28 @@ 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 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,
+  # 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
+    refusal = session_refusal
+    render json: { error: refusal }, status: :unauthorized if refusal
+  end
+
+  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
     Current.company_id = current_user&.company_id
     Current.user = current_user

app/controllers/concerns/api/v1/auth/attempt_limit.rb

@@ -0,0 +1,68 @@
+# 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 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
+      # 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
+
+        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
+
+        # 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)
+          render json: { error: 'Too many attempts, try again later' }, status: :too_many_requests
+        end
+      end
+    end
+  end
+end

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

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,

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

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

app/poros/auth/authenticate_user.rb

@@ -10,26 +10,53 @@ 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 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
-      User.unscoped.find_by(company_id: @company.id, email: @email)
+      return nil if @company.nil?
+
+      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