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

[Environments] Auto reload after Extensions page visit; selectable error text #3589

Merged
merged 11 commits into from
Aug 29, 2024

Conversation

huzaifa-d
Copy link
Contributor

@huzaifa-d huzaifa-d commented Aug 12, 2024

Summary of the pull request

  1. Fixes the issue where the Environments page showed loaded/cached data even if there was a change in the extensions currently enabled or disabled. Now the Environments page is auto-reloaded whenever there is an extension is enabled/disabled and the Environments page is navigated back to.

  2. Fixes the issue where a standard user couldn't be added through a UAC where a different user entered their admin credentials.

  3. Fixes the issue where the errors/info text shown in Info Bars on the Environment/Machine configuration were not selectable.
    image

  4. Fixes project name/alternative title not showing up in Machine configuration flow

  5. Fixes empty properties showing up despite having no data.

image

Validation steps performed

Manually

PR checklist

@huzaifa-d huzaifa-d marked this pull request as ready for review August 14, 2024 20:44
@huzaifa-d huzaifa-d changed the title [Environments] Auto reload after Extensions page visit [Environments] Auto reload after Extensions page visit; selectable error text Aug 21, 2024
@@ -37,21 +37,10 @@ 6. The user is already in the Hyper-V Admin group and the Hyper-V Feature is alr
$adminGroupResult = [OperationStatus]::OperationNotRun
$hyperVGroupSid = 'S-1-5-32-578'

# Check the security token the user logged on with contains the Hyper-V Administrators group SID (S-1-5-32-578). This can only be updated,
# once the user logs off and on again. Even if we add the user to the group later on in the script.
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe you should be able to pass the users sid by doing string formating to replace values within the string. I believe Nick does something similar for the virtualization management page. We'd put a place holder for the user name within the string. Then within the code once we get the whole scripts string, we can replace the placeholder with the users sid

This: https://github.com/microsoft/devhome/blob/a9f6a7361318fb9cf7a05147e6e780c84277ddbd/common/Scripts/ModifyWindowsOptionalFeatures.cs#L218C16-L218C36

is replaced with This:

var scriptString = Script.Replace("FEATURE_STRING_INPUT", featuresString);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not understanding the suggestion? And what's the advantage?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh this was a suggestion sorry. It was for if you wanted to pass in the users S-I-D of the user since you removed lines 42. which had

[System.Security.Principal.WindowsIdentity]::GetCurrent().Groups.Value

I was thinking there might be a way to pass in something like a SID or username so we can get an object that would tell us if the user is in the group or not. But if we really don't need these lines then its all good to ignore.

common/Services/IExtensionService.cs Show resolved Hide resolved
@@ -37,21 +37,10 @@ 6. The user is already in the Hyper-V Admin group and the Hyper-V Feature is alr
$adminGroupResult = [OperationStatus]::OperationNotRun
$hyperVGroupSid = 'S-1-5-32-578'

# Check the security token the user logged on with contains the Hyper-V Administrators group SID (S-1-5-32-578). This can only be updated,
# once the user logs off and on again. Even if we add the user to the group later on in the script.
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh this was a suggestion sorry. It was for if you wanted to pass in the users S-I-D of the user since you removed lines 42. which had

[System.Security.Principal.WindowsIdentity]::GetCurrent().Groups.Value

I was thinking there might be a way to pass in something like a SID or username so we can get an object that would tell us if the user is in the group or not. But if we really don't need these lines then its all good to ignore.

@huzaifa-d huzaifa-d merged commit cc84b64 into main Aug 29, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment