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.

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

Add a few more checks on rollback of Util User. #535

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bevanweiss
Copy link
Contributor

@bevanweiss bevanweiss commented Jun 30, 2024

Add a few more checks on rollback of Util User.
Tidy up the memory management around the ::NetUserGetInfo to align with MS guidance (https://learn.microsoft.com/en-us/windows/win32/api/lmaccess/nf-lmaccess-netusergetinfo#examples)
Removed memory tidyup, it's now in the more recent PR #547

@bevanweiss
Copy link
Contributor Author

I'm a little worried that the rollback tests for the Util User may not actually be functional.
When I uncomment the Assert's in ConfigureUsers, CreateUser and CreateUserRollback and run the rollback tests I'm only seeing the ConfigureUsers assert triggered.

My expectation was that all three would be triggered, the Configure would have setup the Create and the Rollback CAs, then the CreateUser should have triggered, Completed, then the CAFail would have triggered sometime later, which should have then resulted in the CreateUserRollback triggering.
Whilst the ConfigureUsers completes fine, neither of the other CAs appear to eventuate (in the ProductFail / ProductCommentFail tests).

@barnson
Copy link
Member

barnson commented Jun 30, 2024

Rollback is scheduled only in this if block:

if ((USER_EXISTS_YES == ueUserExists) || (USER_EXISTS_INDETERMINATE == ueUserExists) || !(psu->iAttributes & SCAU_DONT_CREATE_USER))

@bevanweiss
Copy link
Contributor Author

Rollback is scheduled only in this if block:

if ((USER_EXISTS_YES == ueUserExists) || (USER_EXISTS_INDETERMINATE == ueUserExists) || !(psu->iAttributes & SCAU_DONT_CREATE_USER))

hmmm.... so in the Rollback Test (ProductFail / ProductCommentFail) I should not have expected CreateUser / CreateUserRollback to be executed?
I was really expecting to have CreateUser for each user (at least testName1 and testName2 which were to be created by the MSI).
And then post CAFail, I was expecting CreateUserRollback for each CreateUser that had actually modified state.

@barnson
Copy link
Member

barnson commented Jun 30, 2024

I haven't looked at the tests yet. I just remembered that User is different than a lot because rollback of a removal isn't possible.

Signed-off-by: Bevan Weiss <bevan.weiss@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants