On 04/03/2012 07:50 AM, Imre Farkas wrote:
On 03/30/2012 03:50 PM, Scott Seago wrote:
> Rather than paste in the entire text, I'm just linking to the wiki
> writeup. Please read the whole thing first and if you want to comment on
> anything specific, quote it and comment on this thread.
>
>
>
https://www.aeolusproject.org/redmine/projects/aeolus/wiki/Adding_User_Gr...
>
>
>
> Scott
>
> In order not to tie conductor to LDAP for all installations, it
> should be a site-wide configuration whether groups come from LDAP or
> not. We'll need the conductor UserGroup/membership model anyway for
> joins in permission queries, so this configuration will simply
> determine how the group membership is maintained -- via LDAP hooks or
> via UI actions.
Is there any reason why we do not want to support both cases at the
same time? Probably it would be a better user experience to let the
user have both and indicate the source of a given group in the UI. If
we define 2 different scopes on UserGroup (user_defined,
ldap_imported), it won't be much pain to handle both of them.
Yeah on the code side it's not a lot more work to support both at all --
we'll just need to have a per-group flag in addition to global flags. I
wasn't sure we wanted to enable local groups when we were using ldap,
but with two global config flags we'd have the most flexibility. I.e.
global config flags "enable ldap user groups" and "enable local user
groups" -- ldap only set means we only do external groups, internal only
set means we don't do ldap groups, but with both set we allow both types.
> There are a couple of options available for keeping the
UserGroups in
> sync with LDAP groups:
> * Use delayed_job, cron, or something similar to re-sync on a
> specified schedule
> * Sync on use (with a pre-defined "minimum time between re-sync calls")
I would suggest a third approach: there could be some kind of auto
re-sync mechanism, and the user would also have to option to request
an update if he doesn't want to wait for the autoupdate to be called.
We can't rely on the user to say whether he wants to not wait for
auto-update, since the reason we're updating is to figure out if the
user has permission to view the page we're in the process of loading. If
we've got an auto-resync in place, we're done. We don't need to ever
block for user pages. The sync-on-use option is for the purpose of
avoiding the auto-resync.
I suspect we've got two very different opinions among the group, from
what I've seen so far. Some have said we should avoid polling
auto-resync if at all possible, even if it means blocking the occasional
page load to re-sync on demand. Others have said we should just
schedule re-sync and be done with it. I don't think we need a complex
hybrid of the two though -- either of the approaches will be sufficient,
but lets not try to do both at once.
> The main advantages of sync-on-use are that 1) we make the
minimum
> number of re-sync calls that still guarantees the data is fresh and
> 2) we don't require any external scheduling infrastructure. The main
> disadvantage is that the first permissions check after an idle period
> greater than the refresh interval (or one check per interval period
> if the server is under active use) will take longer than usual to
> return, as it has to wait for the LDAP group membership queries, any
> necessary writes for data changes, and the permission denormalization
> hooks for any changes to cascade down the object inheritance
> hierarchy. If this takes a long time with large datasets (many
> instances, images, users, etc), this will impact performance in a
> user-visible way.
To eliminate the downside of updating on user input, we could use
delayed_job for that, too: instead of re-syncing with the ldap server
during the request, we could just create a delayed_job task for that,
inform the user that the background job has been created through a
flash notice, and wait for the next available worker to pick up the
task and do the autoupdate asynchronously.
Imre
The point of the re-sync is to make sure group membership in the local
db is up-to-date so that we can perform permissions checks properly. It
would make no sense to load the user's page and then give a flash
message saying that the background job to re-sync permissions so we know
whether or not the user has permission to view the page we just showed
him. See above -- if we're doing it in the background, it needs to be
reasonably up-to-date at all times (meaning a polling over time). If
we're going to avoid polling (and thus have potentially out-of-sync
groups for a long time) then we've got to re-sync before doing the next
permissions check.
Scott