From 2b63e61475597ee728e5f3674344b51e87856d39 Mon Sep 17 00:00:00 2001 From: Malaber Date: Thu, 30 Apr 2026 09:32:37 +0200 Subject: [PATCH] fix(sync): protect follower sync tokens Store read-only receiver tokens as keyed digests and expose only sync_auth_configured in owner responses. Keep leader push tokens write-only in API responses because outbound sync still needs to send them. Refs TUR-84 --- app/controllers/tournaments_controller.rb | 4 +- app/models/tournament.rb | 43 ++++++++++++++++-- app/serializers/tournament_serializer.rb | 5 +++ app/services/tournament_sync_schema.rb | 1 + ...d_sync_auth_token_digest_to_tournaments.rb | 36 +++++++++++++++ db/schema.rb | 5 ++- doc/leader_follower.md | 7 +-- .../tournaments_controller_spec.rb | 15 +++++++ spec/e2e/http/tournament_sync_test_spec.rb | 45 +++++++++++++++++++ spec/models/tournament_spec.rb | 34 ++++++++++++++ 10 files changed, 183 insertions(+), 12 deletions(-) create mode 100644 db/migrate/20260430120000_add_sync_auth_token_digest_to_tournaments.rb diff --git a/app/controllers/tournaments_controller.rb b/app/controllers/tournaments_controller.rb index 11cf0f2..6c08577 100644 --- a/app/controllers/tournaments_controller.rb +++ b/app/controllers/tournaments_controller.rb @@ -319,9 +319,7 @@ class TournamentsController < ApplicationController token = request.authorization.to_s.delete_prefix('Bearer ').presence || request.headers['X-Tournament-Sync-Token'].to_s return render json: { error: 'Missing sync token' }, status: :unauthorized if token.blank? - matches = token.bytesize == @tournament.sync_auth_token.to_s.bytesize && - ActiveSupport::SecurityUtils.secure_compare(token, @tournament.sync_auth_token.to_s) - return if matches + return if @tournament.sync_token_matches?(token) render json: { error: 'Invalid sync token' }, status: :unauthorized end diff --git a/app/models/tournament.rb b/app/models/tournament.rb index 26653ee..0daf7ae 100644 --- a/app/models/tournament.rb +++ b/app/models/tournament.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'securerandom' +require 'openssl' class Tournament < ApplicationRecord TIMER_MODES = %w[countdown countup].freeze @@ -26,6 +27,7 @@ class Tournament < ApplicationRecord after_initialize :generate_code after_create_commit :ensure_default_beamer! before_validation :normalize_timer_reason + before_validation :protect_follower_sync_token before_validation :clear_follower_sync_token_when_disabling_read_only_mode after_commit :broadcast_timer_state_change, if: :saved_change_to_timer_state? @@ -50,7 +52,27 @@ class Tournament < ApplicationRecord end def sync_accepts_push? - read_only_mode? && sync_auth_token.present? + read_only_mode? && sync_auth_configured? + end + + def sync_auth_configured? + sync_auth_token.present? || sync_auth_token_digest.present? + end + + def sync_token_matches?(token) + return false if token.blank? + + if sync_auth_token_digest.present? + expected_digest = self.class.sync_auth_token_digest(token) + return ActiveSupport::SecurityUtils.secure_compare(expected_digest, sync_auth_token_digest) + end + + token.bytesize == sync_auth_token.to_s.bytesize && + ActiveSupport::SecurityUtils.secure_compare(token, sync_auth_token.to_s) + end + + def self.sync_auth_token_digest(token) + OpenSSL::HMAC.hexdigest('SHA256', Rails.application.secret_key_base, token.to_s) end private @@ -80,15 +102,15 @@ class Tournament < ApplicationRecord end def sync_configuration_blank? - sync_target_url.blank? && sync_auth_token.blank? + sync_target_url.blank? && !sync_auth_configured? end def sync_configuration_complete? - sync_target_url.present? && sync_auth_token.present? + sync_target_url.present? && sync_auth_configured? end def follower_sync_token_only? - read_only_mode? && sync_auth_token.present? && sync_target_url.blank? + read_only_mode? && sync_auth_configured? && sync_target_url.blank? end def ensure_default_beamer! @@ -100,11 +122,24 @@ class Tournament < ApplicationRecord def clear_follower_sync_token_when_disabling_read_only_mode return if read_only_mode? + + self.sync_auth_token_digest = nil return if sync_target_url.present? self.sync_auth_token = nil end + def protect_follower_sync_token + return unless read_only_mode? + + if sync_auth_token.present? + self.sync_auth_token_digest = self.class.sync_auth_token_digest(sync_auth_token) + self.sync_auth_token = nil + elsif will_save_change_to_sync_auth_token? && sync_auth_token.blank? + self.sync_auth_token_digest = nil + end + end + def normalize_timer_reason self.timer_reason = timer_reason.presence self.timer_reason_text = timer_reason_text&.strip&.presence diff --git a/app/serializers/tournament_serializer.rb b/app/serializers/tournament_serializer.rb index 2691690..8777d91 100644 --- a/app/serializers/tournament_serializer.rb +++ b/app/serializers/tournament_serializer.rb @@ -5,6 +5,7 @@ class TournamentSerializer < SimpleTournamentSerializer :instant_finalists_amount, :intermediate_round_participants_amount attribute :read_only_mode, if: :sync_metadata_visible? attribute :sync_target_url, if: :sync_metadata_visible? + attribute :sync_auth_configured, if: :sync_metadata_visible? attribute :sync_last_push_error, if: :sync_metadata_visible? # NEVER expose sync_auth_token anywhere - it should only ever be written to or checked against @@ -41,4 +42,8 @@ class TournamentSerializer < SimpleTournamentSerializer def sync_metadata_visible? scope.present? && scope == object.owner end + + def sync_auth_configured + object.sync_auth_configured? + end end diff --git a/app/services/tournament_sync_schema.rb b/app/services/tournament_sync_schema.rb index d698bf5..ee0a9ac 100644 --- a/app/services/tournament_sync_schema.rb +++ b/app/services/tournament_sync_schema.rb @@ -24,6 +24,7 @@ class TournamentSyncSchema read_only_mode sync_target_url sync_auth_token + sync_auth_token_digest sync_source_tournament_id sync_last_pushed_at sync_last_push_error diff --git a/db/migrate/20260430120000_add_sync_auth_token_digest_to_tournaments.rb b/db/migrate/20260430120000_add_sync_auth_token_digest_to_tournaments.rb new file mode 100644 index 0000000..9e514f6 --- /dev/null +++ b/db/migrate/20260430120000_add_sync_auth_token_digest_to_tournaments.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'openssl' + +class AddSyncAuthTokenDigestToTournaments < ActiveRecord::Migration[7.0] + class MigrationTournament < ActiveRecord::Base + self.table_name = 'tournaments' + end + + def up + unless column_exists?(:tournaments, :sync_auth_token_digest) + add_column :tournaments, :sync_auth_token_digest, :string + end + + MigrationTournament.reset_column_information + MigrationTournament + .where(read_only_mode: true) + .where.not(sync_auth_token: [nil, '']) + .find_each do |tournament| + tournament.update!( + sync_auth_token: nil, + sync_auth_token_digest: sync_auth_token_digest(tournament.sync_auth_token) + ) + end + end + + def down + remove_column :tournaments, :sync_auth_token_digest if column_exists?(:tournaments, :sync_auth_token_digest) + end + + private + + def sync_auth_token_digest(token) + OpenSSL::HMAC.hexdigest('SHA256', Rails.application.secret_key_base, token.to_s) + end +end diff --git a/db/schema.rb b/db/schema.rb index 85ae707..262dbe1 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_04_28_120000) do +ActiveRecord::Schema[8.1].define(version: 2026_04_30_120000) do create_table "beamers", force: :cascade do |t| t.json "config", default: {}, null: false t.datetime "created_at", null: false @@ -158,15 +158,16 @@ ActiveRecord::Schema[8.1].define(version: 2026_04_28_120000) do t.boolean "public", default: true t.boolean "read_only_mode", default: false, null: false t.string "sync_auth_token" + t.string "sync_auth_token_digest" t.datetime "sync_last_imported_snapshot_at" t.string "sync_last_push_error" t.datetime "sync_last_pushed_at" t.integer "sync_source_tournament_id" t.string "sync_target_url" t.string "timer_mode" - t.datetime "timestamp" t.string "timer_reason" t.text "timer_reason_text" + t.datetime "timestamp" t.datetime "updated_at", precision: nil, null: false t.integer "user_id", null: false t.index ["code"], name: "index_tournaments_on_code", unique: true diff --git a/doc/leader_follower.md b/doc/leader_follower.md index 632e805..cdc9cb0 100644 --- a/doc/leader_follower.md +++ b/doc/leader_follower.md @@ -24,6 +24,7 @@ Create empty tournament with: - `sync_auth_token: ` Do not send `teams` payload for follower. Backend allows empty read only follower tournament creation. +The token is write-only. Backend responses never return it; owner responses only expose `sync_auth_configured`. ### 2. Create normal leader tournament on local backend @@ -178,9 +179,9 @@ Owner-facing frontend can expose: Recommended owner UX: -1. create remote follower first -2. copy remote `sync_auth_token` -3. paste remote `sync_state` URL into leader +1. generate a shared token outside the backend +2. create remote follower with that `sync_auth_token` +3. paste same token and remote `sync_state` URL into leader 4. save leader sync config 5. show last push time / error state 6. allow explicit follower takeover by disabling `read_only_mode` diff --git a/spec/controllers/tournaments_controller_spec.rb b/spec/controllers/tournaments_controller_spec.rb index 89c32e5..4f396b3 100644 --- a/spec/controllers/tournaments_controller_spec.rb +++ b/spec/controllers/tournaments_controller_spec.rb @@ -150,9 +150,11 @@ RSpec.describe TournamentsController, type: :controller do json = deserialize_response(response) expect(json[:read_only_mode]).to eq(true) expect(json[:sync_target_url]).to eq('https://remote.example.com/tournaments/1/sync_state') + expect(json[:sync_auth_configured]).to eq(true) expect(json[:sync_last_pushed_at]).to eq(pushed_at.iso8601) expect(json[:sync_last_push_error]).to eq('push failed') expect(json).not_to have_key(:sync_auth_token) + expect(json).not_to have_key(:sync_auth_token_digest) expect(json).to eq(TournamentSerializer.new(@tournament, scope: @tournament.owner).as_json) end @@ -389,8 +391,12 @@ RSpec.describe TournamentsController, type: :controller do expect(response).to have_http_status(:created) tournament = Tournament.find(deserialize_response(response)[:id]) expect(tournament.read_only_mode?).to eq(true) + expect(tournament.sync_auth_token).to be_nil + expect(tournament.sync_auth_token_digest).to be_present expect(tournament.teams).to be_empty expect(tournament.stages).to be_empty + expect(deserialize_response(response)).not_to have_key(:sync_auth_token) + expect(deserialize_response(response)).not_to have_key(:sync_auth_token_digest) end end @@ -602,6 +608,9 @@ RSpec.describe TournamentsController, type: :controller do expect(@tournament.read_only_mode?).to eq(false) expect(@tournament.sync_target_url).to include('/sync_state') expect(@tournament.sync_auth_token).to eq('new-token') + expect(@tournament.sync_auth_token_digest).to be_nil + expect(deserialize_response(response)).not_to have_key(:sync_auth_token) + expect(deserialize_response(response)).not_to have_key(:sync_auth_token_digest) end it 'allows follower takeover when only read_only_mode is disabled' do @@ -618,6 +627,7 @@ RSpec.describe TournamentsController, type: :controller do expect(@tournament.read_only_mode?).to eq(false) expect(@tournament.sync_target_url).to be_blank expect(@tournament.sync_auth_token).to be_blank + expect(@tournament.sync_auth_token_digest).to be_blank end it 'blocks normal updates while tournament is read only' do @@ -818,6 +828,8 @@ RSpec.describe TournamentsController, type: :controller do expect(@tournament.sync_source_tournament_id).to eq(123) expect(@tournament.name).to eq('Synced Tournament') expect(@tournament.teams.pluck(:name)).to eq(['Alpha']) + expect(deserialize_response(response)).not_to have_key(:sync_auth_token) + expect(deserialize_response(response)).not_to have_key(:sync_auth_token_digest) end it 'rejects invalid tokens' do @@ -857,6 +869,9 @@ RSpec.describe TournamentsController, type: :controller do expect(json[:id]).to eq(@tournament.id) expect(json[:sync_last_pushed_at]).to eq(pushed_at.iso8601) expect(json[:sync_last_push_error]).to be_nil + expect(json[:sync_auth_configured]).to eq(true) + expect(json).not_to have_key(:sync_auth_token) + expect(json).not_to have_key(:sync_auth_token_digest) expect(TournamentSyncPusher).to have_received(:push!).with(@tournament) end diff --git a/spec/e2e/http/tournament_sync_test_spec.rb b/spec/e2e/http/tournament_sync_test_spec.rb index 08cb789..a1d26f9 100644 --- a/spec/e2e/http/tournament_sync_test_spec.rb +++ b/spec/e2e/http/tournament_sync_test_spec.rb @@ -44,6 +44,8 @@ RSpec.describe 'Tournament sync test HTTP E2E' do expect(test_sync[:status]).to eq(200) expect(test_sync.dig(:json, :sync_last_pushed_at)).not_to be_nil expect(test_sync.dig(:json, :sync_last_push_error)).to be_nil + expect(test_sync.dig(:json, :sync_auth_configured)).to eq(true) + expect_no_sync_token_exposed!(test_sync, sync_token) after_leader = fetch_tournament(leader.fetch(:id)) after_follower = fetch_tournament(follower.fetch(:id)) @@ -54,6 +56,41 @@ RSpec.describe 'Tournament sync test HTTP E2E' do .to match_array(after_leader.fetch(:teams).map { |team| team.fetch(:name) }) end + it 'never exposes sync auth token after create, update, show, or sync test' do + follower_create = client.post('/tournaments', body: { + name: "Protected Follower #{SecureRandom.hex(3)}", + description: 'Follower token should stay write only', + public: true, + read_only_mode: true, + sync_auth_token: sync_token + }) + expect(follower_create[:status]).to eq(201) + expect_no_sync_token_exposed!(follower_create, sync_token) + + follower_id = follower_create.dig(:json, :id) + follower_owner_read = client.get("/tournaments/#{follower_id}") + expect(follower_owner_read[:status]).to eq(200) + expect(follower_owner_read.dig(:json, :sync_auth_configured)).to eq(true) + expect_no_sync_token_exposed!(follower_owner_read, sync_token) + + follower_public_read = anonymous_client.get("/tournaments/#{follower_id}") + expect(follower_public_read[:status]).to eq(200) + expect_no_sync_token_exposed!(follower_public_read, sync_token) + + leader = create_group_stage_tournament(name_prefix: 'Protected Leader') + configure_sync = client.patch("/tournaments/#{leader.fetch(:id)}", body: { + sync_target_url: "#{base_url}/tournaments/#{follower_id}/sync_state", + sync_auth_token: sync_token + }) + expect(configure_sync[:status]).to eq(200) + expect(configure_sync.dig(:json, :sync_auth_configured)).to eq(true) + expect_no_sync_token_exposed!(configure_sync, sync_token) + + test_sync = client.post("/tournaments/#{leader.fetch(:id)}/test_sync") + expect(test_sync[:status]).to eq(200) + expect_no_sync_token_exposed!(test_sync, sync_token) + end + it 'syncs team action list changes made through item id route and business key route' do follower = create_follower_tournament(name_prefix: 'Sync Action Follower') leader = create_group_stage_tournament( @@ -170,6 +207,14 @@ RSpec.describe 'Tournament sync test HTTP E2E' do response.fetch(:json) end + def expect_no_sync_token_exposed!(response, token) + serialized_json = response.fetch(:json).inspect + + expect(serialized_json).not_to include('sync_auth_token') + expect(serialized_json).not_to include('sync_auth_token_digest') + expect(serialized_json).not_to include(token) + end + def finish_group_stage_and_create_playoffs!(tournament_id) tournament = fetch_tournament(tournament_id) group_stage = tournament.fetch(:stages).find { |stage| stage.fetch(:level) == -1 } diff --git a/spec/models/tournament_spec.rb b/spec/models/tournament_spec.rb index c613d59..21a1267 100644 --- a/spec/models/tournament_spec.rb +++ b/spec/models/tournament_spec.rb @@ -43,6 +43,40 @@ RSpec.describe Tournament, type: :model do end end + describe 'sync auth token protection' do + it 'stores follower receiver tokens only as a digest' do + tournament = create(:tournament, read_only_mode: true, sync_auth_token: 'shared-secret') + + expect(tournament.sync_auth_token).to be_nil + expect(tournament.sync_auth_token_digest).to be_present + expect(tournament.sync_token_matches?('shared-secret')).to eq(true) + expect(tournament.sync_token_matches?('wrong-secret')).to eq(false) + expect(tournament.sync_accepts_push?).to eq(true) + end + + it 'keeps leader push token available for outbound sync' do + tournament = create( + :tournament, + sync_target_url: 'https://remote.example.com/tournaments/1/sync_state', + sync_auth_token: 'shared-secret' + ) + + expect(tournament.sync_auth_token).to eq('shared-secret') + expect(tournament.sync_auth_token_digest).to be_nil + expect(tournament.sync_push_enabled?).to eq(true) + end + + it 'clears follower token digest on takeover' do + tournament = create(:tournament, read_only_mode: true, sync_auth_token: 'shared-secret') + + tournament.update!(read_only_mode: false) + + expect(tournament.sync_auth_token).to be_nil + expect(tournament.sync_auth_token_digest).to be_nil + expect(tournament.sync_auth_configured?).to eq(false) + end + end + describe '#matches' do context 'group stage tournament' do before do