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

Properly resolve the policy name #56272

Merged
merged 7 commits into from
Mar 11, 2020
Merged

Conversation

twangboy
Copy link
Contributor

What does this PR do?

Some states were not resolving the policy name from the refId.
Additionally, there was an issue where the element value wasn't being encoded properly

What issues does this PR fix or reference?

#44618

Previous Behavior

Policy name could only be set using refId and not the policy name. lgpo.get_policy_info would return only the refId's and not the policy name.

local:
    ----------
    message:
    policy_aliases:
        - Let Windows apps access account information
        - LetAppsAccessAccountInfo
        - Windows Components\App Privacy\Let Windows apps access account information
    policy_class:
        machine
    policy_elements:
        |_
          ----------
          element_aliases:
              - LetAppsAccessAccountInfo_Enum
              - Default for all apps
          element_id:
              LetAppsAccessAccountInfo_Enum
        |_
          ----------
          element_aliases:
              - LetAppsAccessAccountInfo_UserInControlOfTheseApps_List
              - LetAppsAccessAccountInfo_UserInControlOfTheseApps_List
          element_id:
              LetAppsAccessAccountInfo_UserInControlOfTheseApps_List
        |_
          ----------
          element_aliases:
              - LetAppsAccessAccountInfo_ForceAllowTheseApps_List
              - LetAppsAccessAccountInfo_ForceAllowTheseApps_List
          element_id:
              LetAppsAccessAccountInfo_ForceAllowTheseApps_List
        |_
          ----------
          element_aliases:
              - LetAppsAccessAccountInfo_ForceDenyTheseApps_List
              - LetAppsAccessAccountInfo_ForceDenyTheseApps_List
          element_id:
              LetAppsAccessAccountInfo_ForceDenyTheseApps_List
    policy_found:
        True
    policy_name:
        Let Windows apps access account information
    rights_assignment:
        False

New Behavior

Policy names are now properly resolved. lgpo.get_policy_info now returns policy names:

local:
    ----------
    message:
    policy_aliases:
        - Let Windows apps access account information
        - LetAppsAccessAccountInfo
        - Windows Components\App Privacy\Let Windows apps access account information
    policy_class:
        machine
    policy_elements:
        |_
          ----------
          element_aliases:
              - LetAppsAccessAccountInfo_Enum
              - Default for all apps
          element_id:
              LetAppsAccessAccountInfo_Enum
        |_
          ----------
          element_aliases:
              - LetAppsAccessAccountInfo_UserInControlOfTheseApps_List
              - Put user in control of these specific apps (use Package Family Names)
          element_id:
              LetAppsAccessAccountInfo_UserInControlOfTheseApps_List
        |_
          ----------
          element_aliases:
              - LetAppsAccessAccountInfo_ForceAllowTheseApps_List
              - Force allow these specific apps (use Package Family Names)
          element_id:
              LetAppsAccessAccountInfo_ForceAllowTheseApps_List
        |_
          ----------
          element_aliases:
              - LetAppsAccessAccountInfo_ForceDenyTheseApps_List
              - Force deny these specific apps (use Package Family Names)
          element_id:
              LetAppsAccessAccountInfo_ForceDenyTheseApps_List
    policy_found:
        True
    policy_name:
        Let Windows apps access account information
    rights_assignment:
        False

Tests written?

They're coming

Commits signed with GPG?

Yes

@twangboy twangboy requested a review from a team as a code owner February 28, 2020 17:39
@ghost ghost requested a review from cmcmarrow February 28, 2020 17:39
@twangboy twangboy added the v3000.1 vulnerable version label Feb 28, 2020
@sagetherage sagetherage added this to Considering in 3000.1 Release Feb 28, 2020
@sagetherage sagetherage moved this from Considering to Planning in 3000.1 Release Feb 28, 2020
@sagetherage sagetherage moved this from Planning to Commit in 3000.1 Release Mar 10, 2020
Copy link
Contributor

@waynew waynew left a comment

Choose a reason for hiding this comment

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

Looks like you missed one of the instances @Ch3LL was referring to 😝

tests/unit/modules/test_win_lgpo.py Outdated Show resolved Hide resolved
salt/states/win_lgpo.py Show resolved Hide resolved
Some states were not resolving the policy name from the refId.
Additionally, there was an issue where the element value wasn't being
encoded properly
@dwoz dwoz merged commit 2d78931 into saltstack:master Mar 11, 2020
3000.1 Release automation moved this from Commit to Done Mar 11, 2020
@twangboy twangboy deleted the fix_lgpo_names branch May 26, 2020 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v3000.1 vulnerable version
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants