Validate time zones against the Rails time zone list

AGENTS.md

@@ -110,7 +110,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` (614 tests, 1594 assertions)
+- Run tests: `bin/rails test` (624 tests, 1934 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.

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## [Unreleased]
 
+- API change: `Shop#time_zone` and `Shopkeeper#time_zone` must be a Rails time zone name (`ActiveSupport::TimeZone.all.map(&:name)`, e.g. `Tokyo`, `London`); anything else, IANA ids such as `Asia/Tokyo` included, gets 422 "Time zone is not included in the list" from `PATCH /shops/:id`, `POST /shops`, sign-up and `PATCH /shopkeeper_auth` instead of being stored. `test/config/time_zones_test.rb` pins the list against `test/fixtures/files/rails_time_zones.tsv`, which the iOS and Android apps embed for their time zone pickers (#155)
 - Add optional Sentry error monitoring (`sentry-ruby` / `sentry-rails` 7.1). Off unless `SENTRY_DSN` is set (a Kamal secret read from `NATIVEAPPTEMPLATEAPI_SENTRY_DSN`, optional); sends only from production, only unhandled errors (no 4xx, no tracing, profiling or breadcrumbs), with the shopkeeper's ID as the only user data, the account ID as a tag, devise_token_auth headers dropped, email addresses masked and `filter_parameters` applied. The README's "Error monitoring (Sentry)" covers turning it on, what is sent and the recommended Sentry privacy settings. New tests: `test/config/sentry_test.rb`, `test/integration/sentry_context_test.rb` (#154)
 - API change: tell the apps when the organization in the URL is gone. An `/<account uuid>/api/...` request for a deleted account now gets `404 {"code":404,"error_code":"account_not_found","error_message":...}` JSON (was a `302` to `/`), and a signed-in shopkeeper who is no longer a member gets `403 {"code":403,"error_code":"not_account_member","error_message":...}` (was Pundit's generic 401, which the apps treated as a sign-out). Messages are in English and Japanese and follow the usual locale rules. Pundit denials inside an organization the shopkeeper belongs to stay 401; non-API paths still redirect (#153)
 - Call a shop 店舗 rather than ショップ in the Japanese messages (model name, item tag's shop attribute, the shop limit error, the sample shop description), matching 店舗名

app/models/shop.rb

@@ -8,6 +8,12 @@ class Shop < ApplicationRecord
   has_many :item_tags, dependent: :destroy
 
   validates :name, presence: true
+  # Rails time zone names only (the apps send ActiveSupport::TimeZone#name);
+  # IANA ids such as "Asia/Tokyo" are rejected. Checked only on change so a
+  # legacy value does not block unrelated updates
+  validates :time_zone,
+    inclusion: {in: ->(_) { ActiveSupport::TimeZone.all.map(&:name) }},
+    if: :will_save_change_to_time_zone?
   validate :limit_count, on: :create
 
   after_create :create_sample_item_tag

app/models/shopkeeper.rb

@@ -31,6 +31,13 @@ class Shopkeeper < ApplicationRecord
 
   validates :locale, inclusion: {in: ->(_) { I18n.available_locales.map(&:to_s) }}
 
+  # Rails time zone names only (the apps send ActiveSupport::TimeZone#name);
+  # IANA ids such as "Asia/Tokyo" are rejected. Checked only on change so a
+  # legacy value does not block unrelated updates
+  validates :time_zone,
+    inclusion: {in: ->(_) { ActiveSupport::TimeZone.all.map(&:name) }},
+    if: :will_save_change_to_time_zone?
+
   # override devise method to include additional info as opts hash
   def send_confirmation_instructions(opts = {})
     generate_confirmation_token! unless @raw_confirmation_token

test/config/time_zones_test.rb

@@ -0,0 +1,19 @@
+require "test_helper"
+
+# Shop#time_zone and Shopkeeper#time_zone accept only the names in
+# ActiveSupport::TimeZone.all. The iOS app (TimeZoneData.swift, checked by
+# TimeZoneDataTest) and the Android app (TimeZones.kt, checked by
+# TimeZonesTest) embed this same list of names and labels for their time zone
+# pickers, so a change here makes the apps send names the API rejects or miss
+# new ones. When this test fails after a Rails or tzdata upgrade, update
+# test/fixtures/files/rails_time_zones.tsv and both apps.
+class TimeZonesTest < ActiveSupport::TestCase
+  test "Rails time zones match the list the apps embed" do
+    # name<TAB>label per line, generated from activesupport 8.1.4 with
+    # ActiveSupport::TimeZone.all.map { |z| [z.name, z.to_s].join("\t") }
+    expected = file_fixture("rails_time_zones.tsv").readlines(chomp: true).map { |line| line.split("\t") }
+
+    assert_equal 152, expected.size
+    assert_equal expected, ActiveSupport::TimeZone.all.map { |zone| [zone.name, zone.to_s] }
+  end
+end

test/controllers/api/v1/shopkeeper/shops_controller_test.rb

@@ -84,6 +84,17 @@ class Api::V1::Shopkeeper::ShopsControllerTest < ActionDispatch::IntegrationTest
     assert_equal ["Open late", "Osaka"], [@shop.description, @shop.time_zone]
   end
 
+  test "update rejects an unknown time zone with 422" do
+    patch api_v1_shopkeeper_shop_url(@shop),
+      params: {shop: {time_zone: "Bogus"}},
+      headers: @shopkeeper.create_new_auth_token
+
+    assert_response :unprocessable_entity
+    assert_equal 422, response.parsed_body["code"]
+    assert_equal "Time zone is not included in the list", response.parsed_body["error_message"]
+    assert_equal "Tokyo", @shop.reload.time_zone
+  end
+
   test "update returns validation error with blank name" do
     patch api_v1_shopkeeper_shop_url(@shop),
       params: {shop: {name: ""}},

test/controllers/shopkeeper_auth/registrations_controller_test.rb

@@ -33,6 +33,25 @@ class ShopkeeperAuth::RegistrationsControllerTest < ActionDispatch::IntegrationT
     assert_equal "Osaka", shopkeeper.reload.time_zone
   end
 
+  test "rejects an unknown time zone on update with 422" do
+    shopkeeper.create_default_account
+    patch shopkeeper_registration_url, params: {time_zone: "Bogus"}, headers: shopkeeper.create_new_auth_token
+
+    assert_response :unprocessable_entity
+    assert_equal 422, response.parsed_body["code"]
+    assert_equal "Time zone is not included in the list", response.parsed_body["error_message"]
+    assert_equal "Tokyo", shopkeeper.reload.time_zone
+  end
+
+  test "rejects an unknown time zone on sign up with 422" do
+    assert_no_difference ["Shopkeeper.count", "Account.count", "Shop.count"] do
+      post shopkeeper_registration_url, params: {email: "bogus-zone@example.com", name: "Bogus Zone", password: "password", time_zone: "Bogus", current_platform: "ios"}
+    end
+
+    assert_response :unprocessable_entity
+    assert_equal "Time zone is not included in the list", response.parsed_body["error_message"]
+  end
+
   test "delete current shopkeeper" do
     assert_difference "Shopkeeper.count", -1 do
       delete shopkeeper_registration_url, headers: shopkeeper.create_new_auth_token

test/fixtures/files/rails_time_zones.tsv

@@ -0,0 +1,152 @@
+International Date Line West	(GMT-12:00) International Date Line West
+American Samoa	(GMT-11:00) American Samoa
+Midway Island	(GMT-11:00) Midway Island
+Hawaii	(GMT-10:00) Hawaii
+Alaska	(GMT-09:00) Alaska
+Pacific Time (US & Canada)	(GMT-08:00) Pacific Time (US & Canada)
+Tijuana	(GMT-08:00) Tijuana
+Arizona	(GMT-07:00) Arizona
+Mazatlan	(GMT-07:00) Mazatlan
+Mountain Time (US & Canada)	(GMT-07:00) Mountain Time (US & Canada)
+Central America	(GMT-06:00) Central America
+Central Time (US & Canada)	(GMT-06:00) Central Time (US & Canada)
+Chihuahua	(GMT-06:00) Chihuahua
+Guadalajara	(GMT-06:00) Guadalajara
+Mexico City	(GMT-06:00) Mexico City
+Monterrey	(GMT-06:00) Monterrey
+Saskatchewan	(GMT-06:00) Saskatchewan
+Bogota	(GMT-05:00) Bogota
+Eastern Time (US & Canada)	(GMT-05:00) Eastern Time (US & Canada)
+Indiana (East)	(GMT-05:00) Indiana (East)
+Lima	(GMT-05:00) Lima
+Quito	(GMT-05:00) Quito
+Atlantic Time (Canada)	(GMT-04:00) Atlantic Time (Canada)
+Caracas	(GMT-04:00) Caracas
+Georgetown	(GMT-04:00) Georgetown
+La Paz	(GMT-04:00) La Paz
+Puerto Rico	(GMT-04:00) Puerto Rico
+Santiago	(GMT-04:00) Santiago
+Newfoundland	(GMT-03:30) Newfoundland
+Asuncion	(GMT-03:00) Asuncion
+Brasilia	(GMT-03:00) Brasilia
+Buenos Aires	(GMT-03:00) Buenos Aires
+Montevideo	(GMT-03:00) Montevideo
+Greenland	(GMT-02:00) Greenland
+Mid-Atlantic	(GMT-02:00) Mid-Atlantic
+Azores	(GMT-01:00) Azores
+Cape Verde Is.	(GMT-01:00) Cape Verde Is.
+Casablanca	(GMT+00:00) Casablanca
+Dublin	(GMT+00:00) Dublin
+Edinburgh	(GMT+00:00) Edinburgh
+Lisbon	(GMT+00:00) Lisbon
+London	(GMT+00:00) London
+Monrovia	(GMT+00:00) Monrovia
+UTC	(GMT+00:00) UTC
+Amsterdam	(GMT+01:00) Amsterdam
+Belgrade	(GMT+01:00) Belgrade
+Berlin	(GMT+01:00) Berlin
+Bern	(GMT+01:00) Bern
+Bratislava	(GMT+01:00) Bratislava
+Brussels	(GMT+01:00) Brussels
+Budapest	(GMT+01:00) Budapest
+Copenhagen	(GMT+01:00) Copenhagen
+Ljubljana	(GMT+01:00) Ljubljana
+Madrid	(GMT+01:00) Madrid
+Paris	(GMT+01:00) Paris
+Prague	(GMT+01:00) Prague
+Rome	(GMT+01:00) Rome
+Sarajevo	(GMT+01:00) Sarajevo
+Skopje	(GMT+01:00) Skopje
+Stockholm	(GMT+01:00) Stockholm
+Vienna	(GMT+01:00) Vienna
+Warsaw	(GMT+01:00) Warsaw
+West Central Africa	(GMT+01:00) West Central Africa
+Zagreb	(GMT+01:00) Zagreb
+Zurich	(GMT+01:00) Zurich
+Athens	(GMT+02:00) Athens
+Bucharest	(GMT+02:00) Bucharest
+Cairo	(GMT+02:00) Cairo
+Harare	(GMT+02:00) Harare
+Helsinki	(GMT+02:00) Helsinki
+Jerusalem	(GMT+02:00) Jerusalem
+Kaliningrad	(GMT+02:00) Kaliningrad
+Kyiv	(GMT+02:00) Kyiv
+Pretoria	(GMT+02:00) Pretoria
+Riga	(GMT+02:00) Riga
+Sofia	(GMT+02:00) Sofia
+Tallinn	(GMT+02:00) Tallinn
+Vilnius	(GMT+02:00) Vilnius
+Baghdad	(GMT+03:00) Baghdad
+Istanbul	(GMT+03:00) Istanbul
+Kuwait	(GMT+03:00) Kuwait
+Minsk	(GMT+03:00) Minsk
+Moscow	(GMT+03:00) Moscow
+Nairobi	(GMT+03:00) Nairobi
+Riyadh	(GMT+03:00) Riyadh
+St. Petersburg	(GMT+03:00) St. Petersburg
+Volgograd	(GMT+03:00) Volgograd
+Tehran	(GMT+03:30) Tehran
+Abu Dhabi	(GMT+04:00) Abu Dhabi
+Baku	(GMT+04:00) Baku
+Muscat	(GMT+04:00) Muscat
+Samara	(GMT+04:00) Samara
+Tbilisi	(GMT+04:00) Tbilisi
+Yerevan	(GMT+04:00) Yerevan
+Kabul	(GMT+04:30) Kabul
+Almaty	(GMT+05:00) Almaty
+Astana	(GMT+05:00) Astana
+Ekaterinburg	(GMT+05:00) Ekaterinburg
+Islamabad	(GMT+05:00) Islamabad
+Karachi	(GMT+05:00) Karachi
+Tashkent	(GMT+05:00) Tashkent
+Chennai	(GMT+05:30) Chennai
+Kolkata	(GMT+05:30) Kolkata
+Mumbai	(GMT+05:30) Mumbai
+New Delhi	(GMT+05:30) New Delhi
+Sri Jayawardenepura	(GMT+05:30) Sri Jayawardenepura
+Kathmandu	(GMT+05:45) Kathmandu
+Dhaka	(GMT+06:00) Dhaka
+Urumqi	(GMT+06:00) Urumqi
+Rangoon	(GMT+06:30) Rangoon
+Bangkok	(GMT+07:00) Bangkok
+Hanoi	(GMT+07:00) Hanoi
+Jakarta	(GMT+07:00) Jakarta
+Krasnoyarsk	(GMT+07:00) Krasnoyarsk
+Novosibirsk	(GMT+07:00) Novosibirsk
+Beijing	(GMT+08:00) Beijing
+Chongqing	(GMT+08:00) Chongqing
+Hong Kong	(GMT+08:00) Hong Kong
+Irkutsk	(GMT+08:00) Irkutsk
+Kuala Lumpur	(GMT+08:00) Kuala Lumpur
+Perth	(GMT+08:00) Perth
+Singapore	(GMT+08:00) Singapore
+Taipei	(GMT+08:00) Taipei
+Ulaanbaatar	(GMT+08:00) Ulaanbaatar
+Osaka	(GMT+09:00) Osaka
+Sapporo	(GMT+09:00) Sapporo
+Seoul	(GMT+09:00) Seoul
+Tokyo	(GMT+09:00) Tokyo
+Yakutsk	(GMT+09:00) Yakutsk
+Adelaide	(GMT+09:30) Adelaide
+Darwin	(GMT+09:30) Darwin
+Brisbane	(GMT+10:00) Brisbane
+Canberra	(GMT+10:00) Canberra
+Guam	(GMT+10:00) Guam
+Hobart	(GMT+10:00) Hobart
+Melbourne	(GMT+10:00) Melbourne
+Port Moresby	(GMT+10:00) Port Moresby
+Sydney	(GMT+10:00) Sydney
+Vladivostok	(GMT+10:00) Vladivostok
+Magadan	(GMT+11:00) Magadan
+New Caledonia	(GMT+11:00) New Caledonia
+Solomon Is.	(GMT+11:00) Solomon Is.
+Srednekolymsk	(GMT+11:00) Srednekolymsk
+Auckland	(GMT+12:00) Auckland
+Fiji	(GMT+12:00) Fiji
+Kamchatka	(GMT+12:00) Kamchatka
+Marshall Is.	(GMT+12:00) Marshall Is.
+Wellington	(GMT+12:00) Wellington
+Chatham Is.	(GMT+12:45) Chatham Is.
+Nuku'alofa	(GMT+13:00) Nuku'alofa
+Samoa	(GMT+13:00) Samoa
+Tokelau Is.	(GMT+13:00) Tokelau Is.

test/models/shop_test.rb

@@ -69,4 +69,32 @@ def setup
       end
     end
   end
+
+  # The apps send Rails time zone names (ActiveSupport::TimeZone#name)
+  test "should accept every Rails time zone name" do
+    shop = @account.shops.new(name: "Test Shop", created_by: @shopkeeper)
+    ActiveSupport::TimeZone.all.map(&:name).each do |time_zone|
+      shop.time_zone = time_zone
+      assert shop.valid?, time_zone
+    end
+  end
+
+  # ActiveSupport::TimeZone["Asia/Tokyo"] finds a zone, but "Asia/Tokyo" is an
+  # IANA id, not one of the Rails names the apps send and list, so reject it
+  test "should reject a time zone that is not a Rails time zone name" do
+    shop = @account.shops.new(name: "Test Shop", created_by: @shopkeeper)
+    ["Bogus", "", "Asia/Tokyo"].each do |time_zone|
+      shop.time_zone = time_zone
+      assert_not shop.valid?, time_zone
+      assert_includes shop.errors[:time_zone], "is not included in the list"
+    end
+  end
+
+  # A stored zone that is not a Rails name must not block unrelated updates
+  test "should not revalidate an unchanged time zone" do
+    shop = @account.shops.create!(name: "Test Shop", created_by: @shopkeeper)
+    shop.update_column(:time_zone, "Asia/Tokyo")
+    shop.name = "Renamed"
+    assert shop.valid?
+  end
 end

test/models/shopkeeper_test.rb

@@ -85,6 +85,26 @@ class ShopkeeperTest < ActiveSupport::TestCase
     end
   end
 
+  # The apps send Rails time zone names (ActiveSupport::TimeZone#name)
+  test "should accept every Rails time zone name" do
+    shopkeeper = shopkeepers(:one)
+    ActiveSupport::TimeZone.all.map(&:name).each do |time_zone|
+      shopkeeper.time_zone = time_zone
+      assert shopkeeper.valid?, time_zone
+    end
+  end
+
+  # ActiveSupport::TimeZone["Asia/Tokyo"] finds a zone, but "Asia/Tokyo" is an
+  # IANA id, not one of the Rails names the apps send and list, so reject it
+  test "should reject a time zone that is not a Rails time zone name" do
+    shopkeeper = shopkeepers(:one)
+    ["Bogus", "", "Asia/Tokyo"].each do |time_zone|
+      shopkeeper.time_zone = time_zone
+      assert_not shopkeeper.valid?, time_zone
+      assert_includes shopkeeper.errors[:time_zone], "is not included in the list"
+    end
+  end
+
   test "should have many shops through accounts" do
     shopkeeper = shopkeepers(:one)
     shopkeeper.create_default_account
@@ -287,4 +307,12 @@ class ShopkeeperTest < ActiveSupport::TestCase
       assert_nil item_tag.created_by_id
     end
   end
+
+  # A stored zone that is not a Rails name must not block unrelated updates
+  test "should not revalidate an unchanged time zone" do
+    shopkeeper = shopkeepers(:one)
+    shopkeeper.update_column(:time_zone, "Asia/Tokyo")
+    shopkeeper.name = "Renamed"
+    assert shopkeeper.valid?
+  end
 end