Skip to content

fix: require state verification on all app installs#28781

Draft
pedroccastro wants to merge 1 commit intomainfrom
fix/require-state-on-app-install
Draft

fix: require state verification on all app installs#28781
pedroccastro wants to merge 1 commit intomainfrom
fix/require-state-on-app-install

Conversation

@pedroccastro
Copy link
Copy Markdown
Contributor

What does this PR do?

Removes the nonce exemption for basecamp3, webex, and tandem OAuth integrations. All app installs now require state parameter verification via HMAC nonce, consistent with other integrations.

Changes

  • Removed NONCE_EXEMPT_APPS allowlist from decodeOAuthState
  • Removed appSlug parameter from decodeOAuthState
  • Added encodeOAuthState to basecamp3, webex, and tandem /add handlers
  • Moved decodeOAuthState + auth checks to the top of callbacks (before token exchange and DB writes)
  • Updated tests

How should this be tested?

Automated

TZ=UTC yarn vitest run packages/app-store/_utils/oauth/decodeOAuthState.test.ts            

Manual

  1. Install basecamp3/webex/tandem integration → OAuth flow should complete normally
  2. Open a callback URL directly without state parameter (e.g. /api/integrations/basecamp3/callback?code=test) → should return 403
  3. Verify existing integrations continue to work

Mandatory Tasks

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • N/A I have updated the developer docs in /docs if this PR makes changes that would require a documentation change.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant