Skip to content

Commit 1399dcb

Browse files
dadachiclaude
andauthored
Fix SessionsController nuking current_platform on header-less sign-in (#49)
* Fix two latent bugs in permissions and sign-in flows PermissionsController#index compared `confirmed_*_version < current_*_version`, where `current_version` returns nil if no row is flagged current. `string < nil` raises ArgumentError → 500. Add a `version_outdated?` helper that treats a missing current version as "nothing to update". ShopkeeperAuth::SessionsController#create unconditionally assigned `request.headers["source"]` to current_platform and called `save!(validate: false)`. Sign-ins without the source header overwrote the user's stored platform with nil, bypassing the presence/inclusion validation. Now skip the assignment when the header is blank. Adds regression tests for both paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Revert PermissionsController nil-version guard Per review: a missing current PrivacyVersion/TermsVersion is a server-side data integrity problem (no row published as current). Crashing loud is preferable to silently telling the client "you're up to date" — the latter would mask the data issue and mislead clients. Keeps the SessionsController fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Require 'source' header on shopkeeper sign-in Per direction: the source header (ios/android) is mandatory. Reject the request with 401 when missing instead of skipping the current_platform update — silently signing the user in without the header would let API tools accumulate sessions that lack platform attribution and bypass the presence/inclusion validation on Shopkeeper. Adds the locale key and updates the regression test for the blank-header path. Updates the existing "no params" test to send the header so the bad_credentials assertion remains the path under test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Make 'source' header optional on shopkeeper sign-in Reverts the earlier "require source header" form. Anti-mass-signup is now handled at the right layer by the sign-up rate_limit introduced in PR #50, so the sign-in header has no security job left. current_platform is informational metadata; rejecting sign-ins on missing metadata is too aggressive — it breaks non-mobile callers (curl, CI, integration tools, future web client) without a real benefit. Skip the current_platform update when the header is blank: the existing stored value is preserved (instead of being nuked to nil by the original buggy code path). Drop the missing_source locale key. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent e5e8d33 commit 1399dcb

2 files changed

Lines changed: 29 additions & 1 deletion

File tree

app/controllers/shopkeeper_auth/sessions_controller.rb

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@ def create
33
super
44
return if @resource.blank?
55

6-
@resource.current_platform = request.headers["source"]
6+
source = request.headers["source"]
7+
return if source.blank?
8+
9+
@resource.current_platform = source
710
@resource.save!(validate: false)
811
end
912

test/controllers/shopkeeper_auth/sessions_controller_test.rb

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,29 @@ class ShopkeeperAuth::SessionsControllerTest < ActionDispatch::IntegrationTest
2828
delete destroy_shopkeeper_session_url, headers: shopkeeper.create_new_auth_token
2929
assert_response :success
3030
end
31+
32+
test "successful sign-in updates current_platform to the source header value" do
33+
shopkeeper = shopkeepers(:one)
34+
shopkeeper.create_default_account
35+
shopkeeper.update_column(:current_platform, "ios")
36+
37+
post shopkeeper_session_url,
38+
params: {email: shopkeeper.email, password: "password"},
39+
headers: {source: "android"}
40+
41+
assert_response :success
42+
assert_equal "android", shopkeeper.reload.current_platform
43+
end
44+
45+
test "successful sign-in without source header preserves the existing current_platform" do
46+
shopkeeper = shopkeepers(:one)
47+
shopkeeper.create_default_account
48+
shopkeeper.update_column(:current_platform, "ios")
49+
50+
post shopkeeper_session_url,
51+
params: {email: shopkeeper.email, password: "password"}
52+
53+
assert_response :success
54+
assert_equal "ios", shopkeeper.reload.current_platform
55+
end
3156
end

0 commit comments

Comments
 (0)