feat: add CSRF nonce to Slack OAuth state parameter #946
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 22f5bd0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@cursor review |
packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts
Show resolved
Hide resolved
|
@cursor review |
packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts
Outdated
Show resolved
Hide resolved
|
@cursor review |
packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 9d524a1. Configure here.
| nonceStorageKey: getSlackNonceStorageKey( | ||
| knockSlackChannelId, | ||
| knock.userId!, | ||
| ), |
There was a problem hiding this comment.
Polling path bypasses CSRF nonce verification entirely
Medium Severity
useAuthPolling runs concurrently alongside useAuthPostMessageListener in SlackAuthButton and can detect auth success via server-side polling, setting the connection status to "connected" and calling onAuthenticationComplete without any CSRF nonce verification. This provides an alternative success path that completely bypasses the nonce check this PR adds. The stored nonce in sessionStorage is also never cleaned up when polling detects success.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 9d524a1. Configure here.
There was a problem hiding this comment.
Noted and discussed here: https://knocklabs.slack.com/archives/C06FHSQ18G4/p1775855122551969?thread_ts=1775582016.010559&cid=C06FHSQ18G4
No action at this time.
packages/react-core/src/modules/core/hooks/useAuthPostMessageListener.ts
Outdated
Show resolved
Hide resolved
9d524a1 to
22f5bd0
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #946 +/- ##
==========================================
+ Coverage 63.85% 64.04% +0.19%
==========================================
Files 209 209
Lines 10007 10060 +53
Branches 1281 1297 +16
==========================================
+ Hits 6390 6443 +53
Misses 3592 3592
Partials 25 25
|



Description
A customer reported that Slack's app review process requires a nonce in the OAuth state parameter: https://knocklabs.slack.com/archives/C06FHSQ18G4/p1775582016010559
In this PR we now generate a random nonce via crypto.randomUUID and include in the state param when building the auth URL, store it in sessionStorage, and verify it against the nonce echoed back in the postMessage payload.
Note, the postMessage listener accepts both the legacy raw string ("authComplete") and a new structured format ({ type, nonce }) for backward compatibility with older API versions that haven't been updated yet.
There is a corresponding PR in switchboard that echoes back the provided nonce: https://github.com/knocklabs/switchboard/pull/6104