Skip to content

Fixed oauth - #279

Merged
sfreeman422 merged 4 commits into
masterfrom
fix/front-end-auth
Sep 8, 2026
Merged

Fixed oauth#279
sfreeman422 merged 4 commits into
masterfrom
fix/front-end-auth

Conversation

@sfreeman422

@sfreeman422 sfreeman422 commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

This pull request makes two major changes: it migrates Slack authentication from the legacy OAuth v2 flow to Slack's OpenID Connect flow, and it removes the backend and frontend support for linking Sleeper fantasy accounts. The authentication update improves security and simplifies the login process, while the removal of Sleeper account-linking endpoints and related code streamlines the fantasy module.

Slack Authentication Migration:

  • Migrated from Slack OAuth v2 (/oauth/v2/authorize) to OpenID Connect (/openid/connect/authorize) for user authentication, updating all relevant endpoints, parameters, and test cases to use the new flow and OpenID scopes. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12]

Sleeper Account-Linking Removal:

  • Removed the /fantasy/profile endpoint and all backend logic for linking Sleeper accounts, including controller routes, service methods, and related tests. [1] [2] [3] [4] [5] [6] [7]
  • Removed frontend logic and UI for linking Sleeper accounts, including the linkSleeperUser hook and its usage/tests. [1] [2] [3] [4]

Documentation Updates:

  • Updated the README.md to clarify the new Slack authentication setup and to remove references to legacy scopes and flows. [1] [2]

Copilot AI lite review requested due to automatic review settings September 8, 2026 18:49

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The Slack OpenID Connect implementation is missing required authorization/token parameters (response_type, nonce, and grant_type), which can break sign-in in production.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR updates the Slack sign-in flow by switching from legacy Slack OAuth v2 to Slack OpenID Connect endpoints, and streamlines the fantasy module by removing Sleeper account-linking functionality across backend and frontend.

Changes:

  • Migrate Slack auth endpoints and payload expectations to Slack OpenID Connect (/openid/connect/authorize, openid.connect.token, openid.connect.userInfo) and update tests/docs accordingly.
  • Remove Sleeper account-linking API/UI, including the /fantasy/profile endpoint, hook logic, and related tests.
  • Update README Slack configuration guidance to reflect the OpenID Connect sign-in flow.
File summaries
File Description
README.md Updates Slack setup instructions to describe OpenID Connect login scope/behavior.
packages/frontend/src/pages/FantasyPage.tsx Removes Sleeper-linking UI and replaces it with a “linking unavailable” message.
packages/frontend/src/pages/FantasyPage.spec.tsx Updates UI tests to reflect removal of account-linking.
packages/frontend/src/hooks/useFantasy.ts Removes linkSleeperUser API call and exposed hook method.
packages/frontend/src/hooks/useFantasy.spec.ts Removes tests covering the deleted account-linking hook behavior.
packages/backend/src/fantasy/fantasy.service.ts Removes Sleeper-linking service method.
packages/backend/src/fantasy/fantasy.service.spec.ts Removes unit tests for the deleted service method.
packages/backend/src/fantasy/fantasy.controller.ts Removes the /profile route.
packages/backend/src/fantasy/fantasy.controller.spec.ts Updates controller tests to confirm the linking endpoint is no longer exposed.
packages/backend/src/auth/auth.controller.ts Switches to Slack OpenID Connect endpoints and adjusts token/userinfo handling.
packages/backend/src/auth/auth.controller.spec.ts Updates auth tests to the OpenID Connect URLs/response shapes.
packages/backend/src/auth/auth.const.ts Updates Slack auth constants to OpenID Connect URLs.
Review details

Suppressed comments (2)

packages/backend/src/auth/auth.controller.ts:106

  • Slack’s openid.connect.token endpoint expects grant_type=authorization_code in the form body. Without it, the code exchange can fail even with a valid code.
      new URLSearchParams({
        client_id: clientId,
        client_secret: clientSecret,
        code,
        redirect_uri: redirectUri,
      }).toString(),

packages/backend/src/auth/auth.controller.spec.ts:100

  • Once grant_type=authorization_code is added to the token exchange, this test should assert it’s present in the posted form body (otherwise the test won’t catch a broken OpenID token exchange payload).
      expect(Axios.post).toHaveBeenCalledWith(
        'https://slack.com/api/openid.connect.token',
        expect.stringContaining('code=valid-code'),
        { headers: { 'Content-Type': 'application/x-www-form-urlencoded' } },
      );
  • Files reviewed: 12/12 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/backend/src/auth/auth.controller.ts
Comment thread packages/backend/src/auth/auth.controller.spec.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The Slack OpenID token exchange is missing a required grant_type=authorization_code parameter, which can break authentication in production.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

packages/backend/src/auth/auth.controller.ts:108

  • Slack's OpenID Connect token exchange requires grant_type=authorization_code. The current request omits it, which can cause the /openid.connect.token call to fail even with a valid code.
      new URLSearchParams({
        client_id: clientId,
        client_secret: clientSecret,
        code,
        redirect_uri: redirectUri,
      }).toString(),

packages/frontend/src/pages/FantasyPage.tsx:266

  • This change removes the "Pending waivers" UI, but the PR description only mentions removing Sleeper account-linking and migrating Slack auth. If the pending-waivers removal is intentional, the PR description should call it out (and you may want to remove pendingWaivers from the fantasy API/model to avoid computing/sending data that the frontend no longer renders).
              <section>
                <div className="mb-3 flex flex-wrap items-center justify-between gap-3">
                  <div>
                    <h2 className="flex items-center gap-2 text-lg font-semibold">
                      <UserPlus className="h-5 w-5 text-primary" aria-hidden="true" /> Waiver wire proposals
                    </h2>
                    <p className="mt-1 text-sm text-muted-foreground">Waivers process Wednesday and Sunday.</p>
  • Files reviewed: 13/13 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment on lines +112 to +114

expect(blocks[0].text.text).toBe('AAPL :chart:');
});
@sfreeman422
sfreeman422 merged commit f4939c2 into master Sep 8, 2026
11 checks passed
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.

3 participants