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
cobrand hook to deny access to pages #2092
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2092 +/- ##
==========================================
- Coverage 78.7% 78.09% -0.61%
==========================================
Files 182 178 -4
Lines 11738 11493 -245
Branches 2181 2113 -68
==========================================
- Hits 9238 8976 -262
- Misses 1738 1798 +60
+ Partials 762 719 -43
Continue to review full report at Codecov.
|
40e1dcd
to
35f5a97
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.
Couple of small things.
@@ -39,6 +39,7 @@ sub auto : Private { | |||
|
|||
# decide which cobrand this request should use | |||
$c->setup_request(); | |||
$c->detach('/auth/redirect', []) if $c->cobrand->call_hook(check_login_disallowed => $c); |
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.
You don't need to pass in $c
here, it's available as $self->{c}
in a cobrand during a web request.
t/app/controller/root.t
Outdated
|
||
$mech->get('/'); | ||
is $mech->status, 302, 'disallowed page issues a redirect'; | ||
is $mech->res->headers->header('location'), 'http://localhost/auth?r=auth', 'redirects to auth page'; |
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.
Redirecting to /auth?r=auth
doesn't feel right; if you fetched the front page, shouldn't it have been /auth?r=
?
Add a check in the root controller `auto` to a cobrand hook that denies access if it returns true. This goes here so that cobrands and users are set up, which is not the case for `check_login_required`. Used to do things like deny site access unless the user is a superuser.
af8dfc9
to
85ae59f
Compare
Add a check in the root controller
auto
to a cobrand hook that deniesaccess if it returns true. This goes here so that cobrands and users are
set up, which is not the case for
check_login_required
. Used to dothings like deny site access unless the user is a superuser.