So, we have the following issues:

  • Not all admins will want or require to manage their (CVS) account passwords with Drupal, as it needs to be hooked up with the VCS - that assumes that you have the permission to manage those accounts at all! Furthermore, it won't be necessary if you just want to use the "viewer" functionality of a backend.
  • Subversion, Git and Mercurial use the same kind of user authentication systems - HTTP(s), or SSH (probably with an .ssh_keys based solution). Subversion also has an additional "svnserve" possibility, so backend specific and backend agnostic authentication methods need to co-exist side by side.

So let's make those authentications pluggable. What constitues an authentication method plugin?

  • Storing the password that the user has entered in the registration form.
  • And based on that, import/export functionality.

The idea is to let authentication method plugins register themselves with Version Control API (much like hook_versioncontrol_backends()) and let backend modules specify which authentication methods they support, as additional info that we get from hook_versioncontrol_backends(). The Version Control API will determine if those plugins exist, and let the admin choose which authentication methods should be enabled for each repository. On the register page, the user can then select which one of the predefined authentication methods he wants to use (HTTPs or SSH, for example), and if no method has been enabled by the admin then the user doesn't get to enter a password at all.

Authentication methods will be specified with unique string identifiers that they pass with the registration hook. More details (or a finished implementation in the first place) to follow soon.

Comments

jpetso’s picture

Version: 5.x-1.1 » 5.x-2.x-dev

Oops, still not implemented in 5.x-2.x? Well, seems I was a little too confident. Still important, because a) the VCS account "subsystem" needs to be improved anyways, and b) account synchronization for anything else but CVS is pretty much impossible with the current system. Guess I should have outlined my design with a bit more detail (of course I forgot nearly everything since I wrote the original issue), but then again it should still not be all too difficult to just separate authentication from Drupal/VCS account associations. Input, help & everything appreciated.

jpetso’s picture

Oh hey, and I totally forgot to link to my analysis of this issue (and the subsequent findings in the comments section): http://groups.drupal.org/node/8527

marvil07’s picture

Version: 5.x-2.x-dev » 6.x-1.0-rc2

Just to point dhax review: http://groups.drupal.org/node/23464 really related and where it _is_ proposed a solution :-D

marvil07’s picture

Version: 6.x-1.0-rc2 » 6.x-2.x-dev
marvil07’s picture

sdboyer’s picture

Issue tags: +git sprint 5

Ha, look at this, this is pretty much exactly one of the issues I need to open in order to split up #965890: [meta] Revisit VersioncontrolAccount and bring it up to speed.

However, Jacob's original description of what we need is still focused on the old central command-and-control vcapi paradigm, which we've thoroughly established to be the wrong route.

So to be clear, we like this part:

So, we have the following issues:

  • Not all admins will want or require to manage their (CVS) account passwords with Drupal, as it needs to be hooked up with the VCS - that assumes that you have the permission to manage those accounts at all! Furthermore, it won't be necessary if you just want to use the "viewer" functionality of a backend.
  • Subversion, Git and Mercurial use the same kind of user authentication systems - HTTP(s), or SSH (probably with an .ssh_keys based solution). Subversion also has an additional "svnserve" possibility, so backend specific and backend agnostic authentication methods need to co-exist side by side.

So let's make those authentications pluggable. What constitues an authentication method plugin?

  • Storing the password that the user has entered in the registration form.
  • And based on that, import/export functionality.

But the rest is kinda stuck in the old, centralized command-and-control thinking than we like:

The idea is to let authentication method plugins register themselves with Version Control API (much like hook_versioncontrol_backends()) and let backend modules specify which authentication methods they support, as additional info that we get from hook_versioncontrol_backends(). The Version Control API will determine if those plugins exist, and let the admin choose which authentication methods should be enabled for each repository. On the register page, the user can then select which one of the predefined authentication methods he wants to use (HTTPs or SSH, for example), and if no method has been enabled by the admin then the user doesn't get to enter a password at all.

...maybe. I need to ponder on this a bit. In any case, though, this is one of the biggies we're tackling in sprint 5.

marvil07’s picture

sdboyer also point me to User authentication in non-CVS repositories as a good reading, linking here.

sdboyer’s picture

Quick thing that lent me some clarity. Whatever we do, it should keep in mind that there are two discrete steps with auth:

  1. Determining if the provided credentials successfully authenticate as a particular Drupal user.
  2. Determining if the authenticated Drupal user has access, and what kind of access, to a particular repository.

The implementation details of these ought not be the concern of VCAPI (as much as possible) - we just care about checking the outcomes.

sdboyer’s picture

Assigned: Unassigned » sdboyer
Priority: Normal » Critical

This is on me to work on until tomorrow, after which it'll be marvil07's.

marvil07’s picture

tagging

greg.1.anderson’s picture

I have had really good luck with organic groups, og subgroups, and particularly og user roles to control access to external Apache resources by way of the mod_auth_mysql module. Using these same techniques, I could easily imagine using information from the og tables to write out an authz_svn access files; this could also be relevant to other VCSs that support Apache authentication.

There are a lot of modules in the og suite; building an access control model with these would be preferable to sticking with the existing VCS API "motivation texts" mechanism, as og provides a lot of flexibility in configuration.

sdboyer’s picture

@greg.1.anderson - thanks for chiming in here. If we retain the mapping table at all, it'll be as part of a plugin - in no way a required part of the system. And re: your comment #965890-13: [meta] Revisit VersioncontrolAccount and bring it up to speed, we're currently planning on getting rid of the VersioncontrolAccount class entirely.

sdboyer’s picture

StatusFileSize
new18.3 KB

OK, this patch is VERY much in-progress, but I want to get it out there pre-review so we at least have something.

Basically, we introduce a method, VersioncontrolRepository::getAuthHandler() that gets an auth plugin. These plugins must implement an interface which answers all possible auth-type questions. I've got one fallback, "free-for-all" plugin (which just says "yes!" to every auth check), and I'm partway through a much more complicated plugin that handles granular, per-repo/per-user auth. I'm intending that plugin to sit at the head of a family of "account" auth plugins, as there are a lot of use cases where you might want to extend and slightly tweak its logic (vc_project probably being one). There are also a bunch of schema changes pursuant to the account plugin - and if you look, you'll notice it's gonna be pretty easy for us to do per-branch ACLs! the much more challenging part will be the UI for it :)

Before this goes in, I want to at LEAST write another plugin that does auth based on user roles. Chances are that for d.o, we're going to want a combo of the accounts-based and role-based approaches, where we let certain global role settings short-circuit all the db-based auth. That'll allow us to easily flip a switch that disables someone's git access, or (much less likely, if we even want it at all) gives someone access to all repos.

my local changelog for the attached patch:

commit ec9b81e4773cd6f5debcbaa2d8ce0c2d31d4fcf9
Author: Sam Boyer <drupal@samboyer.org>
Date:   Thu Dec 2 17:40:47 2010 -0800

    Remove possibility of potentially confusing errors resulting from no
    plugin being set when VersioncontrolRepository::getAuthorMapper() is
    called.

commit 660e79769666067e506595e3d05bdbe5a142015a
Author: Sam Boyer <drupal@samboyer.org>
Date:   Thu Dec 2 17:57:12 2010 -0800

    Add VersioncontrolRepository::getAuthorizationHandler() method and
    introduce VersioncontrolAuthorizationHandlerInterface to keep track of
    methods plugins will need to implement.

commit 0ad359f603460a1f70319f34a6b23ae379fea895
Author: Sam Boyer <drupal@samboyer.org>
Date:   Fri Dec 3 01:09:48 2010 -0800

    Change 'Authorization' to 'Auth' in method name and interface name, as
    it just seems nicer.

commit e89b278927c8bd30c2e8fe1771f899853efacced
Author: Sam Boyer <drupal@samboyer.org>
Date:   Fri Dec 3 01:10:16 2010 -0800

    Add a bunch of methods to VersioncontrolAuthHandlerInterface.

commit 3f83e94728f33ffe93ac84b41f41deceae328ce2
Author: Sam Boyer <drupal@samboyer.org>
Date:   Fri Dec 3 01:10:43 2010 -0800

    Rename auth plugin type from 'authorization' to 'vcs_auth', and
    introduce the first plugin, 'FFA', which just lets everything through.

commit 8b9ca6eaee9158ad7cee51121ad9bccd91748c10
Author: Sam Boyer <drupal@samboyer.org>
Date:   Fri Dec 3 01:21:34 2010 -0800

    Change VersioncontrolAuthHandlerInterface::authAnyWrite() to
    VersioncontrolAuthHandlerInterface::authBase(), as it's a little clearer
    what the method is actually for.

commit a8d179685cb0c66b58c016aae02a8b82f1eb0d50
Author: Sam Boyer <drupal@samboyer.org>
Date:   Fri Dec 3 09:25:15 2010 -0800

    Initial work on the 'accounts' auth plugin, which should be the basis
    for a family of plugins that do db-based granular control of repo
        access.

commit 989aa042f926c5d4328af43e7f45ca7b96eaf1df
Author: Sam Boyer <drupal@samboyer.org>
Date:   Fri Dec 3 09:27:42 2010 -0800

    Update schema to reflect new pluggable auth system.
    
    This kills the {versioncontrol_accounts} table, and introduces a
    {versioncontrol_auth_account} table and
    {versioncontrol_auth_account_label} table to support the
    VersioncontrolAuthHandlerMappedAccounts family of auth plugins.

commit 3f53fc212742e4f1d7866c57c3cabef6ce047be9
Author: Sam Boyer <drupal@samboyer.org>
Date:   Fri Dec 3 10:01:21 2010 -0800

    Update VersioncontrolAuthHandlerInterface to take user arguments rather
    than setting a single user as a property.
    
    Update plugin instances to reflect these changes, as well.

commit b22d0c2aaba516b62a69c21a5fb53ada9e0caeaa
Author: Sam Boyer <drupal@samboyer.org>
Date:   Fri Dec 3 10:01:57 2010 -0800

    Update {versioncontrol_auth_account_label} with some slightly saner
    field names.

Also, it's important to note that in my original version, I was expecting to pop off lots of these auth plugins, so VersioncontrolRepository::getAuthHandler() doesn't cache them. With the architecture of the account auth plugin family the way it is, though, it's really much better if we do cache them. That's one of the first things on my list to follow up and change.

sdboyer’s picture

Status: Active » Needs work
sdboyer’s picture

PS - there's clearly UI and CRUD considerations that haven't been touched on at all thus far in the account auth plugin family. CRUD is pretty easy, UI's a little more tricky. Probably going to use either some additional interfaces, or some properties on the plugin definition.

greg.1.anderson’s picture

I like the direction this is taking. It looks like it is easy to provide a simple "pass-through" VersioncontrolAuthHandlerInterface that just assumes that the drupal name == the vcs name.

Regarding the UI for per-branch access control, I recommend setting up versioncontrol_project to allow a single repository + branch to be associated with each project, just like it is today in the vc_1.x API. Each project needs a "Role to read" and a "Role to write". Really, you can use just one role for each, "vcs_read" and "vcs_write", and have all the control you need. Let me explain how; this is an expansion of what I was saying in #11.

1. If a (drupal) user has the "vcs_read" role, then they can read any branch of any available repository.

2. If a suer has the "vcs_write" role, then they can also globally write.

3. In order to provide per-branch access control, you must create a project and set the repository and branch of that project appropriately. Access control is based on projects. I would also recommend adding controls in versioncontrol_project to specify which specific role should be used for "vcs_read" and "vcs_write" for better control in the non-fine-grained, non-og case.

4. For a given project, either the project_project type must be a group node, or if it is not, the project must have its audience set to some group. Otherwise, only people with global access (those who have the "vcs_read" / "vcs_write" role).

5. The module og_user_roles can then be used to add vcs_read and vcs_write as roles that can be granted on a per-group basis. If you add these roles to all groups, then the group admin can decide who has the vcs_read and vcs_write role for their group. If a user already has the vcs_read / vcs_write role, that fact will take precedence over what group admins do.

I have already successfully done 1, 2, 4 and 5, and I'm working on temporarily adapting vc_project in the 1.x api to provide #3 without having to create vcs users for everyone. It would be possible and more Drupal-like to use a permission rather than a role for vcs_read / vcs_write, but I haven't tried that yet. (It should be pretty much the same, just somewhat more complicated SQL.) I'd like to help out here so this same model can be used in the 2.x branch, which I'd like to migrate to when it's feasible for me to do so.

I think that 1-3 is essentially exactly what you were already planning on implementing in #13 (paragraph 2), minus the code to provide a UI for setting access control for individual accounts.

I would like to make the pitch that versioncontrol_api should require og for branch-level access. Without og, you can provide global read only or global read/write access to all repositories. Fine-grain access is as easy to add as turning on og and og_user_roles.

As I mentioned before, og already has a great API for creating groups, assigning administrators to the group, letting the administrator have control over their group (without giving them control over other people's groups), and there are also a lot of og-related modules to allow you to fine-tune the way your groups behave. It would just confuse the versioncontrol_api interfaces to add a separate UI for this; plus, that's a lot of extra code you'd have to write just to replicate og functionality. I don't see any benefit to trying to make a "simpler" "og-free" access control UI.

Here's my proposal / offer to go with my pitch: if you implement role-based global access control as you describe in #13, then I'll submit a patch + documentation that converts it into fine-grain access with og and og_user_roles. I'm presuming you're going to hook up all of the VersioncontrolBranch stuff as you describe above, so that each branch is already doing role-based access checks.

greg.1.anderson’s picture

Oh, one thing that #16 leaves open is, who has the right to add a repository + branch to a project? In the short term, I would do this with a Drupal permission. Anyone with this permission could potentially compromise security by making a new project whose branch is a subbranch of an existing project; this user would then gain control of all of the vcs paths below that point.

It is an interesting question to define what branches a partially-privileged user should be allowed to attach to a project. Maybe the branch should be required to be a subbranch of an existing project that the user is an og administrator of. Then you could make meta-projects that define where your section admins are allowed to create subprojects. That would work reasonably well, and wouldn't require a new complicated UI form.

marvil07’s picture

+++ includes/VersioncontrolRepository.php
@@ -499,6 +499,25 @@ abstract class VersioncontrolRepository implements VersioncontrolEntityInterface
+    $obj = $this->getPluginClass('auth_handler', 'vcs_auth', 'handler');

Just to be consistent with user_mapping_methods it would be better to use user_auth_methods.

+++ versioncontrol.install
@@ -858,3 +935,130 @@ function versioncontrol_update_6308() {
+  db_drop_table($ret, 'versioncontrol_accounts');

IMO this is not the right place to remove {versioncontrol_accounts} table, since it completely breaks versioncontrol_account_status module.

Maybe is better to deal with #983926: Remove account class before follow working here since that seems to me like the right place to remove it.

Well, I need to take a more closely look to this.

Powered by Dreditor.

sdboyer’s picture

I don't actually like user_auth_methods, but I wasn't going to nitpick :P More importantly, though, I don't think it's quite as accurate to call it an "auth method" - it's an entire handler that does potentially quite a lot, including furnishing some UI, etc.

sdboyer’s picture

@greg.1.anderson #16

I would like to make the pitch that versioncontrol_api should require og for branch-level access. Without og, you can provide global read only or global read/write access to all repositories. Fine-grain access is as easy to add as turning on og and og_user_roles.

As I mentioned before, og already has a great API for creating groups, assigning administrators to the group, letting the administrator have control over their group (without giving them control over other people's groups), and there are also a lot of og-related modules to allow you to fine-tune the way your groups behave. It would just confuse the versioncontrol_api interfaces to add a separate UI for this; plus, that's a lot of extra code you'd have to write just to replicate og functionality. I don't see any benefit to trying to make a "simpler" "og-free" access control UI.

Here's my proposal / offer to go with my pitch: if you implement role-based global access control as you describe in #13, then I'll submit a patch + documentation that converts it into fine-grain access with og and og_user_roles. I'm presuming you're going to hook up all of the VersioncontrolBranch stuff as you describe above, so that each branch is already doing role-based access checks.

re: requiring og - so, my first reaction is a kneejerk hell no :) As I read the rest, though, you make some interesting points - certainly something to consider. I would have to pretty much abandon the approach I've started to take here to introduce an actual og dependency, though, so I don't think that will happen. But I'd be perfectly happy with another 'family' of auth plugins that was og-centric. And I'm definitely open to being convinced of more.

Oh, one thing that #16 leaves open is, who has the right to add a repository + branch to a project? In the short term, I would do this with a Drupal permission.

I think you might need to look at the patch again. But there are several answers to that - first, versioncontrol itself has nothing to do with deciding what repos can go on projects. That's all versioncontrol_project. And this patch is really focused on the question of authorizing manipulations of repo data once it's already been created, but punting the responsibility for just what auth logic gets used to whoever (e.g., versioncontrol_project) 'owns' the repository, via these auth plugins.

Anyone with this permission could potentially compromise security by making a new project whose branch is a subbranch of an existing project; this user would then gain control of all of the vcs paths below that point.

I don't follow this at all, primarily because I don't understand what a 'subbranch' is, or how that would cause any sort of collision. I think you're thinking the datastructures work/interact in some way that they don't :) Sounds maybe like you're still thinking in the old, super-size all-inclusive CVS repository mode? Not how things work going forward - _every_ project has its own repository, and in the new project creation workflow, new repo creation is triggered, there's no path selection. See esp. #961144-73: Determine/finalize technical requirements for post-Git migration project approval process.

In any case, we should talk more. I actually kinda have a hankering to get you on the phone/skype to chat about this :)

greg.1.anderson’s picture

You're right, I did drift a little bit off-topic in #16; sorry. I do understand that it is plugable, I just wandered down into implementation-specific details (e.g. versioncontrol_project).

The same is true about my og comments. I did not mean to imply that versioncontrol_api should depend on og; it shouldn't. What I meant to say was that I don't see any advantage to implementing a non-og UI for branch-level access control. If the primary branch-level access control project is not og, then I could certainly pull together a 'family' of auth plugins that was og-centric as you suggest, so it's all good.

You are correct in your final points. I cannot use git. I must use subversion. Some of my projects will have their own repository, some must be a branch of an existing repository. In this respect, some of my comments (esp. #17) are only my problem, and do not apply to the standard git implementation.

sdboyer’s picture

OK, that clarifies a lot. And is actually great - we are trying very hard to take care that vcapi stays a real API, but having somebody actually interested in a backend other than git is an excellent counterpoint to where the migration team's focus is. Now, as to:

What I meant to say was that I don't see any advantage to implementing a non-og UI for branch-level access control.

Maybe it's just that I'm not familiar with the UI you're talking about, but I don't see how there could be such a categorical win here. The basic 'account' family of plugins I'm working on is entirely structured around interacting with rights granted to individual users, not group-based rights. Which means the UI we need to present is one where, on a per-user basis, admin users can set those users' branch- and tag-level rights on an individual basis. The og use case is, for what I think are obvious reasons, interesting and compelling. But I don't see how its UI could be of benefit to the individual accounts case - it is, after all, based on group-related datastructures which aren't present or relevant in the basic single account case. I'm sure we could get a lot of ideas for how to put together that UI for the individual accounts case by looking at what's been done with og, sure, but I don't understand how we'd actually _use_ it.

If you did want to embark on writing a groups-based auth plugin for vcapi, I'd love to be kept in the loop and help with it as I can. And as for the subdirectories ACL issue, I don't think that needs necessarily be a concern, either. In the past, the svn backend has overlaid the concept of branches onto the directory structure in an svn repository, and could probably do the same thing with projects. Should be able to build proper access control enforcement around that without too much difficulty.

greg.1.anderson’s picture

Cool, I got it -- you have more permissions to manage than I do. For now I'm just worried about "in" and "not in", so og makes a lot of sense.

sdboyer’s picture

A beautiful example of why this system ought to be pluggable :) Let's make sure to connect next week, though - I'd like to become as familiar as reasonably possible with your use case. There's that great rule about an API not _really_ being an API until it works well for at least two, preferably three, differing implementations...

sdboyer’s picture

StatusFileSize
new0 bytes

Another round on this patch; I believe this one should complete the lower-level mechanics. The permissions for interacting with repositories are potentially very verbose, though, and could use some sanity checking to ensure a) they're not too verbose (I think there's valid use cases for everything I have in there) and b) the logic is actually correct. I spent a couple hours tearing at my hair, trying to figure out the most efficient way to write the checking algorithms, and I may well have made mistakes.

Which leaves...the UI questions. Ugh. I'm still undecided as to how much really ought to live in this plugin, and how much ought to be other systems decorating it.

Also, re: #18, I reverted the removal of {versioncontrol_accounts}.

sdboyer’s picture

StatusFileSize
new20.88 KB

Whoops, empty patch file.

sdboyer’s picture

Issue tags: +git sprint 6

tagging for sprint 6

sdboyer’s picture

StatusFileSize
new25.73 KB

OK, posting an incremental patch here that does introduces SOME of the tests we need to verify the plugin actually works like we expect. That means testing all the plugin's methods - basically, CRUD and the VersioncontrolAuthHandlerInterface. marvil07 is gonna pick those up, see if he can't finish it, while I ponder and think if there are any other parts of this that need to go in before being committed.

Related to that, we're going to punt on the UI. See #992074: Decide if & what UI to provide for authentication plugins.

marvil07’s picture

StatusFileSize
new26.03 KB
new5.77 KB

I solved some little problems:

- add second parameter to db_select() calls at VersioncontrolAuthHandlerMappedAccounts class
- save auth account data to DB and compare it to the inserted dummy data with two asserts.

but I end up on a question that is going to be quicker to answer for you: versioncontrol_auth_account schema information is inconsistent(see hook_schema(), hook_update_6309() and the test array $super_user_data at VersioncontrolAccountAuthPlugin::testAuthPluginCrud()), so what is the expected right data for that table?

sdboyer’s picture

Yeah, I caught that second parameter to the db_select() later last night, too. I'm not sure what you mean by schema inconsistencies, but the question about what the data is SUPPOSED to look like is legit - I haven't written that down anywhere yet :) Here's a pastebin of a quick test that I wrote which verifies that all the logic actually works correctly. Which is one of the big blockers.


ctools_include('plugins');
$repo = versioncontrol_repository_load(15);
$class_name = ctools_plugin_load_class('versioncontrol', 'vcs_auth', 'account', 'handler');
$authplug = new $class_name();
$authplug->setRepository($repo);

$super_user_data = array(
  'access' => VersioncontrolAuthHandlerMappedAccounts::GRANT,
  'branch_create' => VersioncontrolAuthHandlerMappedAccounts::DENY,
  'branch_update' => VersioncontrolAuthHandlerMappedAccounts::GRANT,
  'branch_delete' => VersioncontrolAuthHandlerMappedAccounts::GRANT,
  'tag_create' => VersioncontrolAuthHandlerMappedAccounts::DENY,
  'tag_update' => VersioncontrolAuthHandlerMappedAccounts::GRANT,
  'tag_delete' => VersioncontrolAuthHandlerMappedAccounts::GRANT,
  'per-label-auth' => array(
    204 => array(
      'update' => VersioncontrolAuthHandlerMappedAccounts::GRANT,
      'delete' => VersioncontrolAuthHandlerMappedAccounts::GRANT,
    ),
    205 => array(
      'update' => VersioncontrolAuthHandlerMappedAccounts::GRANT,
      'delete' => VersioncontrolAuthHandlerMappedAccounts::DENY,
    ),
    206 => array(
      'update' => VersioncontrolAuthHandlerMappedAccounts::DENY,
      'delete' => VersioncontrolAuthHandlerMappedAccounts::GRANT,
    ),
    207 => array(
      'update' => VersioncontrolAuthHandlerMappedAccounts::DENY,
      'delete' => VersioncontrolAuthHandlerMappedAccounts::DENY,
    ),
    216 => array(
      'update' => VersioncontrolAuthHandlerMappedAccounts::GRANT,
      'delete' => VersioncontrolAuthHandlerMappedAccounts::GRANT,
    ),
    217 => array(
      'update' => VersioncontrolAuthHandlerMappedAccounts::GRANT,
      'delete' => VersioncontrolAuthHandlerMappedAccounts::DENY,
    ),
    218 => array(
      'update' => VersioncontrolAuthHandlerMappedAccounts::DENY,
      'delete' => VersioncontrolAuthHandlerMappedAccounts::GRANT,
    ),
    219 => array(
      'update' => VersioncontrolAuthHandlerMappedAccounts::DENY,
      'delete' => VersioncontrolAuthHandlerMappedAccounts::DENY,
    ),
  ),
);

$branches = $repo->loadBranches(array(204, 205, 206, 207));
$tags = $repo->loadTags(array(216, 217, 218, 219));

$authplug->setUserData(1, $super_user_data);
// var_dump($authplug);

$ret = array();
$ret['global auth'] = $authplug->authAccess(1);
$ret['branch create'] = $authplug->authBranchCreate(1);
$ret['tag create'] = $authplug->authTagCreate(1);
$ret['branch update 204'] = $authplug->authBranchUpdate(1, $branches[204]);
$ret['branch delete 204'] = $authplug->authBranchDelete(1, $branches[204]);
$ret['branch update 205'] = $authplug->authBranchUpdate(1, $branches[205]);
$ret['branch delete 205'] = $authplug->authBranchDelete(1, $branches[205]);
$ret['branch update 206'] = $authplug->authBranchUpdate(1, $branches[206]);
$ret['branch delete 206'] = $authplug->authBranchDelete(1, $branches[206]);
$ret['branch update 207'] = $authplug->authBranchUpdate(1, $branches[207]);
$ret['branch delete 207'] = $authplug->authBranchDelete(1, $branches[207]);
$ret['tag update 216'] = $authplug->authTagUpdate(1, $tags[216]);
$ret['tag delete 216'] = $authplug->authTagDelete(1, $tags[216]);
$ret['tag update 217'] = $authplug->authTagUpdate(1, $tags[217]);
$ret['tag delete 217'] = $authplug->authTagDelete(1, $tags[217]);
$ret['tag update 218'] = $authplug->authTagUpdate(1, $tags[218]);
$ret['tag delete 218'] = $authplug->authTagDelete(1, $tags[218]);
$ret['tag update 219'] = $authplug->authTagUpdate(1, $tags[219]);
$ret['tag delete 219'] = $authplug->authTagDelete(1, $tags[219]);
var_dump($ret);

Output from that looks like this:

array(19) {
  ["global auth"]=>
  bool(true)
  ["branch create"]=>
  bool(false)
  ["tag create"]=>
  bool(false)
  ["branch update 204"]=>
  bool(true)
  ["branch delete 204"]=>
  bool(true)
  ["branch update 205"]=>
  bool(true)
  ["branch delete 205"]=>
  bool(false)
  ["branch update 206"]=>
  bool(false)
  ["branch delete 206"]=>
  bool(true)
  ["branch update 207"]=>
  bool(false)
  ["branch delete 207"]=>
  bool(false)
  ["tag update 216"]=>
  bool(true)
  ["tag delete 216"]=>
  bool(true)
  ["tag update 217"]=>
  bool(true)
  ["tag delete 217"]=>
  bool(false)
  ["tag update 218"]=>
  bool(false)
  ["tag delete 218"]=>
  bool(true)
  ["tag update 219"]=>
  bool(false)
  ["tag delete 219"]=>
  bool(false)
}
marvil07’s picture

StatusFileSize
new26.59 KB
new6.04 KB

Ok, still needs to finish the test, but this:

    - Actually use mapped account plugin at repo creation on the mapped
      account auth plugin test.
    - Unify schema for {versioncontrol_auth_account}
    - minor: Add auth_handler as one possible plugin.
    - Do not use array_merge_reursive for repo plugins data member.
      If we do so, we end up with two plugins for a given plugin_slot
      instead of one plugin.(recursive join both arrays instead of set one
      of them)
    - Force returning an array(not object which is the default) for
      VersioncontrolAuthHandlerMappedAccounts::userData, which is what we
      are assuming.
sdboyer’s picture

Ah, and I forgot - I have a skitch image describing the way I'm imagining the auth logic flow works internally for the 'account' plugin:

Only local images are allowed.

sdboyer’s picture

StatusFileSize
new25.55 KB

OK, another incremental patch. I think we're actually pretty much there, since we're punting on UI. What needs to be added now is documentation. Here's the commit log:

 55962fe (master) Removed explicit file location from simple_mail plugin; ctools figures it out correctly if it's not there, but chokes if it is.
 1bd9995 Add VersioncontrolRepository::getAuthorizationHandler() method and introduce VersioncontrolAuthorizationHandlerInterface to keep track of methods plugins will need to implement.
 8873ba6 Change 'Authorization' to 'Auth' in method name and interface name, as it just seems nicer.
 d573c73 Add a bunch of methods to VersioncontrolAuthHandlerInterface.
 0f6db26 Rename auth plugin type from 'authorization' to 'vcs_auth', and introduce the first plugin, 'FFA', which just lets everything through.
 09bd721 Change VersioncontrolAuthHandlerInterface::authAnyWrite() to VersioncontrolAuthHandlerInterface::authBase(), as it's a little clearer what the method is actually for.
 ffcf1b1 Initial work on the 'accounts' auth plugin, which should be the basis for a family of plugins that do db-based granular control of repo     access.
 7449ddb Update schema to reflect new pluggable auth system.
 2b9bd63 Update VersioncontrolAuthHandlerInterface to take user arguments rather than setting a single user as a property.
 3fa2db5 Update {versioncontrol_auth_account_label} with some slightly saner field names.
 be09697 Cache instanciated auth plugin on the VersioncontrolRepository object in VersioncontrolRepository::getAuthHandler().
 ffcafe0 Remove explicit file declarations from plugins, they make ctools bork without full paths. Fine to do this since our plugins follow the naming convention.
 5f39b74 May be premature to remove {versioncontrol_accounts} table, so putting it back in.
 1da3ac3 VersioncontrolAuthHandlerInterface shouldn't require a VersioncontrolBranch or VersioncontrolTag for the respective create checks.
 a2937d2 Finish up the auth-checking logic in the accounts plugin. It's very verbose, possibly too much so.
 b75ee9a Add save method and getters and setters for user data.
 ba6d1cb The auth plugin fetcher was looking for the wrong type of plugin.
 fd4a4d4 Small tweaks and docblock additions to the accounts plugin.
 197a477 Initial sketchy work on tests for the account auth plugin.
 280d9f6 Turns out you can't type-hint 'object' and have it work for stdClass. Stupid PHP.
 49e6e3e Fix the queries to specify the table alias, as all queries were failing before we did that. Also some other unrelated, minor tweaks.
 4aed705 Remove the defunct perms bitmask, and update the error-setting to not use $user-name since we're just doing uids now.
 65a0fd7 (HEAD, pluggable-auth) Integrate assorted incremental changes from marvil07.
marvil07’s picture

I end up with a weird error at tests, so posting an update based onthe last patch that:

- Re-insert lost changes in the patch at #223891-33
- minor: account auth plugin test: add uid for the comparison(db info autoreturns it, but we set it at setUserData() not in the array passed)
- Add methods to make it easier to create branches and tags on tests.
- Convert sdboyer #223891-30 test into repeatable test.

sdboyer’s picture

StatusFileSize
new19.77 KB
new41.77 KB

New patch, sorted out the db issue (reserved field names were causing errors, evidently). All tests but one are now passing, leaving that one to marvil07 to sort out. New patch also adds a metric ton of documentation to the auth plugin.

Once we get that last test fixed, I think we're RTBC.

marvil07’s picture

Status: Needs work » Needs review
StatusFileSize
new41.81 KB
new1.32 KB

Last test not passing fixed!

sdboyer’s picture

Status: Needs review » Fixed

Committed. http://drupal.org/cvs?commit=461590

We've still got to think about the UI here, as this is TOTALLY not configurable from the UI at the moment - not even in the way that mapping plugins can be set on the repo edit page. Right now, systems (like, say, project*) that want to use this just have to write their own UI entirely, and we could certainly _help_ with that, at least a bit. But we didn't want that to delay getting the fundamental functionality in, and I didn't want to try to make too many guesses without a specific use case in mind. Which is why we've opened #992074: Decide if & what UI to provide for authentication plugins.

To be honest, I feel less confident about this architectural move than any of the other ones we've made since I started working on the migration. I do expect this to grow and change a fair bit as we learn more. But that's why we've opted towards a more minimalist approach with this - hopefully it shouldn't be too tough to rework/expand later.

Status: Fixed » Closed (fixed)
Issue tags: -git phase 2, -git sprint 5, -versioncontrol account refactor, -git sprint 6

Automatically closed -- issue fixed for 2 weeks with no activity.

  • Commit dd1448d on repository-families, drush-vc-sync-unlock by sdboyer:
    Issue #223891 by sdboyer, marvil07: make authentication management...

  • Commit dd1448d on repository-families by sdboyer:
    Issue #223891 by sdboyer, marvil07: make authentication management...