Stop failed sign-ins writing current_platform; fix resending orphaned invites

.rubocop.yml

@@ -1126,9 +1126,6 @@ Style/Documentation:
 Style/DocumentationMethod:
   Enabled: false
 
-Style/DoubleCopDisableDirective:
-  Enabled: false
-
 Style/DoubleNegation:
   Enabled: false
 

AGENTS.md

@@ -103,7 +103,7 @@ Cross-platform push via the `noticed` (v3) and `action_push_native` gems. APNs f
   - `json_response` for parsing JSON API responses
   - `create_new_auth_token` for generating auth headers (Devise Token Auth)
   - Fixtures in test/fixtures/ and seed data in db/fixtures/test/
-- Run tests: `bin/rails test` (537 tests, 1331 assertions)
+- Run tests: `bin/rails test` (540 tests, 1342 assertions)
 
 ### Development Server Configuration
 - `HOST` (Wi-Fi IP) and `PORT` are required in `.env`; `Procfile.dev` uses `${HOST:?...}` so Rails fails loudly if unset, and `development.rb` uses `ENV.fetch("HOST")` for `action_mailer.default_url_options`. Must match `NATIVEAPPTEMPLATE_API_DOMAIN` in the iOS scheme and Android `gradle.properties`. Never `127.0.0.1`, `localhost`, or `0.0.0.0`.

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## [Unreleased]
 
+- Fix a failed sign-in saving the `source` header as `current_platform`: anyone knowing an email could set it to any string, after which that shopkeeper's profile updates returned 422. Only successful sign-ins write it, and only `ios` or `android`. Fix resending an invitation whose inviter was deleted (the mail job raised on `nil.name`); the mail names the app instead. Rename `.erb-lint.yml` to `.erb_lint.yml` and drop a removed RuboCop cop from `.rubocop.yml` (#149)
 - Deploy with Kamal 2 to GitHub Container Registry instead of Render: add `Dockerfile`, `.dockerignore`, `config/deploy.yml` (kamal-proxy serving a Cloudflare Origin Certificate with `forward_headers: true` and `ssl_redirect: false`, Postgres 18 as the `db` accessory on the private Docker network, Solid Queue in Puma, Active Storage volume, amd64 builder), `.kamal/secrets`, `bin/kamal` and `bin/thrust`. Add the missing `GET /up` health check route and `assume_ssl = true` so kamal-proxy's health check passes. Add Cloudflare's ranges to `trusted_proxies` (`config/cloudflare_ips.yml`) so `rate_limit` keys on the caller rather than the edge. Remove `render.yaml`, `bin/render-*.sh` and the staging environment, which only served Render PR previews. New tests: `test/config/deploy_config_test.rb`, `test/integration/health_check_test.rb`, `test/integration/trusted_proxies_test.rb` (#144)
 - Point the `SOLID_QUEUE_IN_PUMA` comment in `.env.sample` at `config/deploy.yml`; it still referred to the removed `render.yaml` (#147)
 - Require `devise_token_auth ~> 1.3` in the Gemfile (the lockfile already resolved 1.3.0) so `bundle update` cannot step back to 1.2.x (#142)
@@ -23,7 +24,7 @@
 - API change: members and invitations must have at least one role (422 "Roles can't be blank"). A role-less member made `GET /permissions` raise (#121)
 - API change: password reset and confirmation `redirect_url` must be on the API's own host. Other hosts get 422 for resets and fall back to the default page for confirmations (#120)
 - Harden admin sign-in: rate limit by IP and email, `reset_session` on sign-in and sign-out, and Madmin now rejects a deleted admin's session (#119)
-- AGENTS.md: correct the CORS and test-worker notes, and keep the test count current (#118, #127, #132, #147)
+- AGENTS.md: correct the CORS and test-worker notes, and keep the test count current (#118, #127, #132, #147, #149)
 - Clear finished Solid Queue jobs every hour via `config/recurring.yml` (#117)
 - API change: `item_tags#update` no longer accepts `state`. Completion goes through the `complete`/`idle` events, and an unknown state no longer returns 500 (#116)
 - API change: API errors are always JSON `{code, error_message}`, including missing records (404), missing parameters (400), unknown API routes and 500s; before, they were HTML unless the request sent a JSON Content-Type (#115)

app/controllers/shopkeeper_auth/sessions_controller.rb

@@ -15,20 +15,13 @@ class ShopkeeperAuth::SessionsController < DeviseTokenAuth::SessionsController
     if: -> { params[:email].present? },
     with: RENDER_LOGIN_THROTTLED
 
-  def create
-    super
-    return if @resource.blank?
-
-    source = request.headers["source"]
-    return if source.blank?
-
-    @resource.current_platform = source
-    @resource.save!(validate: false)
-  end
-
   protected
 
+  # devise_token_auth calls this only after the password checks out, so a
+  # failed sign-in never writes the platform
   def render_create_success
+    update_current_platform
+
     @resource.token = @token.token
     @resource.client = @token.client
     @resource.expiry = @token.expiry
@@ -52,4 +45,14 @@ def render_destroy_success
   def render_destroy_error
     render json: {code: 404, error_message: I18n.t("devise_token_auth.sessions.user_not_found")}, status: :not_found
   end
+
+  private
+
+  # The apps send "ios" or "android"; anything else keeps the stored value
+  def update_current_platform
+    source = request.headers["source"]
+    return unless Shopkeeper::CURRENT_PLATFORMS.include?(source)
+
+    @resource.update_column(:current_platform, source)
+  end
 end

app/mailers/shopkeeper/notification_mailer.rb

@@ -2,15 +2,17 @@ class Shopkeeper::NotificationMailer < ShopkeeperMailer
   def invited
     @accounts_invitation = params[:accounts_invitation]
     @account = @accounts_invitation.account
-    @invited_by = @accounts_invitation.invited_by
+    # invited_by is nullified when the inviter is deleted; the invitation can
+    # still be resent, so name the app instead
+    @inviter_name = @accounts_invitation.invited_by&.name || ConfigSettings.mobile_app.name
 
     name = @accounts_invitation.name
     email = @accounts_invitation.email
 
     mail(
       to: email_address_with_name(email, name),
-      from: email_address_with_name(ConfigSettings.email.default_from, @invited_by.name),
-      subject: I18n.t("shopkeeper.notification_mailer.invited.subject", inviter: @invited_by.name, account: @account.name)
+      from: email_address_with_name(ConfigSettings.email.default_from, @inviter_name),
+      subject: I18n.t("shopkeeper.notification_mailer.invited.subject", inviter: @inviter_name, account: @account.name)
     )
   end
 

app/models/shopkeeper.rb

@@ -1,4 +1,6 @@
 class Shopkeeper < ApplicationRecord
+  CURRENT_PLATFORMS = %w[ios android].freeze
+
   extend Devise::Models
   # Include default devise modules. Others available are:
   # :confirmable, :lockable, :timeoutable and :omniauthable
@@ -25,7 +27,7 @@ class Shopkeeper < ApplicationRecord
 
   validates :current_platform,
     presence: true,
-    inclusion: {in: %w[ios android]}
+    inclusion: {in: CURRENT_PLATFORMS}
 
   # override devise method to include additional info as opts hash
   def send_confirmation_instructions(opts = {})

app/views/shopkeeper/notification_mailer/invited.html.erb

@@ -1,5 +1,5 @@
 <h1 style="font-weight:400;line-height:1.25;">
-  <%= t(".header_html", inviter: @invited_by.name, account: @account.name) %>
+  <%= t(".header_html", inviter: @inviter_name, account: @account.name) %>
 </h1>
 
 <p style="margin-bottom:2rem;">

app/views/shopkeeper/notification_mailer/invited.text.erb

@@ -1,4 +1,4 @@
-<%= strip_tags t(".header_html", inviter: @invited_by.name, account: @account.name) %>
+<%= strip_tags t(".header_html", inviter: @inviter_name, account: @account.name) %>
 
 <%= t(".you_can_accept_or_decline") %>
 

test/controllers/shopkeeper_auth/sessions_controller_test.rb

@@ -53,4 +53,37 @@ class ShopkeeperAuth::SessionsControllerTest < ActionDispatch::IntegrationTest
     assert_response :success
     assert_equal "ios", shopkeeper.reload.current_platform
   end
+
+  # Bug: devise_token_auth finds the shopkeeper by email before checking the
+  # password, so the source header used to be saved on a failed sign-in too.
+  # Anyone knowing an email could set current_platform to any string, which
+  # then failed the inclusion validation and broke that shopkeeper's updates.
+  test "failed sign-in does not change current_platform" do
+    shopkeeper = shopkeepers(:one)
+    shopkeeper.create_default_account
+    shopkeeper.update_column(:current_platform, "ios")
+
+    post shopkeeper_session_url,
+      params: {email: shopkeeper.email, password: "wrong-password"},
+      headers: {source: "android"}
+
+    assert_response :unauthorized
+    assert_equal "ios", shopkeeper.reload.current_platform
+  end
+
+  # Shopkeeper validates current_platform against ios/android
+  # (Shopkeeper::CURRENT_PLATFORMS); a sign-in must not store anything else.
+  test "successful sign-in with an unsupported source header preserves the existing current_platform" do
+    shopkeeper = shopkeepers(:one)
+    shopkeeper.create_default_account
+    shopkeeper.update_column(:current_platform, "ios")
+
+    post shopkeeper_session_url,
+      params: {email: shopkeeper.email, password: "password"},
+      headers: {source: "hacked"}
+
+    assert_response :success
+    assert_equal "ios", shopkeeper.reload.current_platform
+    assert shopkeeper.valid?, shopkeeper.errors.full_messages.to_sentence
+  end
 end

test/mailers/shopkeeper/notification_mailer_test.rb

@@ -29,6 +29,25 @@ class Shopkeeper::NotificationMailerTest < ActionMailer::TestCase
     assert_match @account.name, mail.body.encoded
   end
 
+  # Bug: deleting the inviter nullifies invited_by, and resending such an
+  # invitation raised NoMethodError on nil.name inside the mail job. The mail
+  # now names the app instead (config/settings.yml mobile_app.name).
+  test "invited names the app when the inviter has been deleted" do
+    @invitation.update!(invited_by: nil)
+
+    mail = Shopkeeper::NotificationMailer.with(accounts_invitation: @invitation).invited
+
+    expected_subject = I18n.t(
+      "shopkeeper.notification_mailer.invited.subject",
+      inviter: ConfigSettings.mobile_app.name,
+      account: @account.name
+    )
+    assert_equal expected_subject, mail.subject
+    assert_equal [@invitation.email], mail.to
+    assert_match @invitation.token, mail.body.encoded
+    assert_match "#{ConfigSettings.mobile_app.name} has invited you", mail.text_part.body.decoded
+  end
+
   test "confirmation_instructions renders the expected subject, recipient, and body" do
     mail = Shopkeeper::NotificationMailer.with(
       resource: @shopkeeper,