Skip to content

Auto-resync Slack details upon changes - #1525

Open
skyfallwastaken wants to merge 1 commit into
mainfrom
slack-auto-resync
Open

Auto-resync Slack details upon changes#1525
skyfallwastaken wants to merge 1 commit into
mainfrom
slack-auto-resync

Conversation

@skyfallwastaken

Copy link
Copy Markdown
Member

Summary of the problem

If you change your avatar or display name on Slack, it doesn't get picked up by Hackatime until a) the nightly re-sync completes, or b) you sign out and sign back in. Both are equally bad solutions!

Describe your changes

Listen for user_change events and trigger a re-sync if they're relevant!

Screenshots / Media

N/A

Copilot AI review requested due to automatic review settings August 4, 2026 23:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@greptile-apps

greptile-apps Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a signed Slack Events API endpoint that listens for user profile changes and queues profile synchronization, extending that synchronization to reconcile Slack email addresses.

  • Adds user_change event subscription and request-signature verification.
  • Detects relevant username, avatar, and email changes before enqueueing.
  • Reconciles Slack email sources and adds request/job coverage.

Confidence Score: 4/5

The former-Slack-email authentication path should be fixed before merging because a reassigned mailbox can gain access to the original user's account.

Email reconciliation converts superseded Slack addresses into identities accepted by the unfiltered magic-link sign-in flow, with no removal or re-verification boundary.

Files Needing Attention: app/models/concerns/slack_integration.rb

Security Review

The new email reconciliation retains a former Slack address as a valid magic-link identity. If that mailbox is later reassigned, its new owner can authenticate to the original Hackatime account. How this was verified: The demoted address remains in EmailAddress, and the magic-link job looks it up without a source or re-verification check.

Important Files Changed

Filename Overview
app/controllers/slack_controller.rb Adds signed Slack event handling, profile-change detection, and asynchronous sync dispatch with request coverage.
app/models/concerns/slack_integration.rb Extends profile synchronization to email identities but preserves superseded Slack addresses as active sign-in credentials.
app/jobs/slack_profile_sync_job.rb Retains the existing synchronization and rate-limit behavior while broadening its error-reporting message.
config/routes.rb Exposes the new Slack Events API callback route.
slack_manifest_harbor.yml Configures user_change delivery to the new callback endpoint.
spec/requests/slack_spec.rb Covers URL verification, signed event handling, change filtering, enqueueing, and invalid signatures.
test/jobs/slack_profile_sync_job_test.rb Covers email reconciliation and conflicts, while explicitly encoding preservation of old Slack emails for sign-in.

Sequence Diagram

sequenceDiagram
  participant Slack
  participant Controller as SlackController
  participant Job as SlackProfileSyncJob
  participant API as Slack API
  participant DB as Hackatime DB
  Slack->>Controller: POST /slack/events (user_change)
  Controller->>Controller: Verify signing secret and compare profile
  Controller->>Job: Enqueue user ID
  Job->>API: users.info
  API-->>Job: Current profile and email
  Job->>DB: Update username/avatar
  Job->>DB: Reconcile Slack EmailAddress
Loading
Prompt To Fix All With AI
### Issue 1
app/models/concerns/slack_integration.rb:82
**Former email remains authenticated**

If a former Slack mailbox is reassigned, converting its address to `source: :signing_in` leaves it accepted by the unfiltered magic-link flow, allowing the new mailbox owner to access the original user's Hackatime account. **How this was verified:** The demoted address remains in `EmailAddress`, and `HandleEmailSigninJob` looks it up without a source or re-verification check.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "Auto-resync Slack details upon changes" | Re-trigger Greptile


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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 security Former email remains authenticated

If a former Slack mailbox is reassigned, converting its address to source: :signing_in leaves it accepted by the unfiltered magic-link flow, allowing the new mailbox owner to access the original user's Hackatime account. How this was verified: The demoted address remains in EmailAddress, and HandleEmailSigninJob looks it up without a source or re-verification check.

Knowledge Base Used: Authentication: sessions, API keys, and OAuth

Prompt To Fix With AI
This is a comment left during a code review.
Path: app/models/concerns/slack_integration.rb
Line: 82

Comment:
**Former email remains authenticated**

If a former Slack mailbox is reassigned, converting its address to `source: :signing_in` leaves it accepted by the unfiltered magic-link flow, allowing the new mailbox owner to access the original user's Hackatime account. **How this was verified:** The demoted address remains in `EmailAddress`, and `HandleEmailSigninJob` looks it up without a source or re-verification check.

**Knowledge Base Used:** [Authentication: sessions, API keys, and OAuth](https://app.greptile.com/mahadk/-/custom-context/knowledge-base/hackclub/hackatime/-/docs/api-authentication.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants