-
-
Notifications
You must be signed in to change notification settings - Fork 62
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
Add simple auth to all admin-only controllers #837
Conversation
@@ -67,7 +67,7 @@ def status | |||
elsif matches_as_provider.any? | |||
status = matches_as_provider.map{|m| m.completed?}.any? ? 'completed' : 'matched' | |||
end | |||
update_attributes(state: status) | |||
update(state: status) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gets rid of an annoying deprecation warning :)
@@ -13,7 +13,7 @@ | |||
location_attributes: { zip: '12e45' }, | |||
}} | |||
|
|||
before { sign_in create(:user) } | |||
before { sign_in create(:user, :admin) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe ListingController
is only currently used by admins?
6cbf215
to
fc07237
Compare
Inheriting from AdminController gives us rudimentary authorization based on user role (must be admin or sysadmin). See #835. Also changes one occurrence of the deprecated `update_attributes` -> `update`.
fc07237
to
050a237
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfecto! Some of these are likely to need to come out of this category, but until we have the relevant functionality, all of these should be restricted to admin/sys_admin only. Tks!
Why
Gives all admin-only controllers rudimentary authorization by inheriting from
AdminController
.See #835 for background.
Pre-Merge Checklist
Testing
This is part of the work to re-open user registration.
Once all that work is ready, we'll need to do some extensive manual regression testing.
Specific review requests
Most importantly, please check whether all these controllers are actually only for admin use.
Next Steps
?
Outstanding Questions, Concerns and Other Notes
?