Not respecting extended group context
miro_dietiker - October 24, 2009 - 15:14
| Project: | OG User Roles |
| Version: | 6.x-4.0 |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Description
OGUR introduces dynamic per group context roles. Since og allows to set/extend the group context by module, ogur should always rely on it. This means at very first ogur should ask og_get_group_context for og context detection.
We have a contrib module to detect further group contexts and set them using og_set_group_context. If not relying on this API, no dynamic roles are present in the extended context.
As a quick fix i've added the og_get_group_context as a context fallback.
| Attachment | Size |
|---|---|
| ogur_context.patch | 761 bytes |
#1
+++ og_user_roles.module 24 Oct 2009 15:13:34 -0000@@ -123,6 +123,10 @@ function og_user_roles_init() {
+ // OG original context fallback
This inline comment needs more explanation about what is done here and why.
+++ og_user_roles.module 24 Oct 2009 15:13:34 -0000@@ -123,6 +123,10 @@ function og_user_roles_init() {
+ if(!$group_node) {
There should be a space between the if and the opening parenthesis. See http://drupal.org/coding-standards
Also, did you run the tests with this patch applied?
I'm on crack. Are you, too?
#2
#3
I've run into this same limitation. I'm trying to deploy this module on localize.drupal.org to make use of multiple user levels in groups. Currently, users are either admins or members of a group and their permission in the group to submit translations is dependent on this level. However, if you are not running og, the module has way more granular permissions. So our og model currently limits permissions to the og membership levels because we do not (yet) have in-og role controls. og_user_roles seems to be the way to go, but in our case, we actually need permission settings for our own tools based on og membership, so we need to have og_user_roles compatible with custom contexts.
Here is an updated patch. I've run the og_user_role tests, and got 87 passes and no exceptions/fails with this patch.
Manual verification of the patches site also proves that it indeed has an effect on menu items now showing up where a role is granted. :)
#4
Thanks!
Would it be possible to test this advanced context somehow? This patch is touching the core functionality of OGUR - which is privilege escalation - so we better make sure that this does not grant more than we want. ;)
#5
Better title.
#6
@sun: Well, og_init() has this code snippet:
<?php// Set group context and language if needed.
if ($group_node = og_determine_context()) {
og_set_theme($group_node);
og_set_group_context($group_node);
// TODOL: is this too late for menu links and such?
og_set_language($group_node);
}
?>
So what it does basically, is to use the context determination code which is very similar to OGUR's copy of the same og_determine_context() function. Then if that is determined, it sets that context. I did not find any other place og module would set the context (apart from views handlers).
Now OGUR has a copy of og_determine_context(), so if that gets us a context, it is already set in OGUR. It is only some "third party" code that sets the context if it is different in any way. Such as the views handlers in og_views. So we can write a mock module, which sets a context on an URL we pick and test for that. Better ideas?
#7
Hm. I now visually compared og_determine_context() and og_user_roles_determine_context(). There is no difference aside from a bug that pwolanin fixed (which probably should have been fixed in og.module as well, or better yet, first).
We only implement this fork, because we need to circumvent some statically cached access check results in the menu system (as described in the code docs). But aside from that, og_user_roles does not set a group context and it also shouldn't, because that is the responsibility of og.module. All we do is to use the very same logic as in og_determine_context() to determine the context of the current page, so we can assign additional user roles before the rest of the page logic runs. The immediate next step in the page logic is og_init(), which determines the context on its own and sets the context, if it is valid.