diff --git a/.gitignore b/.gitignore index 0503704..023b8bd 100644 --- a/.gitignore +++ b/.gitignore @@ -43,6 +43,11 @@ /config/credentials/development.key /config/credentials/production.key +# Kamal "demo" destination for the public demo of this template. It holds the +# real server IP, which must not reach this public repo or generated apps. +/config/deploy.demo.yml +/.kamal/secrets.demo + # Ignore other files .byebug_history .DS_Store diff --git a/AGENTS.md b/AGENTS.md index a3995d6..5582134 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -66,6 +66,7 @@ bin/rails dbconsole # Database console - **Multi-tenancy**: acts_as_tenant for complete data isolation between accounts - **RBAC**: Role and Permission models for fine-grained access control - **Organization errors**: an `//api/...` request for a deleted account gets `404 {code, error_code: "account_not_found", error_message}` from `AccountMiddleware` (via `Api::V1::Shopkeeper::AccountNotFoundController`, JSON whatever the request headers); a signed-in shopkeeper who is not a member gets `403 {code, error_code: "not_account_member", error_message}` from `Api::V1::Shopkeeper::BaseController`. The apps switch to the personal organization on these instead of signing out. Pundit denials stay `401 {code, error_message}`; devise_token_auth's invalid-token 401 is `{errors: [...]}`. Non-API paths with an unknown account id still redirect to `/`. +- **Mail throttling**: every endpoint that emails an address the caller names is rate-limited (429 `too_many_requests`). Password reset and confirmation resend: 5/minute per IP and 3/hour per address (`ThrottleMailRequests`); invitation create + resend: 20/hour per shopkeeper; sign-up: 10 per 3 minutes per IP. A new mail-sending endpoint needs a limit too (`test/integration/mail_throttle_test.rb`). ### Key Models & Relationships - `Account` - Top-level tenant/organization @@ -110,7 +111,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` (624 tests, 1934 assertions) +- Run tests: `bin/rails test` (635 tests, 2016 assertions) ### Internationalization - Responses are in English (default) or Japanese, picked by the `SetLocale` concern (included in `ApplicationController` and `NonApiApplicationController`). It wraps `process_action`, not `around_action`, so `rescue_from` handlers and `rate_limit` responses are localized too. @@ -135,7 +136,9 @@ Cross-platform push via the `noticed` (v3) and `action_push_native` gems. APNs f - Image processing with Active Storage and `image_processing` gem ### Deployment -- **This repo itself is never deployed.** It is the template that apps are generated from, so there is no production environment or production data. Don't weigh changes against deploy risk (migration downtime, existing records); do keep the Kamal config below correct, because generated apps inherit it. +- **This repo runs as a public demo** at `api.nativeapptemplate.com`, deployed with the Kamal `demo` destination (`bin/kamal deploy -d demo`). Its data is disposable: anyone can sign up and it may be reset at any time, so don't preserve records or plan for zero downtime. But a migration must still run on a populated database (`db:prepare` on deploy), and every change ships to the demo. Generated apps inherit the Kamal config below, so keep it correct. +- Demo-only settings live in `config/deploy.demo.yml` (the real server IP in `servers` and `accessories.db.host`) and `.kamal/secrets.demo`. A destination reads `.kamal/secrets.demo` *instead of* `.kamal/secrets`, so make it a symlink: `ln -s secrets .kamal/secrets.demo`. Both are gitignored (`test/config/deploy_config_test.rb` checks this); the committed `deploy.yml` keeps the `192.168.0.1` placeholder for generated apps. +- The demo sets `DEMO_PURGE_INACTIVE_DAYS: 30` in `deploy.demo.yml`, so `PurgeInactiveDemoShopkeepersJob` (daily, `config/recurring.yml`) deletes shopkeepers whose last sign-in, or sign-up if they never signed in, is older than that, with their owned accounts. Unset, the job does nothing; never put the variable in `deploy.yml`, or every generated app would delete its real users (`deploy_config_test.rb` checks this). - Deployed with [Kamal 2](https://kamal-deploy.org) (`bin/kamal`): `Dockerfile` builds the image, `config/deploy.yml` describes the server, and `.kamal/secrets` reads secrets from the environment and `config/credentials/production.key`. Images go to GitHub Container Registry (`ghcr.io`). - One server runs everything: the app (Puma behind Thruster on port 80, fronted by kamal-proxy serving a Cloudflare Origin Certificate with `forward_headers: true` and `ssl_redirect: false`; Cloudflare's ranges are in `config.action_dispatch.trusted_proxies` via `config/cloudflare_ips.yml` so `rate_limit` sees the caller, not the edge), and PostgreSQL as the `db` accessory on the private Docker network (`DB_HOST=-db`; never published to the internet). Solid Queue runs inside Puma via `SOLID_QUEUE_IN_PUMA=true`, so there is no separate job server. - Everything named `nativeapptemplateapi` in `config/deploy.yml`, `.kamal/secrets` and `config/database.yml` is renamed together by the agent; the accessory's `POSTGRES_USER`/`POSTGRES_DB` must keep matching `database.yml`'s production `username`/`database` (`test/config/deploy_config_test.rb` checks this). diff --git a/app/controllers/api/v1/shopkeeper/accounts/accounts_invitations_controller.rb b/app/controllers/api/v1/shopkeeper/accounts/accounts_invitations_controller.rb index 66ee235..cac6c5e 100644 --- a/app/controllers/api/v1/shopkeeper/accounts/accounts_invitations_controller.rb +++ b/app/controllers/api/v1/shopkeeper/accounts/accounts_invitations_controller.rb @@ -1,4 +1,10 @@ class Api::V1::Shopkeeper::Accounts::AccountsInvitationsController < Api::V1::Shopkeeper::BaseController + # Each create or resend emails an address the caller chooses + rate_limit to: 20, within: 1.hour, only: %i[create resend], + name: "invitation_mails/shopkeeper", + by: -> { current_shopkeeper.id }, + with: -> { render_error(code: 429, message: I18n.t("errors.messages.too_many_requests"), status: :too_many_requests) } + before_action :set_account before_action :set_accounts_invitation, only: %i[show update destroy resend] diff --git a/app/controllers/concerns/throttle_mail_requests.rb b/app/controllers/concerns/throttle_mail_requests.rb new file mode 100644 index 0000000..1916a39 --- /dev/null +++ b/app/controllers/concerns/throttle_mail_requests.rb @@ -0,0 +1,23 @@ +# For devise_token_auth endpoints whose create action emails the address in +# params[:email] (password reset, confirmation resend). The per-address limit +# stops a stranger's inbox from being flooded from many IPs. +module ThrottleMailRequests + extend ActiveSupport::Concern + + RENDER_MAIL_THROTTLED = -> { + render json: {code: 429, error_message: I18n.t("errors.messages.too_many_requests")}, + status: :too_many_requests + } + + included do + rate_limit to: 5, within: 1.minute, only: :create, + name: "mails/ip", + with: RENDER_MAIL_THROTTLED + + rate_limit to: 3, within: 1.hour, only: :create, + name: "mails/email", + by: -> { params[:email].to_s.downcase.gsub(/\s+/, "") }, + if: -> { params[:email].present? }, + with: RENDER_MAIL_THROTTLED + end +end diff --git a/app/controllers/shopkeeper_auth/confirmations_controller.rb b/app/controllers/shopkeeper_auth/confirmations_controller.rb index 1849d24..a12621a 100644 --- a/app/controllers/shopkeeper_auth/confirmations_controller.rb +++ b/app/controllers/shopkeeper_auth/confirmations_controller.rb @@ -1,5 +1,6 @@ class ShopkeeperAuth::ConfirmationsController < DeviseTokenAuth::ConfirmationsController include SameHostRedirect + include ThrottleMailRequests protected diff --git a/app/controllers/shopkeeper_auth/passwords_controller.rb b/app/controllers/shopkeeper_auth/passwords_controller.rb index 5d82a5a..37e2a19 100644 --- a/app/controllers/shopkeeper_auth/passwords_controller.rb +++ b/app/controllers/shopkeeper_auth/passwords_controller.rb @@ -2,6 +2,7 @@ class ShopkeeperAuth::PasswordsController < DeviseTokenAuth::PasswordsController include ActionController::MimeResponds include ActionController::Flash include SameHostRedirect + include ThrottleMailRequests protected diff --git a/app/jobs/purge_inactive_demo_shopkeepers_job.rb b/app/jobs/purge_inactive_demo_shopkeepers_job.rb new file mode 100644 index 0000000..62e909f --- /dev/null +++ b/app/jobs/purge_inactive_demo_shopkeepers_job.rb @@ -0,0 +1,20 @@ +# The public demo lets anyone sign up, so it deletes shopkeepers who have not +# signed in for DEMO_PURGE_INACTIVE_DAYS days, with their owned accounts, shops +# and item tags. The variable is set only in config/deploy.demo.yml; unset (as in +# every generated app) this job does nothing. +# +# devise_token_auth tokens expire 30 days after sign-in (token_lifespan), so with +# 30 days or more nobody still using the app is deleted. +class PurgeInactiveDemoShopkeepersJob < ApplicationJob + queue_as :default + + def perform + value = ENV["DEMO_PURGE_INACTIVE_DAYS"] + return if value.blank? + + days = Integer(value, 10) + raise ArgumentError, "DEMO_PURGE_INACTIVE_DAYS must be a positive number of days, got #{value.inspect}" unless days.positive? + + Shopkeeper.where("COALESCE(current_sign_in_at, created_at) < ?", days.days.ago).find_each(&:destroy!) + end +end diff --git a/config/recurring.yml b/config/recurring.yml index a643a4c..73cfcbb 100644 --- a/config/recurring.yml +++ b/config/recurring.yml @@ -10,3 +10,7 @@ default: &default production: <<: *default + # Does nothing unless DEMO_PURGE_INACTIVE_DAYS is set (public demo only). + purge_inactive_demo_shopkeepers: + class: PurgeInactiveDemoShopkeepersJob + schedule: every day at 4:30 diff --git a/test/config/deploy_config_test.rb b/test/config/deploy_config_test.rb index e53aaff..fdae5d8 100644 --- a/test/config/deploy_config_test.rb +++ b/test/config/deploy_config_test.rb @@ -100,6 +100,26 @@ class DeployConfigTest < ActiveSupport::TestCase # bin/setup and CI install gems into vendor/bundle for the host platform; copying # them into the image breaks `bundle install` there and bloats the image. + # The public demo deploys this repo with `bin/kamal deploy -d demo`, which + # merges config/deploy.demo.yml over deploy.yml and reads .kamal/secrets.demo + # instead of .kamal/secrets (Kamal::Secrets#secrets_filenames). The demo file + # holds the real server IP; committing it would publish the origin behind + # Cloudflare in this public repo and hand it to every generated app. + test "demo destination files stay out of git" do + %w[config/deploy.demo.yml .kamal/secrets.demo].each do |path| + assert system("git", "check-ignore", "--quiet", "--no-index", path, chdir: Rails.root.to_s), + "#{path} must be gitignored" + end + end + + # PurgeInactiveDemoShopkeepersJob deletes shopkeepers when this is set. It + # belongs in config/deploy.demo.yml only; here every generated app would + # inherit it and delete its real users. + test "the committed deploy config never enables the demo purge" do + assert_not DEPLOY.dig("env", "clear").key?("DEMO_PURGE_INACTIVE_DAYS") + assert_not_includes DEPLOY.dig("env", "secret"), "DEMO_PURGE_INACTIVE_DAYS" + end + test "Docker build context excludes vendor/bundle" do assert_includes Rails.root.join(".dockerignore").readlines.map(&:strip), "/vendor/bundle" end diff --git a/test/config/recurring_tasks_test.rb b/test/config/recurring_tasks_test.rb index cff62f4..89c70ce 100644 --- a/test/config/recurring_tasks_test.rb +++ b/test/config/recurring_tasks_test.rb @@ -18,6 +18,13 @@ class RecurringTasksTest < ActiveSupport::TestCase assert_includes SolidQueue::Job.method(:clear_finished_in_batches).parameters, [:key, :sleep_between_batches] end + test "production runs the demo purge, which is a no-op outside the demo" do + options = CONFIG.dig("production", "purge_inactive_demo_shopkeepers") + assert options, "config/recurring.yml has no purge_inactive_demo_shopkeepers task for production" + + assert_equal "PurgeInactiveDemoShopkeepersJob", options["class"] + end + test "every recurring task has a command and a supported schedule" do CONFIG.each do |env, tasks| tasks.each do |key, options| diff --git a/test/integration/mail_throttle_test.rb b/test/integration/mail_throttle_test.rb new file mode 100644 index 0000000..e147f89 --- /dev/null +++ b/test/integration/mail_throttle_test.rb @@ -0,0 +1,97 @@ +require "test_helper" + +# Endpoints that email an address the caller names. Without these limits anyone +# could make the app mail a stranger over and over, and the sending domain's +# reputation would pay for it. Each limit is the one declared on the controller. +class MailThrottleTest < ActionDispatch::IntegrationTest + setup do + @shopkeeper = shopkeepers(:one) + @shopkeeper.create_default_account + @account = @shopkeeper.accounts.first + end + + def post_password_reset(email, ip: nil) + post shopkeeper_password_url, + params: {email: email, redirect_url: "http://www.example.com/reset"}, + headers: ip ? {"REMOTE_ADDR" => ip} : {}, + as: :json + end + + def post_confirmation(email, ip: nil) + post shopkeeper_confirmation_url, + params: {email: email, redirect_url: "http://www.example.com/confirm"}, + headers: ip ? {"REMOTE_ADDR" => ip} : {}, + as: :json + end + + def assert_throttled + assert_response :too_many_requests + assert_equal 429, response.parsed_body["code"] + assert_equal I18n.t("errors.messages.too_many_requests"), response.parsed_body["error_message"] + end + + # PasswordsController: 5 per minute per IP + test "the sixth password reset from the same IP within a minute is rate-limited" do + 5.times do |i| + post_password_reset("noone#{i}@example.com") + assert_not_equal 429, response.status, "request #{i + 1} should not be throttled" + end + + assert_no_enqueued_emails { post_password_reset(@shopkeeper.email) } + assert_throttled + end + + # PasswordsController: 3 per hour per address, whatever the IP + test "the fourth password reset for one address within an hour is rate-limited" do + 3.times do |i| + post_password_reset(@shopkeeper.email, ip: "10.0.0.#{i + 1}") + assert_not_equal 429, response.status, "request #{i + 1} should not be throttled" + end + + assert_no_enqueued_emails { post_password_reset(" #{@shopkeeper.email.upcase} ", ip: "10.0.0.99") } + assert_throttled + end + + # ConfirmationsController: 5 per minute per IP + test "the sixth confirmation resend from the same IP within a minute is rate-limited" do + 5.times do |i| + post_confirmation("noone#{i}@example.com") + assert_not_equal 429, response.status, "request #{i + 1} should not be throttled" + end + + assert_no_enqueued_emails { post_confirmation(@shopkeeper.email) } + assert_throttled + end + + # ConfirmationsController: 3 per hour per address, whatever the IP + test "the fourth confirmation resend for one address within an hour is rate-limited" do + 3.times do |i| + post_confirmation(@shopkeeper.email, ip: "10.0.0.#{i + 1}") + assert_not_equal 429, response.status, "request #{i + 1} should not be throttled" + end + + assert_no_enqueued_emails { post_confirmation(@shopkeeper.email.upcase, ip: "10.0.0.99") } + assert_throttled + end + + # Accounts::AccountsInvitationsController: 20 sends (create + resend) per hour per shopkeeper + test "the twenty-first invitation email from one shopkeeper within an hour is rate-limited" do + headers = @shopkeeper.create_new_auth_token + + 19.times do |i| + post api_v1_shopkeeper_account_accounts_invitations_url(@account), + params: {accounts_invitation: {name: "Invitee #{i}", email: "invitee#{i}@example.com", member: true}}, + headers: headers + assert_response :created + end + + invitation = @account.accounts_invitations.first + post resend_api_v1_shopkeeper_account_accounts_invitation_path(@account, invitation.token), headers: headers + assert_response :success + + assert_no_enqueued_emails do + post resend_api_v1_shopkeeper_account_accounts_invitation_path(@account, invitation.token), headers: headers + end + assert_throttled + end +end diff --git a/test/jobs/purge_inactive_demo_shopkeepers_job_test.rb b/test/jobs/purge_inactive_demo_shopkeepers_job_test.rb new file mode 100644 index 0000000..d6cf1b2 --- /dev/null +++ b/test/jobs/purge_inactive_demo_shopkeepers_job_test.rb @@ -0,0 +1,54 @@ +require "test_helper" + +class PurgeInactiveDemoShopkeepersJobTest < ActiveJob::TestCase + def build_shopkeeper(email, created_at:, current_sign_in_at: nil) + shopkeeper = Shopkeeper.create!(name: "Demo", email: email, password: "password", current_platform: "ios") + shopkeeper.update_columns(created_at: created_at, current_sign_in_at: current_sign_in_at) + shopkeeper + end + + def with_env(value) + original = ENV["DEMO_PURGE_INACTIVE_DAYS"] + ENV["DEMO_PURGE_INACTIVE_DAYS"] = value + yield + ensure + ENV["DEMO_PURGE_INACTIVE_DAYS"] = original + end + + # Generated apps inherit this job and the recurring task. Without the + # variable it must never delete anyone. + test "does nothing when DEMO_PURGE_INACTIVE_DAYS is unset" do + build_shopkeeper("old@example.com", created_at: 1.year.ago, current_sign_in_at: 1.year.ago) + + with_env(nil) do + assert_no_difference("Shopkeeper.count") { PurgeInactiveDemoShopkeepersJob.perform_now } + end + end + + test "deletes shopkeepers whose last sign-in, or sign-up if they never signed in, is older than the limit" do + signed_in_long_ago = build_shopkeeper("a@example.com", created_at: 60.days.ago, current_sign_in_at: 31.days.ago) + never_signed_in = build_shopkeeper("b@example.com", created_at: 31.days.ago) + signed_in_recently = build_shopkeeper("c@example.com", created_at: 60.days.ago, current_sign_in_at: 29.days.ago) + signed_up_recently = build_shopkeeper("d@example.com", created_at: 29.days.ago) + owned_account_ids = signed_in_long_ago.owned_accounts.ids + + with_env("30") { PurgeInactiveDemoShopkeepersJob.perform_now } + + assert_not Shopkeeper.exists?(signed_in_long_ago.id) + assert_not Shopkeeper.exists?(never_signed_in.id) + assert Shopkeeper.exists?(signed_in_recently.id) + assert Shopkeeper.exists?(signed_up_recently.id) + assert_not_empty owned_account_ids + assert_not Account.where(id: owned_account_ids).exists? + end + + test "rejects a value that is not a positive whole number of days" do + build_shopkeeper("old@example.com", created_at: 1.year.ago) + + %w[abc 0 -5].each do |value| + with_env(value) do + assert_raises(ArgumentError) { PurgeInactiveDemoShopkeepersJob.perform_now } + end + end + end +end