Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce authOptions with getServerSession wrapper #4214

Merged
merged 1 commit into from
Feb 21, 2024

Conversation

Vinnl
Copy link
Collaborator

@Vinnl Vinnl commented Feb 15, 2024

References:

Jira: MNTOR-2954
Figma: N/A

Description

Next-Auth's type definitions for getServerSession() don't enforce the first argument, and v5 (aka Auth.js) removes the function altogether, so an upstream fix is probably not in the books. And since the upstream docs recommend a wrapper (see https://next-auth.js.org/configuration/nextjs#getserversession), we can just enforce that through ESLint to avoid future instsances of the auth options not being passed.

How to test

There should be no behavioural changes.

Checklist (Definition of Done)

  • Localization strings (if needed) have been added.
  • Commits in this PR are minimal and have descriptive commit messages.
  • I've added or updated the relevant sections in readme and/or code comments
  • I've added a unit test to test for potential regressions of this bug.
  • Product Owner accepted the User Story (demo of functionality completed) or waived the privilege.
  • All acceptance criteria are met.
  • Jira ticket has been updated (if needed) to match changes made during the development process.
  • Jira ticket has been updated (if needed) with suggestions for QA when this PR is deployed to stage.

@Vinnl Vinnl added the Review: XS Code review time: up to 30min label Feb 15, 2024
@Vinnl Vinnl requested review from rhelmer and mansaj February 15, 2024 15:13
@Vinnl Vinnl self-assigned this Feb 15, 2024
Copy link
Collaborator

@mansaj mansaj left a comment

Choose a reason for hiding this comment

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

Tested locally, ran e2e, LGTM

Next-Auth's type definitions for getServerSession() don't enforce
the first argument, and v5 (aka Auth.js) removes the function
altogether, so an upstream fix is probably not in the books. And
since the upstream docs recommend a wrapper (see
https://next-auth.js.org/configuration/nextjs#getserversession), we
can just enforce that through ESLint to avoid future instsances of
the auth options not being passed.
@Vinnl Vinnl force-pushed the MNTOR-2954-getServerSession-wrapper branch from 2c547ba to 095f7f4 Compare February 21, 2024 14:44
@Vinnl Vinnl merged commit 7375aa0 into main Feb 21, 2024
13 checks passed
@Vinnl Vinnl deleted the MNTOR-2954-getServerSession-wrapper branch February 21, 2024 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Review: XS Code review time: up to 30min
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants