Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Open
bevanweiss opened this issue Jun 18, 2024 · 6 comments · May be fixed by wixtoolset/wix#530
Open

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

bevanweiss opened this issue Jun 18, 2024 · 6 comments · May be fixed by wixtoolset/wix#530
Assignees
@bevanweiss
Labels
extensions feature wip WiX Improvement Proposal
Milestone
v6.0.0-rc.1

Comments

@bevanweiss
Copy link

bevanweiss commented Jun 18, 2024 •
edited
Loading

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.

  1. 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:
#1347
#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.
@bevanweiss bevanweiss added triage wip WiX Improvement Proposal labels Jun 18, 2024
@barnson
Copy link
Member

barnson commented Jun 18, 2024

  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.

@barnson barnson added this to the v6.0.0-rc.1 milestone Jun 18, 2024
@bevanweiss
Copy link
Author

  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 #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
Copy link
Member

barnson commented Jun 18, 2024

  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
Copy link
Author

  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
Copy link
Member

barnson commented Jun 18, 2024

  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 bevanweiss linked a pull request Jun 19, 2024 that will close this issue
Open
@bevanweiss
Copy link
Author

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bevanweiss bevanweiss

Labels
extensions feature wip WiX Improvement Proposal
Projects
None yet
Milestone
v6.0.0-rc.1
Development

Successfully merging a pull request may close this issue.

[REVIEW please] CreateGroups additions bevanweiss/wix
2 participants
@barnson @bevanweiss