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

BED-3671 -- add group shortcutting to SyncLAPSPassword Postprocessing #649

Merged
merged 9 commits into from
Jun 21, 2024

Conversation

maffkipp
Copy link
Contributor

@maffkipp maffkipp commented Jun 7, 2024

Description

Previously the logic for creating SyncLAPSPassword edges needed to query for group membership for all groups that had one of the two permissions. This change takes advantage of the group expansion we are already doing in a separate postprocessing step to avoid unnecessary db queries.

Additionally, the SyncLAPSPassword edge should no longer be created for members of groups that already have that edge, which should also reduce unnecessary noise in the graph; this difference is accounted for in the accompanying test harness.

Once this is merged, I can get another PR up to make the same change for DCSync.

Motivation and Context

  • Improve analysis performance by eliminating unnecessary group expansion
  • avoid creating redundant SyncLAPSPassword edges in the graph

How Has This Been Tested?

I've added a new integration test + harness to ensure these changes are doing what we expect

Screenshots (if appropriate):

Types of changes

  • Chore (a change that does not modify the application functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • Documentation updates are needed, and have been made accordingly.
  • I have added and/or updated tests to cover my changes.
  • All new and existing tests passed.
  • My changes include a database migration.

@maffkipp maffkipp self-assigned this Jun 7, 2024
@maffkipp maffkipp marked this pull request as ready for review June 8, 2024 00:02
@maffkipp maffkipp added enhancement New feature or request api A pull request containing changes affecting the API code. labels Jun 10, 2024
Copy link
Contributor

@urangel urangel left a comment

Choose a reason for hiding this comment

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

Looks good!

@maffkipp maffkipp merged commit 171a353 into main Jun 21, 2024
3 of 4 checks passed
@maffkipp maffkipp deleted the BED-3671--syncLAPSPassword branch June 21, 2024 20:40
@github-actions github-actions bot locked and limited conversation to collaborators Jun 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api A pull request containing changes affecting the API code. enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants