Repository navigation
[ENG-12611] Admin auth middleware - #11989
brianjgeiger merged 2 commits into
Conversation
c7680da to
03b7841
Compare
brianjgeiger
left a comment
brianjgeiger
left a comment
There was a problem hiding this comment.
Overall, this looks great. What I'd love to see is the all views vs. protected views vs. non-protected views be refactored into a utility somewhere that the test script calls, and also to have a new admin panel added that is called "Unprotected views" that will list all the unprotected views and maybe link to them if that's easy enough. This will prevent someone from marking a view as "login-not-required" without it being discoverable on the interface.
03b7841 to
43e5b7a
Compare
|
|
||
|
|
||
| def public_views(): | ||
| return {LoginView} |
There was a problem hiding this comment.
Instead of this, should we use the information from https://github.com/CenterForOpenScience/osf.io/pull/11989/changes#diff-96ba09f60ecd239a0ddc3a3c4c50ebb990141852057b3c8059a736ce11421676R45 to determine the public views, so we know those lists are synchronized?
From a practical perspective, it shouldn't matter too much, because this list will have to be updated to get tests to pass if a view is added that's unprotected, so we could let it through as is. Thoughts?
There was a problem hiding this comment.
If we add a view that is marked as a public view by accident, then the test would fail. If we make this getter fetch public views automatically, then it would rely on the login_required attribute of the view, which could be something done by mistake.
a904410
into
CenterForOpenScience:feature/pbs-26-20
Ticket
Purpose
Changes
Side Effects
QE Notes
CE Notes
Documentation