Skip to content

feat(intercom): separate chat per org#112551

Merged
sentaur-athena merged 4 commits intomasterfrom
athena/intercom-switch-org
Apr 10, 2026
Merged

feat(intercom): separate chat per org#112551
sentaur-athena merged 4 commits intomasterfrom
athena/intercom-switch-org

Conversation

@sentaur-athena
Copy link
Copy Markdown
Member

Intercom creates a chat group per user. A user can access history of all their chats.
By defining user id that is user_id-org_id, we make intercom create a chat group per user<>org pair. That way a user only has access to their chat history from org A on org A and not their other orgs.

@sentaur-athena sentaur-athena requested a review from a team as a code owner April 8, 2026 22:43
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Apr 8, 2026
Comment thread src/sentry/api/endpoints/organization_intercom_jwt.py Outdated
Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit d81e79f. Configure here.

Comment thread src/sentry/api/endpoints/organization_intercom_jwt.py Outdated
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 8, 2026

Backend Test Failures

Failures on 12ed624 in this run:

tests/sentry/api/endpoints/test_organization_intercom_jwt.py::OrganizationIntercomJwtEndpointTest::test_get_jwt_successlog
[gw1] linux -- Python 3.13.1 /home/runner/work/sentry/sentry/.venv/bin/python3
tests/sentry/api/endpoints/test_organization_intercom_jwt.py:42: in test_get_jwt_success
    assert user_data["userId"] == str(self.user.id)
E   AssertionError: assert '6-4557934876753952' == '6'
E     
E     - 6
E     + 6-4557934876753952

@sentaur-athena
Copy link
Copy Markdown
Member Author

I should most probably not expose user and org ID and just encode a string from it. 😅

Copy link
Copy Markdown
Member

@wedamija wedamija left a comment

Choose a reason for hiding this comment

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

This means they'll lose history for any old chats. Is that a problem?

@sentaur-athena
Copy link
Copy Markdown
Member Author

@wedamija this product is not launched yet. You mean they lose the history before this code change right? Then yes, that's ok.

Comment on lines +73 to +74
# intercom creates chat history based on this ID so we need one per (org, user) pair
intercom_user_id = md5_text(f"{user.id}-{organization.id}").hexdigest()
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.

Is this an Intercom specified format? If so, can we add the Intercom documentation link for this? Otherwise, if we're specifying the format, we might want to consider not using md5, as it's known for hash collisions.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@michelletran-sentry no, this was a choice. They don't expect any format. I just wanted to generate a unique string per {user and org} pair.

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.

If we're specifying the format, then the risk is potential hash collision for legitimate compound strings (quite low, but might happen). The JWT is tamper-proof because it's signed, so using a readable id should be fine. I would suggest using the compound key directly. If you would like to preserve obfuscation, then using SHA256 will be less prone to collisions.

@sentaur-athena and I synced offline and it seems that compound keys should be fine.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Awesome. Thanks for the help. I'll go back to commits ago where this as not encoded.

@sentaur-athena sentaur-athena enabled auto-merge (squash) April 10, 2026 20:27
@sentaur-athena sentaur-athena merged commit e3a1992 into master Apr 10, 2026
56 checks passed
@sentaur-athena sentaur-athena deleted the athena/intercom-switch-org branch April 10, 2026 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants