Skip to content

Revert "add missing functional tests for users"#6588

Merged
lamont-granquist merged 1 commit into
masterfrom
lcg/test-revert
Nov 21, 2017
Merged

Revert "add missing functional tests for users"#6588
lamont-granquist merged 1 commit into
masterfrom
lcg/test-revert

Conversation

@lamont-granquist

@lamont-granquist lamont-granquist commented Nov 16, 2017

Copy link
Copy Markdown
Contributor

testing to see if appveyor goes green or not.

so somewhere between v13.5.28 and v13.6.7 we broke appveyor.

looks like it was fa6e4e8

@lamont-granquist lamont-granquist changed the title testing revert to v13.5.28 reverting fa6e4e89b586bb8eaee767d5602b13cb75f6c073 Nov 20, 2017
@lamont-granquist lamont-granquist changed the title reverting fa6e4e89b586bb8eaee767d5602b13cb75f6c073 Revert "add missing functional tests for users" Nov 20, 2017

@btm btm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I spent some time debugging this and found that the functionality we are testing here is broken. That doesn't explain to me why this would cause other tests to fail, yet, but I'd support reverting this until I can fix the feature it is testing.

To make support for passing credentials to remote_file we changed to using LOGON32_LOGON_NEW_CREDENTIALS from LOGON32_LOGON_NETWORK, but LOGON32_LOGON_NEW_CREDENTIALS is described as:

This logon type allows the caller to clone its current token and specify new credentials for outbound connections.

Which means we're not actually replacing the token in this case, so we still have an elevated token.

@lamont-granquist

Copy link
Copy Markdown
Contributor Author

Yep, sounds good to me.

@lamont-granquist lamont-granquist merged commit 7c2665b into master Nov 21, 2017
@lamont-granquist lamont-granquist deleted the lcg/test-revert branch November 21, 2017 20:13
@chef chef locked and limited conversation to collaborators Apr 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants