New default configurations #229
Conversation
scripts/setup/setup.ps1
Outdated
| @@ -123,7 +127,7 @@ function Upgrade() { | |||
|
|
|||
| $installed = $false | |||
| try { | |||
| .\install.ps1 -Path $adminRoot -Version $Version -SkipVerification:$SkipVerification -ServiceName $ServiceName -Port 0 -DistributablePath $DistributablePath -CertHash $CertHash | |||
| .\install.ps1 -Path $adminRoot -Version $Version -SkipVerification:$SkipVerification -ServiceName $ServiceName -Port 0 -DistributablePath $DistributablePath -CertHash $CertHash -LegacyConfigurations:$LegacyConfigurations | |||
There was a problem hiding this comment.
This doesn't look like proper naming. It doesn't mean anything actionable. Whatever this is doing should be explicit.
scripts/setup/config.ps1
Outdated
| } | ||
|
|
||
| ## ensure the member/group exists in the specified group | ||
| function EnsureMember($group, $userOrGroup) { |
There was a problem hiding this comment.
EnsureLocalGroupMember would probably be better to follow the powershell commandlets.
scripts/setup/config.ps1
Outdated
| } | ||
|
|
||
| ## ensure the member/group exists in the specified group | ||
| function EnsureMember($group, $userOrGroup) { |
There was a problem hiding this comment.
This belongs in security.ps1. Also are these commandlets supported on all OS out of the box. I remember we needed to do some work to see what commandlets were available at runtime for group related actions.
scripts/setup/uninstall.ps1
Outdated
| @@ -121,7 +125,10 @@ function Uninstall($_path) | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| Remove-LocalGroup -Name $IISAdminOwnersGroup -ErrorAction "Continue" | |||
There was a problem hiding this comment.
We can't just delete that group in uninstall. We don't know if we created it. If we knew we created it then we could.
There was a problem hiding this comment.
I will assign a description This group is used by IIS Administration API and members have full access to the application. It would be removed during if the application is uninstalled and verify if the description is there before removing
scripts/setup/config.ps1
Outdated
|
|
||
| [parameter()] | ||
| [string] | ||
| $IISAdminOwnersGroup = "IIS Administration API Owners", |
There was a problem hiding this comment.
Looks like this should be in Globals.ps1
scripts/setup/security.ps1
Outdated
|
|
||
| $g = GetLocalGroup $_name | ||
|
|
||
| $installerFlag = .\globals.ps1 'INSTALLER_FLAG' |
There was a problem hiding this comment.
This logic is polluting the remove localgroup method. The caller should do any verification of the group and then call RemoveLocalGroup.
There was a problem hiding this comment.
The idea is that any other invocation can take advantage of the installer flag check by opting in. This can be think of as adding a functionality.
There was a problem hiding this comment.
RemoveLocalGroup doesn't know anything about installer flags. It has one purpose: remove a local group. It should only have parameters to do that.
scripts/setup/globals.ps1
Outdated
| [string] | ||
| $Command | ||
| ) | ||
|
|
||
| $INSTALL_METHOD_KEY = "IIS_ADMIN_INSTALL_METHOD" | ||
| $installerFlag = "This item will be removed when Microsoft IIS Administration API is uninstalled." |
There was a problem hiding this comment.
"This item will be removed if the Microsoft IIS Administration API is uninstalled."
What do you think about this?
"This group will be removed if the Microsoft IIS Administration API is uninstalled, but can be prevented if this sentence is removed."
There was a problem hiding this comment.
I have change the when to if the. Not sure if we want to suggest to the user to modify the description.
scripts/setup/uninstall.ps1
Outdated
| $groupName = .\globals.ps1 'IIS_ADMIN_API_OWNERS' | ||
| $group = .\security.ps1 GetLocalGroup -Name $groupName | ||
| $installerFlag = .\globals.ps1 'INSTALLER_FLAG' | ||
| if ($group.Description.Contains($installerFlag)) { |
We will be creating a group
IIS Administration API Ownersas the default administration owner and administrator for ease of managing access policies.Allow opting out
manage.iis.netin cors rule. Note that it is still included by default