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

win: nsis installer changed SID of grass82.py #2605

Merged
merged 1 commit into from
Oct 22, 2022

Conversation

dnewcomb
Copy link
Contributor

In theory, this should fix 2603. On Windows 10 Professional , the permissions are correct. Will need to test on Windows 10 Enterprise

@hellik hellik requested review from ninsbl and landam October 21, 2022 19:15
@hellik hellik added backport_needed windows Microsoft Windows specific labels Oct 21, 2022
@hellik hellik added this to the 8.2.1 milestone Oct 21, 2022
Copy link
Member

@ninsbl ninsbl left a comment

Choose a reason for hiding this comment

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

It is a small change that seems fine to me. But I have not tested and I do not think this is covered by CI. So is should be tested on the build server.
But it should be fairly easy to change (back) if needed... We should wait for one successfull installer build and test before backporting though...

Copy link
Member

@hellik hellik left a comment

Choose a reason for hiding this comment

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

it seems ok, as @ninsbl suggested wait for the next build for merging PR

@@ -758,8 +758,8 @@ Section "GRASS" SecGRASS
Push 'config_projshare = "$INSTDIR\share\proj"' ; string to replace whole line with
Call ReplaceLineStr

;replace BU with numeric group name for local users
AccessControl::SetOnFile "$INSTDIR\etc\grass@GRASS_VERSION_MAJOR@@GRASS_VERSION_MINOR@.py" "(S-1-5-32-545)" "GenericRead + GenericExecute"
;replace BU with numeric group name for local users. Users S-1-5-32-545 does not work for Windows Enterprise. Try Authenticated Users S-1-5-11
Copy link
Member

Choose a reason for hiding this comment

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

Authenticated Users:

"Any user who accesses the system through a sign-in process has the Authenticated Users identity. This identity allows access to shared resources within the domain, such as files in a shared folder that should be accessible to all the workers in the organization. Membership is controlled by the operating system."

Attribute Value
Well-known SID/RID S-1-5-11
Object class Foreign Security Principal

The Authenticated Users group includes all users whose identities were authenticated when they logged on. This includes local user accounts as well as all domain user accounts from trusted domains.

The Guest account may not work with S-1-5-11 . Though this should not be a big issue.

to sum up, this PR looks good.

is this PR against main? if yes, I'll approve the PR.

@ninsbl ninsbl merged commit c867531 into OSGeo:main Oct 22, 2022
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Feb 17, 2023
marisn pushed a commit to marisn/grass that referenced this pull request Jun 2, 2023
@wenzeslaus wenzeslaus changed the title nsis installer changed SID of grass82.py win: nsis installer changed SID of grass82.py Jun 6, 2023
@dnewcomb dnewcomb deleted the update_NSIS_sid branch June 23, 2023 01:11
neteler pushed a commit to nilason/grass that referenced this pull request Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
windows Microsoft Windows specific
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] NSIS permissions not correct for Windows 10 Enterprise
4 participants