From 01234b59cafe153d3638e8d94dcb52c1594ae5a2 Mon Sep 17 00:00:00 2001 From: Mahad Kalam Date: Wed, 5 Aug 2026 00:28:41 +0100 Subject: [PATCH 1/3] Auto-resync Slack details upon changes --- app/controllers/slack_controller.rb | 31 +++++- app/jobs/slack_profile_sync_job.rb | 2 +- app/models/concerns/slack_integration.rb | 21 ++++ config/routes.rb | 1 + slack_manifest_harbor.yml | 4 + spec/requests/slack_spec.rb | 132 +++++++++++++++++++++++ test/jobs/slack_profile_sync_job_test.rb | 61 +++++++++++ 7 files changed, 249 insertions(+), 3 deletions(-) diff --git a/app/controllers/slack_controller.rb b/app/controllers/slack_controller.rb index 3150f0131..bb6385a90 100644 --- a/app/controllers/slack_controller.rb +++ b/app/controllers/slack_controller.rb @@ -22,8 +22,34 @@ def create SlackCommand::SailorsLogJob.perform_later(params_hash) end + def events + return render(json: { challenge: params[:challenge] }) if params[:type] == "url_verification" + + if params[:type] == "event_callback" && params.dig(:event, :type) == "user_change" + slack_user = params.dig(:event, :user) + user = User.find_by(slack_uid: slack_user[:id]) if slack_user + SlackProfileSyncJob.perform_later(user.id) if user && slack_profile_changed?(user, slack_user) + end + + head :ok + end + private + def slack_profile_changed?(user, slack_user) + profile = slack_user[:profile] || {} + slack_username = + profile[:display_name_normalized].presence || + profile[:real_name_normalized].presence || + slack_user[:name].presence + slack_avatar_url = profile[:image_192].presence || profile[:image_72].presence + slack_email = profile[:email].to_s.strip.downcase.presence + + (slack_username.present? && slack_username != user.slack_username) || + (slack_avatar_url.present? && slack_avatar_url != user.slack_avatar_url) || + (slack_email.present? && slack_email != user.email_addresses.source_slack.pick(:email)) + end + def params_hash @params_hash ||= params.permit(:command, :text, :response_url, :user_id, :team_id, :team_domain, :channel_id, :channel_name, :user_name, :trigger_word).to_h @@ -32,10 +58,11 @@ def params_hash def verify_slack_request return true if Rails.env.development? - signing_secret = ENV["SAILORS_LOG_SLACK_SIGNING_SECRET"] + signing_secret_name = action_name == "events" ? "SLACK_SIGNING_SECRET" : "SAILORS_LOG_SLACK_SIGNING_SECRET" + signing_secret = ENV[signing_secret_name] if signing_secret.blank? # we will never hit this in prod but this is good prep for `config.saas_mode` - Rails.logger.error "[SlackController] SAILORS_LOG_SLACK_SIGNING_SECRET is not configured" + Rails.logger.error "[SlackController] #{signing_secret_name} is not configured" return head(:unauthorized) end diff --git a/app/jobs/slack_profile_sync_job.rb b/app/jobs/slack_profile_sync_job.rb index c94b135f1..0354f791d 100644 --- a/app/jobs/slack_profile_sync_job.rb +++ b/app/jobs/slack_profile_sync_job.rb @@ -21,7 +21,7 @@ def perform(user_id) polynomial_delay = executions**4 + (Kernel.rand * executions**4 * 0.15) + 2 retry_job(wait: [ e.retry_after, polynomial_delay ].max.seconds) rescue => e - report_error(e, message: "Failed to update Slack username and avatar for user #{user_id}") + report_error(e, message: "Failed to update Slack profile for user #{user_id}") raise end end diff --git a/app/models/concerns/slack_integration.rb b/app/models/concerns/slack_integration.rb index 502c73b32..be41204a4 100644 --- a/app/models/concerns/slack_integration.rb +++ b/app/models/concerns/slack_integration.rb @@ -17,6 +17,12 @@ def initialize(retry_after) end end + class EmailConflictError < StandardError + def initialize(email) + super("Slack email is already linked to another Hackatime account: #{email}") + end + end + STATUS_EMOJI_BUCKETS = [ [ 30.minutes, %w[thinking cat-on-the-laptop loading-tumbleweed rac-yap] ], [ 1.hour, %w[working-parrot meow_code] ], @@ -49,6 +55,7 @@ def update_from_slack return unless user_data.present? apply_slack_profile_attributes(user_data) + sync_slack_email(user_data.dig("profile", "email")) self.slack_synced_at = Time.current end @@ -63,6 +70,20 @@ def apply_slack_profile_attributes(slack_user) slack_user["name"].presence end + def sync_slack_email(raw_email) + email = raw_email.to_s.strip.downcase.presence + return unless email + + email_address = EmailAddress.find_by(email: email) + raise EmailConflictError, email if email_address && email_address.user_id != id + + transaction do + email_address ||= email_addresses.create!(email: email, source: :slack) + email_addresses.source_slack.where.not(id: email_address.id).update_all(source: :signing_in) + email_address.update!(source: :slack) unless email_address.source_slack? + end + end + def update_slack_status return unless uses_slack_status? diff --git a/config/routes.rb b/config/routes.rb index df9838b63..7db6b8406 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -225,6 +225,7 @@ def matches?(request) get "my/wakatime_setup/step-4", to: redirect("/setup") post "/sailors_log/slack/commands", to: "slack#create" + post "/slack/events", to: "slack#events" get "/hackatime/v1", to: redirect("/", status: 302) # some clients seem to link this as the user's dashboard instead of /api/v1/hackatime # API routes diff --git a/slack_manifest_harbor.yml b/slack_manifest_harbor.yml index 9680bc361..fc042056f 100644 --- a/slack_manifest_harbor.yml +++ b/slack_manifest_harbor.yml @@ -18,6 +18,10 @@ oauth_config: - users:read - users:read.email settings: + event_subscriptions: + request_url: https://hackatime.hackclub.com/slack/events + user_events: + - user_change org_deploy_enabled: false socket_mode_enabled: false token_rotation_enabled: false diff --git a/spec/requests/slack_spec.rb b/spec/requests/slack_spec.rb index ead5d5d3d..2b218ab95 100644 --- a/spec/requests/slack_spec.rb +++ b/spec/requests/slack_spec.rb @@ -32,4 +32,136 @@ end end end + + path '/slack/events' do + post('Handle Slack Events') do + tags 'Slack' + description 'Handle Slack Events API callbacks for the Hackatime Slack app.' + consumes 'application/json' + produces 'application/json' + + parameter name: :event_payload, in: :body, schema: { + type: :object, + properties: { + type: { type: :string }, + challenge: { type: :string }, + event: { type: :object } + } + } + + response(200, 'successful', document: false) do + let(:event_payload) { { type: 'url_verification', challenge: 'challenge-token' } } + before { allow(Rails.env).to receive(:development?).and_return(true) } + run_test! + end + end + end + + describe 'POST /slack/events' do + include ActiveJob::TestHelper + + let(:signing_secret) { 'signing-secret' } + let(:timestamp) { Time.current.to_i.to_s } + + around do |example| + original_secret = ENV['SLACK_SIGNING_SECRET'] + ENV['SLACK_SIGNING_SECRET'] = signing_secret + example.run + ensure + ENV['SLACK_SIGNING_SECRET'] = original_secret + end + + before do + ActiveJob::Base.queue_adapter = :test + clear_enqueued_jobs + end + + def post_signed_event(payload) + body = payload.to_json + signature = 'v0=' + OpenSSL::HMAC.hexdigest('SHA256', signing_secret, "v0:#{timestamp}:#{body}") + post '/slack/events', params: body, headers: { + 'CONTENT_TYPE' => 'application/json', + 'X-Slack-Request-Timestamp' => timestamp, + 'X-Slack-Signature' => signature + } + end + + it 'responds to Slack URL verification' do + post_signed_event(type: 'url_verification', challenge: 'challenge-token') + + expect(response).to have_http_status(:ok) + expect(response.parsed_body).to eq('challenge' => 'challenge-token') + end + + it 'enqueues a profile sync when the Slack email changes' do + user = User.create!( + timezone: 'UTC', + slack_uid: 'U_EVENT_USER', + slack_username: 'old-name', + slack_avatar_url: 'https://example.com/old.png' + ) + user.email_addresses.create!(email: 'old@example.com', source: :slack) + + expect { + post_signed_event( + type: 'event_callback', + event_id: 'EvProfileChanged', + event: { + type: 'user_change', + user: { + id: user.slack_uid, + name: 'fallback-name', + profile: { + display_name_normalized: user.slack_username, + image_192: user.slack_avatar_url, + email: 'new@example.com' + } + } + } + ) + }.to have_enqueued_job(SlackProfileSyncJob).with(user.id) + + expect(response).to have_http_status(:ok) + end + + it 'ignores a user_change event when only unrelated profile data changed' do + user = User.create!( + timezone: 'UTC', + slack_uid: 'U_STATUS_USER', + slack_username: 'same-name', + slack_avatar_url: 'https://example.com/same.png' + ) + + expect { + post_signed_event( + type: 'event_callback', + event_id: 'EvStatusChanged', + event: { + type: 'user_change', + user: { + id: user.slack_uid, + name: 'fallback-name', + profile: { + display_name_normalized: user.slack_username, + image_192: user.slack_avatar_url, + status_text: 'Coding' + } + } + } + ) + }.not_to have_enqueued_job(SlackProfileSyncJob) + + expect(response).to have_http_status(:ok) + end + + it 'rejects requests with an invalid Slack signature' do + post '/slack/events', params: { type: 'event_callback' }.to_json, headers: { + 'CONTENT_TYPE' => 'application/json', + 'X-Slack-Request-Timestamp' => timestamp, + 'X-Slack-Signature' => 'v0=invalid' + } + + expect(response).to have_http_status(:unauthorized) + end + end end diff --git a/test/jobs/slack_profile_sync_job_test.rb b/test/jobs/slack_profile_sync_job_test.rb index 3e7309503..e1d7135dd 100644 --- a/test/jobs/slack_profile_sync_job_test.rb +++ b/test/jobs/slack_profile_sync_job_test.rb @@ -39,6 +39,67 @@ class SlackProfileSyncJobTest < ActiveJob::TestCase assert_not_nil user.slack_synced_at end + test "reconciles the Slack email while preserving the previous email for sign-in" do + user = User.create!(timezone: "UTC", slack_uid: "U_EMAIL_SYNC") + old_email = user.email_addresses.create!(email: "old@example.com", source: :slack) + stub_request(:get, "https://slack.com/api/users.info?user=U_EMAIL_SYNC") + .with(headers: { "Authorization" => "Bearer workspace-token" }) + .to_return(body: { + ok: true, + user: { + name: "email-sync", + profile: { email: "New@Example.com" } + } + }.to_json) + + SlackProfileSyncJob.perform_now(user.id) + + assert_predicate old_email.reload, :source_signing_in? + assert_predicate user.email_addresses.find_by!(email: "new@example.com"), :source_slack? + end + + test "promotes an already-linked sign-in email when Slack changes to it" do + user = User.create!(timezone: "UTC", slack_uid: "U_EXISTING_EMAIL_SYNC") + old_email = user.email_addresses.create!(email: "old@example.com", source: :slack) + new_email = user.email_addresses.create!(email: "new@example.com", source: :signing_in) + stub_request(:get, "https://slack.com/api/users.info?user=U_EXISTING_EMAIL_SYNC") + .to_return(body: { + ok: true, + user: { + name: "existing-email-sync", + profile: { email: "new@example.com" } + } + }.to_json) + + SlackProfileSyncJob.perform_now(user.id) + + assert_predicate old_email.reload, :source_signing_in? + assert_predicate new_email.reload, :source_slack? + end + + test "does not claim a Slack email linked to another account" do + user = User.create!(timezone: "UTC", slack_uid: "U_EMAIL_CONFLICT") + old_email = user.email_addresses.create!(email: "old@example.com", source: :slack) + other_user = User.create!(timezone: "UTC") + other_email = other_user.email_addresses.create!(email: "taken@example.com", source: :signing_in) + stub_request(:get, "https://slack.com/api/users.info?user=U_EMAIL_CONFLICT") + .to_return(body: { + ok: true, + user: { + name: "email-conflict", + profile: { email: "taken@example.com" } + } + }.to_json) + + assert_raises(SlackIntegration::EmailConflictError) do + SlackProfileSyncJob.perform_now(user.id) + end + + assert_predicate old_email.reload, :source_slack? + assert_equal other_user, other_email.reload.user + assert_nil user.reload.slack_synced_at + end + test "retries Slack rate limits without changing the existing profile" do user = User.create!( timezone: "UTC", From 7213097595ad45c0376f4597128b8f5fc0f7fbfc Mon Sep 17 00:00:00 2001 From: Mahad Kalam Date: Thu, 6 Aug 2026 12:39:38 +0100 Subject: [PATCH 2/3] Prevent old Slack emails from remaining login credentials --- app/models/concerns/oauth_authentication.rb | 2 +- app/models/concerns/slack_integration.rb | 2 +- test/jobs/slack_profile_sync_job_test.rb | 6 ++-- test/models/user_test.rb | 31 +++++++++++++++++++++ 4 files changed, 36 insertions(+), 5 deletions(-) diff --git a/app/models/concerns/oauth_authentication.rb b/app/models/concerns/oauth_authentication.rb index 4d85ed248..c5d4162fd 100644 --- a/app/models/concerns/oauth_authentication.rb +++ b/app/models/concerns/oauth_authentication.rb @@ -97,7 +97,7 @@ def from_slack_token(code, redirect_uri, ip_address = nil) u.email_addresses << email_address unless u.email_addresses.include?(email_address) end - user.email_addresses.source_slack.where.not(email: email).update_all(source: :signing_in) + user.email_addresses.source_slack.where.not(email: email).destroy_all email_address.source = :slack email_address.save! if email_address.persisted? diff --git a/app/models/concerns/slack_integration.rb b/app/models/concerns/slack_integration.rb index be41204a4..b7b81c585 100644 --- a/app/models/concerns/slack_integration.rb +++ b/app/models/concerns/slack_integration.rb @@ -79,7 +79,7 @@ def sync_slack_email(raw_email) transaction do email_address ||= email_addresses.create!(email: email, source: :slack) - email_addresses.source_slack.where.not(id: email_address.id).update_all(source: :signing_in) + email_addresses.source_slack.where.not(id: email_address.id).destroy_all email_address.update!(source: :slack) unless email_address.source_slack? end end diff --git a/test/jobs/slack_profile_sync_job_test.rb b/test/jobs/slack_profile_sync_job_test.rb index e1d7135dd..d815505e0 100644 --- a/test/jobs/slack_profile_sync_job_test.rb +++ b/test/jobs/slack_profile_sync_job_test.rb @@ -39,7 +39,7 @@ class SlackProfileSyncJobTest < ActiveJob::TestCase assert_not_nil user.slack_synced_at end - test "reconciles the Slack email while preserving the previous email for sign-in" do + test "reconciles the Slack email without preserving the previous email for sign-in" do user = User.create!(timezone: "UTC", slack_uid: "U_EMAIL_SYNC") old_email = user.email_addresses.create!(email: "old@example.com", source: :slack) stub_request(:get, "https://slack.com/api/users.info?user=U_EMAIL_SYNC") @@ -54,7 +54,7 @@ class SlackProfileSyncJobTest < ActiveJob::TestCase SlackProfileSyncJob.perform_now(user.id) - assert_predicate old_email.reload, :source_signing_in? + assert_not EmailAddress.exists?(old_email.id) assert_predicate user.email_addresses.find_by!(email: "new@example.com"), :source_slack? end @@ -73,7 +73,7 @@ class SlackProfileSyncJobTest < ActiveJob::TestCase SlackProfileSyncJob.perform_now(user.id) - assert_predicate old_email.reload, :source_signing_in? + assert_not EmailAddress.exists?(old_email.id) assert_predicate new_email.reload, :source_slack? end diff --git a/test/models/user_test.rb b/test/models/user_test.rb index c84e34da7..839784095 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -147,6 +147,37 @@ class UserTest < ActiveSupport::TestCase ENV["SLACK_USER_OAUTH_TOKEN"] = original_token end + test "Slack authentication removes a superseded Slack email" do + user = User.create!(timezone: "UTC", slack_uid: "U_EMAIL_CHANGE") + old_email = user.email_addresses.create!(email: "old@example.com", source: :slack) + new_email = user.email_addresses.create!(email: "new@example.com", source: :signing_in) + stub_request(:post, "https://slack.com/api/oauth.v2.access") + .to_return(body: { + ok: true, + authed_user: { + id: "U_EMAIL_CHANGE", + access_token: "slack-token", + scope: "users:read,users:read.email" + } + }.to_json) + stub_request(:get, "https://slack.com/api/users.info?user=U_EMAIL_CHANGE") + .with(headers: { "Authorization" => "Bearer slack-token" }) + .to_return(body: { + ok: true, + user: { + name: "email-change", + tz: "UTC", + profile: { email: new_email.email } + } + }.to_json) + + authenticated_user = User.from_slack_token("code", "https://example.com/auth/slack/callback") + + assert_equal user, authenticated_user + assert_not EmailAddress.exists?(old_email.id) + assert_predicate new_email.reload, :source_slack? + end + test "HCA authentication fills a missing Slack ID on an existing account" do user = User.create!(timezone: "UTC", hca_id: "hca-existing") stub_request(:post, "https://hca.dinosaurbbq.org/oauth/token") From d5b52ef1d920e2f1c813f3ccb30fadfa046061ad Mon Sep 17 00:00:00 2001 From: Mahad Kalam Date: Thu, 6 Aug 2026 12:46:14 +0100 Subject: [PATCH 3/3] Rollback Slack email changes when OAuth fails --- app/models/concerns/oauth_authentication.rb | 24 +++++++++------- test/models/user_test.rb | 32 +++++++++++++++++++++ 2 files changed, 45 insertions(+), 11 deletions(-) diff --git a/app/models/concerns/oauth_authentication.rb b/app/models/concerns/oauth_authentication.rb index c5d4162fd..85946c081 100644 --- a/app/models/concerns/oauth_authentication.rb +++ b/app/models/concerns/oauth_authentication.rb @@ -97,17 +97,19 @@ def from_slack_token(code, redirect_uri, ip_address = nil) u.email_addresses << email_address unless u.email_addresses.include?(email_address) end - user.email_addresses.source_slack.where.not(email: email).destroy_all - email_address.source = :slack - email_address.save! if email_address.persisted? - - user.slack_uid = data.dig("authed_user", "id") - user.apply_slack_profile_attributes(slack_user) - user.parse_and_set_timezone(slack_user["tz"]) - user.slack_access_token = data["authed_user"]["access_token"] - user.slack_scopes = data["authed_user"]["scope"]&.split(/,\s*/) - user.country_code = country_code_from_ip(ip_address) if user.country_code.blank? - user.save! + User.transaction do + user.email_addresses.source_slack.where.not(email: email).destroy_all + email_address.source = :slack + email_address.save! if email_address.persisted? + + user.slack_uid = data.dig("authed_user", "id") + user.apply_slack_profile_attributes(slack_user) + user.parse_and_set_timezone(slack_user["tz"]) + user.slack_access_token = data["authed_user"]["access_token"] + user.slack_scopes = data["authed_user"]["scope"]&.split(/,\s*/) + user.country_code = country_code_from_ip(ip_address) if user.country_code.blank? + user.save! + end user rescue => e report_error(e, message: "Error creating user from Slack data: #{e.message}") diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 839784095..03b1cd89a 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -178,6 +178,38 @@ class UserTest < ActiveSupport::TestCase assert_predicate new_email.reload, :source_slack? end + test "Slack authentication restores email changes when the user cannot be saved" do + user = User.create!(timezone: "UTC", slack_uid: "U_ORIGINAL") + old_email = user.email_addresses.create!(email: "old@example.com", source: :slack) + new_email = user.email_addresses.create!(email: "new@example.com", source: :signing_in) + User.create!(timezone: "UTC", slack_uid: "U_CONFLICT") + stub_request(:post, "https://slack.com/api/oauth.v2.access") + .to_return(body: { + ok: true, + authed_user: { + id: "U_CONFLICT", + access_token: "slack-token", + scope: "users:read,users:read.email" + } + }.to_json) + stub_request(:get, "https://slack.com/api/users.info?user=U_CONFLICT") + .with(headers: { "Authorization" => "Bearer slack-token" }) + .to_return(body: { + ok: true, + user: { + name: "email-change", + tz: "UTC", + profile: { email: new_email.email } + } + }.to_json) + + assert_nil User.from_slack_token("code", "https://example.com/auth/slack/callback") + + assert_predicate old_email.reload, :source_slack? + assert_predicate new_email.reload, :source_signing_in? + assert_equal "U_ORIGINAL", user.reload.slack_uid + end + test "HCA authentication fills a missing Slack ID on an existing account" do user = User.create!(timezone: "UTC", hca_id: "hca-existing") stub_request(:post, "https://hca.dinosaurbbq.org/oauth/token")