Hi, this is my setup:
Flag module (7.36)
Authcache
Authcache Ajax
Authcache Flag
I have created two flags:
- Subscribe user - users are not allowed to subscribe themselfes
- Like content - users are allowed to like content of themselfes and others
Thanks to Authcache Ajax flags are loaded via ajax.
When user visits his own content, both of the flags are not displayed, yet he should be able to likes his content.
I was able to find the reason of this action. The flags go to frontcontroller in certain order (this is important). The bug happens, when the first flag is Subscribe user. And I found the bug in the code.
First, frontcontroller gets handler and run handle method (file authcache/modules/authcache_p13n/includes/AuthcacheP13nDefaultRequestHandler.inc). Handle method executes build method and this invokes another build method - from contentBuilder, and this is in AuthcacheP13nFragmentAssemblyBuilder.inc
This is the code:
public function build($req, $context) {
global $user;
$result = array();
foreach ($req['a'] as $paramname => $params) {
// Skip when there is no handler present.
if (empty($this->partials[$paramname]['renderer'])) {
continue;
}
$partial = $this->partials[$paramname];
// Run loader.
if (!empty($partial['loader'])) {
$params = $partial['loader']->load($params, $context);
}
foreach ($params as $key => $subject) {
// Run access check.
if (!empty($partial['access']) && !$partial['access']->check($user, $key, $subject, $context)) {
throw new AuthcacheP13nRequestAccessDenied();
}
else {
$result[$paramname][$key] = $partial['renderer']->render($key, $subject, $context);
}
}
}
return $result;
}
There is foreach loop, which throws AccessDenied error with first flag that user is forbidden to use. But then it has impact on next flags as well - loop is interrupted.
Even with access denied for some of the flags, the result still shoud be returned. I believe this code:
// Run access check.
if (!empty($partial['access']) && !$partial['access']->check($user, $key, $subject, $context)) {
throw new AuthcacheP13nRequestAccessDenied();
}
else {
$result[$paramname][$key] = $partial['renderer']->render($key, $subject, $context);
}
should be changed to this:
// Run access check.
if (empty($partial['access']) || $partial['access']->check($user, $key, $subject, $context)) {
$result[$paramname][$key] = $partial['renderer']->render($key, $subject, $context);
}
with this code everything works.
Sadly i have no idea if this change has any bad impact on other functionality, but i provide a little patch if you find it useful.
Comment | File | Size | Author |
---|---|---|---|
#14 | multiple_flags_403_for-2711505-14.patch | 5.31 KB | znerol |
#14 | interdiff.txt | 1.25 KB | znerol |
#13 | interdiff.txt | 4.86 KB | znerol |
#13 | multiple_flags_403_for-2711505-13.patch | 4.86 KB | znerol |
#2 | multiple-flags-403-2711505-2.patch | 937 bytes | matysek145 |
Comments
Comment #2
matysek145 CreditAttribution: matysek145 commentedComment #3
matysek145 CreditAttribution: matysek145 commentedComment #12
znerol CreditAttribution: znerol commentedLooking into this now. I understand the problem and I agree on the solution. I am fixing tests now. One problem I see is that we are introducing an inconsistency here. I.e., the assembly builder still might short-circuit with a 404 if only one fragment fails to load. So I guess we also should protect against that case (and simply return empty fragments for unloadable content).
Comment #13
znerol CreditAttribution: znerol commentedLet's try this.
Comment #14
znerol CreditAttribution: znerol commentedModify test case such that it now mirrors exactly the situation reported in this issue.
Comment #16
znerol CreditAttribution: znerol commentedComment #18
znerol CreditAttribution: znerol commented