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

feat: frontend access controll #3100

Merged

Conversation

Kuchenpirat
Copy link
Collaborator

@Kuchenpirat Kuchenpirat commented Feb 2, 2024

What type of PR is this?

  • cleanup
  • feature

What this PR does / why we need it:

This PR adds middlewares to all pages that expect the user to be registered / or have special flags enabled (exept the individual cookbook page)

  • Admin (all pages are auth & admin only) already implemented via fix: admin pages accessible by non admin users #2988
  • g/_groupSlug
    • index.html (already implemented in RecipeExplorerPage)
    • cookbooks
      • _slug (depends on wheter cookbook is private or not) which is tricky because if the user does not have rights to get the cookbook, he will not get anything returned, so its not easy checking wheter he has the rights.
      • index
    • r
      • _slug
        • index (already implemented)
        • ingredient parser
      • create (incl all sub pages via create.vue)
    • recipes
      • organizers (categories, tags, tools)
      • timeline
    • shared (no restrictions needed)
  • group
    • index
    • members
    • migrations
    • notifiers
    • webhooks
    • data
    • mealplan
    • reports (i added only auth here, but realistically nobody would ever guess the right id, and even then the report would not get loaded and there is not much to do here, so it should be fine)
  • register (not needed)
  • shopping-lists
  • user (all pages)
  • password-forgot/reset-password/login (no auth needed)

Which issue(s) this PR fixes:

Fixes #3098
Partially Fixes #3083 (frontend only)
Partially Fixes #3049 (frontend only)
Fixes #3082

Special notes for your reviewer:

I had to add the middleware to most pages because the middleware will only affect all sub pages if there is a .vue file on the same level with the same name as the folder (see e.g. /group/data). I don't think it would be a huge problem
This will add frontend restrictions only, we should tighten some backend stuff and check more if the required user flags are set.

The only page that is not included is the page for individual cookbooks. But this would need a custom integration any way as we need to check whether the cookbook is private or not.

This does also disable access to the group settings page for any user that does not have the permission to manage group settings. We could also disable all inputs when the user does not have the right permission. Feedback on this is welcome.

I think this can be merged as it, as this is a substantial improvement and a finished product within itself.

Testing

Manually tested each page with users that should have access to it, and users that should not (if availlable).

@boc-the-git
Copy link
Collaborator

@Kuchenpirat should this still be draft? Looks like some to do items not done as yet.

@Kuchenpirat
Copy link
Collaborator Author

Kuchenpirat commented Feb 5, 2024

The only page that is not included is the page for individual cookbooks. But this would need a custom integration any way as we need to check whether the cookbook is private or not.

Not really a draft, but have to check out the conflicts once i am back home. (Should be easy tho)
This PR should solve this more broadly for all pages where a general solution middleware was useable. Which the cookbook page is not one of them.

@Kuchenpirat
Copy link
Collaborator Author

should be good to go again👍

@Kuchenpirat
Copy link
Collaborator Author

Kuchenpirat commented Feb 7, 2024

thanks @boc-the-git. I have updated the comments to reflect what the middleware is actually doing.

Copy link
Collaborator

@boc-the-git boc-the-git left a comment

Choose a reason for hiding this comment

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

Looks good mate, good enhancements 👍

@boc-the-git boc-the-git merged commit a7775ea into mealie-recipes:mealie-next Feb 18, 2024
9 checks passed
@Kuchenpirat Kuchenpirat deleted the feat-frontend-access-controll branch February 18, 2024 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants