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

Use Pundit for authorization #720

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nickcharlton
Copy link
Member

Authorization is the process of ensuring that users have access to the data that only they should access. It's a common flow found in many different Rails applications.

Pundit is takes the approach of using regular Ruby classes and OO design which makes it pleasant to use.

This recommendation comes after it's been used on many applications, including support included in Administrate.

https://github.com/varvet/pundit
https://administrate-demo.herokuapp.com/authorization

Authorization is the process of ensuring that users have access to the
data that only they should access. It's a common flow found in many
different Rails applications.

Pundit is takes the approach of using regular Ruby classes and OO design
which makes it pleasant to use.

This recommendation comes after it's been used on many applications,
including support included in Administrate.

https://github.com/varvet/pundit
https://administrate-demo.herokuapp.com/authorization
@nickcharlton
Copy link
Member Author

I had a discussion a week or so ago where this came up, and we realised it wasn't mentioned in the guides, even though it's been the go-to gem for several years.

This is the first gem recommendation in this file, though, but it's not something I would consider a good candidate for Suspenders, because you might not always need the solution.

@DoodlingDev
Copy link

Love pundit. 👍🏻

I might even push back on the idea that it's not for Suspenders even. Especially with authentication baked in to Rails 8.. and substantive application is going to need access control of some kind, otherwise one is probably ignoring a lot of the rest of Suspenders anyway

@JoelQ
Copy link
Contributor

JoelQ commented Feb 17, 2025

Agreed with @DoodlingDev, I think this would probably be a good fit in suspenders. Especially since we now have a generator-based architecture so it can be optional. Do we have a general sense that gem recommendations belong over there? Thoughts @stevepolitodesign?

@JoelQ
Copy link
Contributor

JoelQ commented Feb 17, 2025

I could see a lot of value documenting our best practices around authorization though!

@stevepolitodesign
Copy link
Contributor

Agreed with @DoodlingDev, I think this would probably be a good fit in suspenders. Especially since we now have a generator-based architecture so it can be optional. Do we have a general sense that gem recommendations belong over there? Thoughts @stevepolitodesign?

For now, we're trying to keep Suspenders as close to rails new as possible.

Although most applications will benefit from authorization tooling, I'm not sure if it's needed on day 0.

For example, you can get pretty far with scoping queries to the current_user

# Bad
@project = Project.find(params[:id])

# Good
@project = current_user.projects.find(params[:id])

@@ -54,6 +54,9 @@
- [Use blocks](/ruby/sample_2.rb#L10) when declaring date and time attributes in
FactoryBot factories.
- Use `touch: true` when declaring `belongs_to` relationships.
- Use [Pundit][] when you need to restrict access to models and data.
Copy link
Contributor

Choose a reason for hiding this comment

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

When you are developing an application with Pundit it can be easy to forget to authorize some action. People are forgetful after all. Since Pundit encourages you to add the authorize call manually to each controller action, it's really easy to miss one.

Would it be too prescriptive to enforce that policies and scopes are used

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm in favor of that! Although you should be doing one or the other. I've worked in a codebase that does both on every action and it leads to lots of weird code, unnecessary nil, and skipping the before actions 😭

Copy link
Contributor

@laicuRoot laicuRoot left a comment

Choose a reason for hiding this comment

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

@nickcharlton
Copy link
Member Author

For now, we're trying to keep Suspenders as close to rails new as possible.

Although most applications will benefit from authorization tooling, I'm not sure if it's needed on day 0.

A certain internal application of ours hasn't had any authorization aside from scoping queries until this week!

I think it's something I'd add quite soon, but not immediately in a new application. I wonder if we've got a specific way we'd like to set it up, it would be a good candidate for a generator on Suspenders, just one that we don't run by default?

@stevepolitodesign
Copy link
Contributor

I wonder if we've got a specific way we'd like to set it up, it would be a good candidate for a generator on Suspenders, just one that we don't run by default?

I'd be curious to explore this in a general sense. Things like this might pair well with The Guides.

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.

8 participants