wixtoolset / issues

WiX Toolset Issues Tracker
http://wixtoolset.org/
130 stars 24 forks source link

WIP: User Group creation / removal and Group->Group membership creation #8577

Open bevanweiss opened 5 months ago

bevanweiss commented 5 months ago

User story

As a setup developer, I need to configure custom User Groups (both Local / Domain level) to handle grouping together of authorizations for my applications. This requires that I either need to be able to author these groups and default memberships into an installer process (MSI), or that I need to document for the end user(s) how to perform these actions steps themselves.

It should be easy for me to author an installer within WiX that allows for creation (at install time) of Local / Domain user groups, and to configure both user and nested group memberships for groups.

Proposal

The proposal consists of two aspects:

  1. The ability to Add a new (user) Group (or modify an existing) Currently the below is NOT allowed:

    <Component ... >
    <util:Group .... >
    </Component>

    The proposal is to allow this syntax of nesting a util:Group element under a Component element (similar to the util:User element), and to expand the util:Group element attributes similar to that available for the util:User NOTE: nullable syntax show for types below.. i.e. FormattedString? indicates that entry is not required (can be null)

    <Component ... >
    <util:Group 
       Domain=FormattedString? 
       Id=String?
       Name=FormattedString
       Comment=FormattedString?
       CreateGroup=wxs:YesNoTypeUnion?
       FailIfExists=wxs:YesNoTypeUnion?
       RemoveComment=wxs:YesNoTypeUnion?
       RemoveOnUninstall=wxs:YesNoTypeUnion?
       UpdateIfExists=wxs:YesNoTypeUnion?
       Vital=wxs:YesNoTypeUnion? />
    </Component>

    This would leverage the NetGroupAdd/NetGroupDel functions (lmaccess.h), along with the NetGroupSetInfo (to set the Group comment if the Group already exists). The current proposal would be the extend the existing Wix4Group table with the additional columns: Comment, Attributes to accommodate the data ingestion into the custom action. An alternative would be to create a new Wix4CreateGroup table, with these columns alongside a reference column back to the Wix4Group table. This may be more desirable to maintain backwards compatibility for Patches.

  2. The ability to Add a nested Group member to another Group

    <util:Group ....>
    <GroupRef Id=String />
    </util:Group>

    This would leverage the NetLocalGroupAddMembers function (lmaccess.h) to create the membership relationship between the two groups. A new Wix4GroupGroup table (naming just based on Wix4UserGroup.. perhaps a more descriptive Wix4GroupMembership might be warranted), this table would just have two columns: Member_, Group_ following the ordering of the Wix4UserGroup table

Sequencing of execution would be:

  1. Group Creation [CreateGroups]
  2. Group->Group memberships [CreateGroupMemberships]
  3. User Creation + User->Group memberships [as existing]

Considerations

Reference to the following two prior feature requests: https://github.com/wixtoolset/issues/issues/1347 https://github.com/wixtoolset/issues/issues/2186

NOT CURRENTLY PROPOSED: Another possibility might be to support nesting util:Group under util:Group elements, as below:

<util:Group or util:GroupRef ....>
  <util:Group or util:GroupRef ...>
    <util:Group or util:GroupRef ... />
  </util:Group>
</util:Group>

Or possibly doing similar with util:User

<util:Group or util:GroupRef ....>
  <util:Group or util:GroupRef ...>
    <util:User or util:UserRef ... />
  </util:Group>
</util:Group>

I think this would be trickier to support, since the current util:User logic simply considers if the immediate parent is a Component type, and if so flags it for creation. Having nested levels may make this more confusing for an author, as well as more challenging for the WiX compiler logic.

barnson commented 5 months ago
  1. Prefer a new Wix6Group table name.
  2. Prefer Wix6GroupGroup table name.
  3. Prefer ParentGroup and ChildGroup column names and ParentGroup should come first (a la FeatureComponent table, not the Wix4UserGroup table).
  4. Need a bit on rollback---because groups have SIDs, they can't just be re-created on rollback. Users have the same problem: They're handled in a commit custom action. Membership could be rolled back, however.

Overall looks good! Please think about sending in multiple draft pull requests so we can review and provide feedback in manageable chunks.

bevanweiss commented 5 months ago
  1. Prefer a new Wix6Group table name.

To replace the entire WixGroup current table name, or as an additional table, with a reference back to the original? So the CA view would be:

vcsGroupQuery = L"SELECT `Group`, `Component_`, `Name`, `Domain` FROM `Wix4Group` WHERE `Group`=?";
or
vcsGroupQuery = L"SELECT `Group`, `Component_`, `Name`, `Domain`, `Comment`, `Attributes` FROM `Wix4Group`,`Wix6Group` ON `Group` = `Group_` WHERE `Group`=?";
  1. Need a bit on rollback---because groups have SIDs, they can't just be re-created on rollback. Users have the same problem: They're handled in a commit custom action. Membership could be rolled back, however.

I haven't tried it, but the GROUP_INFO_3 structure captures SID, so potentially it might be possible to 're-create' if the SID was stored away for rollback. I suspect the Win32API ignores this SID on create though (even if it doesn't explicitly document that).. seems like a possible security vulnerability otherwise.

There's almost certainly going to be a merge conflict against any fix for https://github.com/wixtoolset/issues/issues/8576. I've got something in my current code for that already, but will rebase around whatever is put in by Rob for that. I'll tidy up what I've got (to align with updated table names), and will put in the first draft PR tonight (AEST).

barnson commented 5 months ago
  1. All-new table is fine. We did that in WiX v5. WiX extensions are version-specific so the prefixes exist just to allow mixing of merge modules built with a different version of WiX.
  2. Yeah, the doc in that space isn't great, but I don't know that I'd trust that it's supported if it's not explicitly documented.
  3. Feel free to include it; Rob took the issue in part to look for other potential leaks in code that old---it's WiX v2-era old.
bevanweiss commented 5 months ago
  1. Apologies, I must be slow on the pick-up today... So with an 'all-new table'.. is this saying that overall in the WiX table space there would be BOTH a Wix4Group AND a Wix6Group table (i.e. a whole new table for Wix6). Which is what I think might be the right way moving forward..

    I'd like to think that the existing Wix4Group references (both internal to WiX and 3rd party) could remain untouched, certainly the likes of the IIS and the COM extensions appear to reference Wix4Group already. My thoughts are 'Create / Modify Group' Functionality is a bit of a (Wix v6) extension to the current Wix v4 functionality. So existing extensions that only expect Wix4Group stuff should work without any needed knowledge (or changes) due to the extra Wix6 'group stuff'.

barnson commented 5 months ago
  1. We want to avoid adding columns to the existing table. (It should be fine, but let's not tempt fate with WiX v4/v5.)
  2. Replacing the old table with a new table combining the old table with the new columns makes a lot of sense...
  3. ...but then you point out advantages of keeping the old table around. Makes sense to avoid unnecessary changes in other extensions.

So I've changed my mind and agree on two tables. @robmen, do you have any concerns?

bevanweiss commented 4 months ago

@barnson I've got the basic Group Addition / Removal working. However I'm struggling a little on the best way to do the Group Membership modifications. Because Groups can be Parent / Child to other Groups, there needs to be a split between Creating / Removing Groups, and modifying Group Memberships (between groups)...

I've got a couple of challenges:

  1. I'd like to decouple the Group-Group membership CA from the Create/Remove Group. But I'd want to reference the Group's Component state (Installed vs Action) to determine what action should occur for the Group-Group membership, if either component is being uninstalled then the relationship should be removed, else if either component is being installed, then the relationship should be added. The GroupGroup table definition is just [('Parent_'=>Group), ('Child_'=>Group)], ideally I'd like a query that returns Parent_, Parent_.Component_, Child_, Child_.Component_ so that I can easily get the states for each side of the relationship. But my Windows Installer SQL-foo is weak... and I only know the classic SQL table alias method to do a join to get these in, which installer says 'No' to. Any query magic? (or should I just do the individual parameterised query lookups to the Group table in code?)
  2. Scheduling of the ConfigureGroups should be BEFORE InstallFiles (like ConfigureUsers), but if there are any Users to create, then ConfigureGroups should ALSO be BEFORE ConfigureUsers (so that the Groups exist by the time ConfigureUsers occurs). Any way to do this 'conditional' sequencing (i.e. BEFORE="InstallFiles" unless ConfigureUsers CA will be present, in which case it should be BEFORE="Wix4ConfigureUsers_??"
  3. Ditto for ConfigureGroupGroupMemberships which should be AFTER="ConfigureGroups" (so that all the groups exist for the memberships). To nicely tidy things up, it's probably desirable to have a ConfigureRemoveGroupGroupMemberships which should be BEFORE="ConfigureGroups" to remove group memberships for groups that will be removed.