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

Not found is being displayed within the admin panel styling for non authenticated users #8716

Closed
karlapsite opened this issue Oct 15, 2024 · 9 comments
Assignees

Comments

@karlapsite
Copy link

karlapsite commented Oct 15, 2024

Link to reproduction

https://github.com/karlapsite/payload/tree/fix/create-user-redirection-issue-reproduction

diff

Environment Info

Should be latest n' greatest:

Payload: v3.0.0-beta.114
Node.js: 22.6.0
Next.js: 15.0.0-canary.17

Describe the Bug

I'm playing around with the blank and website templates with a coworker. We're a little confused why navigating in browser to /admin/foobar doesn't forward us to /admin/create-first-user like it does in other cases... I haven't created my first account yet... it seems a little weird to be able to peek at the public collections (or render any semblance of the admin portal at all yet) Clicking on anything does redirect me to /admin/create-first-user

Is this intentional behavior?

If we don't want users to peak the unauthed admin page... is there an easy way to modify the admin not-found template to redirect us somehow? or is there a cleaner place to catch/forward this case?

A similar issue seems to exist after the first account has been created... my expectation was to see the admin portal redirect the user to the login component and prevent them from seeing the admin portal/sidebar.

To my knowledge this isn't a security issue, as the user can only see the names of collections that are public read. Authorized collections won't appear in the sidebar... So this is only disorienting at best

Reproduction Steps

  1. Comment out the devUser in the _community template.
  2. Before creating an account, or logging in:
  3. Navigate to /admin/foobar
    Image

Adapters and Plugins

No response

@karlapsite karlapsite added status: needs-triage Possible bug which hasn't been reproduced yet v3 labels Oct 15, 2024
karlapsite added a commit to karlapsite/payload that referenced this issue Oct 15, 2024
Choosing to evauate whether users exist in the db earlier allows payload
to redirect users to a more appropriate view if the first admin user
hasn't been created yet.
karlapsite added a commit to karlapsite/payload that referenced this issue Oct 15, 2024
Choosing to evauate whether users exist in the db earlier allows payload
to redirect users to a more appropriate view if the first admin user
hasn't been created yet.
@paulpopus paulpopus self-assigned this Oct 15, 2024
@github-actions github-actions bot removed the status: needs-triage Possible bug which hasn't been reproduced yet label Oct 15, 2024
@karlapsite
Copy link
Author

@paulpopus I did end up fiddling with the order of operations in packages/next/src/views/Root/index.tsx which you can see here

  1. I wasn't sure if I this was something that made sense to open a PR for.
  2. I didn't have much success running the e2e tests to completion... they appeared to hang.
  3. I'm not sure how to logout in the local _community dev instance... something seems to forward the user through the login experience there, and fairly certain there's a related issue there.

@akhrarovsaid
Copy link
Contributor

Hey @karlapsite,

So I actually reproduced your issue using the _community folder in the monorepo. You can avoid the default login by commenting out these lines. I don't believe there's an issue there as this is done for a smoother dev experience while working with the test folders.

@karlapsite
Copy link
Author

karlapsite commented Oct 16, 2024

Thanks for the tip @akhrarovsaid! The specific issue with the dev credentials in my third point is a little nitty-gritty.

I didn't have too much trouble commenting out the devUser to play around with the /create-first-user flow... but once I made my devUser... I wanted to make sure that payload would properly redirect users to the login page... and there was some trouble there.

When using default credentials, I couldn't logout. manually navigating to /admin/logout indicated that I did logout in a toast, but I suspect something in the _community folder auto-logs in the devUser if those specific dev credentials are present. I worked around this by removing the devUser account, and manually creating an account with different credentials. That seems to let me play around with login/logout just fine...

When navigating to /admin/foobar when there is a dev account but the user just isn't logged in... the same issue is present.

After some playing around, It would seem a change like the one I tinkered with could address both sides of this issue.

Separately, my change appears to have awoken some sort of login redirection feature, but it does not appear to work as expected... which might be out-of-scope. I'm unsure if that should be addressed here and now, or if that would be a good candidate for a follow-up issue.

@akhrarovsaid
Copy link
Contributor

Hey @karlapsite,

Actually you're correct - the test folder has helper functions that triggers auto-login. Have a look below at the root of this functionality which seems to originate from buildConfigWithDefaults present in all the config.ts files in the test folder.

Auto login predicate in buildConfigWithDefaults
Login function helper
Default credentials

I hope this may help you with your proposed changes and tests. Cheers!

@paulpopus paulpopus changed the title Potentially missing redirect to create-first-user on bogus /admin/* routes Not found is being displayed within the admin panel styling for non authenticated users Oct 16, 2024
@paulpopus
Copy link
Contributor

Hey all, thanks for all the investigations here, I wouldnt touch the default login and such. The issue here is actually just that the notFound page is visible within the layout of the admin panel if you are not authenticated. It's not isolated to the create first user flow, so I updated the title

@karlapsite
Copy link
Author

karlapsite commented Oct 16, 2024

Yep... not trying to change default login... it just gave me a little trouble when debugging both cases.

I'm still curious if reordering redirection and notFound would address the issue.

@paulpopus
Copy link
Contributor

Thanks guys for the report here, went down a rabbithole with more functionality around this tested. This will be fixed in #8820

Non authorised users should be redirected to /login as per v2 behaviour in this situation.

paulpopus added a commit that referenced this issue Oct 30, 2024
…ith notFound page (#8820)

This PR aims to fix a few issues with the notFound page and custom views
so it matches v2 behaviour:
- Non authorised users should always be redirected to the login page
regardless if not found or valid URL
- Previously notFound would render for non users too potentially
exposing valid but protected routes and creating a confusing workflow as
the UI was being rendered as well
- Custom views are now public by default
- in our `admin` test suite, the `/admin/public-custom-view` is
accessible to non users but
`/admin/public-custom-view/protected-nested-view` is not unless the
checkbox is true in the Settings global, there's e2e coverage for this
- Fixes #8716
Copy link
Contributor

🚀 This is included in version v3.0.0-beta.121

Copy link
Contributor

github-actions bot commented Nov 1, 2024

This issue has been automatically locked.
Please open a new issue if this issue persists with any additional detail.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants