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

fix(middleware): use includes() for NextAuth pages #5104

Merged
merged 3 commits into from Sep 18, 2022
Merged

fix(middleware): use includes() for NextAuth pages #5104

merged 3 commits into from Sep 18, 2022

Conversation

Juneezee
Copy link
Contributor

@Juneezee Juneezee commented Aug 6, 2022

Something went wrong with #5085, GitHub wasn't updating the PR with the new commits

☕️ Reasoning

Some users could be setting their signIn and error pages option to
/ to disable the automatically generated pages, as suggested in #2330 (reply in thread).

This PR reverts the behaviour for matching signIn and error
pages in handleMiddleware to pre-v4.10.3.

EDIT:

const signInPage = "/"
const errorPage = "/"
const publicPaths = [signInPage, errorPage, "/_next", "/favicon.ico"]

// pathname = "/protected" will always return true
publicPaths.some((p) => pathname.startsWith(p))

Fixes: aedabc8 ("fix: avoid redirect on always public paths")

🧢 Checklist

  • Documentation
  • Tests
  • Ready to be merged

🎫 Affected issues

📌 Resources

@vercel
Copy link

vercel bot commented Aug 6, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Updated
next-auth ⬜️ Ignored (Inspect) Sep 18, 2022 at 3:52AM (UTC)

@github-actions github-actions bot added the core Refers to `@auth/core` label Aug 6, 2022
Copy link
Member

@ThangHuuVu ThangHuuVu left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the clean PR 🙌

@vercel vercel bot temporarily deployed to Preview September 11, 2022 03:59 Inactive
Juneezee and others added 3 commits September 16, 2022 16:49
Some users could be setting their `signIn` and `error` pages option to
`/` to disable the automatically generated pages, as suggested in [1].

This commit reverts the behaviour for matching `signIn` and `error`
pages in `handleMiddleware` to pre-v4.10.3.

```
const signInPage = "/"
const errorPage = "/"
const publicPaths = [signInPage, errorPage, "/_next", "/favicon.ico"]

// pathname = "/" will return true
publicPaths.some((p) => pathname.startsWith(p))
```

Fixes: aedabc8 ("fix: avoid redirect on always public paths")
Reference [1]: #2330 (reply in thread)
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
@ThangHuuVu ThangHuuVu merged commit 44f2a47 into nextauthjs:main Sep 18, 2022
@ThangHuuVu
Copy link
Member

Thanks again @Juneezee! 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Refers to `@auth/core`
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants