-
Notifications
You must be signed in to change notification settings - Fork 0
feat: ✨ Implement azdo security permission update command and fix bugs in handling permissions
#113
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Update member resolution logic to support resolving identities via the Identity API and improve descriptor handling and logging. Rename ResolveMemberDescriptor to ResolveSubject on the extension client and add a new ResolveIdentity method that delegates to the azure-devops identity client. Use util helpers for descriptor and SID detection, and include more detailed debug fields when resolving by descriptor.
Add IsDescriptor and IsSecurityIdentifier helpers to validate Azure DevOps descriptor strings and Windows-style security identifiers (SIDs).
Add a mockgen invocation to generate a MockSecurityClient for the Azure DevOps security client (github.com/microsoft/azure-devops-go-api/azuredevops/v7/security).
Add a new internal/test helper module to simplify running live acceptance scenarios against Azure DevOps. The file builds a guarded test entrypoint that is opt-in via env, constructs authenticated command contexts from environment variables, and provides a compact step runner with guaranteed cleanup semantics. These utilities make acceptance tests easier to author and more robust against environment formatting issues.
Introduce a reusable Poll function and supporting types for test code. Poll accepts a PollFunc and PollOptions to control retries via tries, fixed delay or exponential backoff (starting at 2s), and an optional timeout. It returns the last encountered error with context when attempts/timeouts are reached. This utility simplifies retrying flaky operations in acceptance tests.
Introduce a ConfigReader interface and provide a default and string-backed implementations to decouple config file parsing from consumers. Add NewConfigWithReader to allow injecting a custom reader (used for testing), and wire NewConfig to use the default reader. Also refactor internal config loading to use yamlmap.Map directly and add NewStringConfigReader for easily creating readers from YAML strings in tests.
Update group membership add/remove commands to use the extensions client's ResolveSubject method instead of ResolveMemberDescriptor. This aligns the commands with the updated client API for resolving member subjects.
Make subject resolution more robust and informative. Use ResolveSubject for member resolution and include the organization in the missing subject error message to aid debugging. Add fallback logic when the identity lookup returns no entries or an empty descriptor, and log that situation instead of failing immediately. Adjust transformResponse to iterate ACE dictionary entries and accept resolved descriptor via buildPermissionEntry
Use ResolveIdentity instead of ResolveMemberDescriptor + Identity API lookups and pass the resolved member.Descriptor directly to the security QueryAccessControlLists request. This removes the extra identity client/read-identities roundtrip and simplifies descriptor handling. Also update logging and transformResponse to no longer require a descriptor parameter. This fixes subject descriptor resolution mismatches when querying ACLs.
Add unit test internal/cmd/security/permission/list/list_test.go that verifies the command uses a resolved identity Descriptor when the identity lookup does not provide enough information. The test ensures ReadIdentities is skipped and QueryAccessControlLists is called with the subject descriptor, asserting the command reports "No permissions found."
Ensure the parsed SubjectTarget now has its Subject field set when parsing input. Previously only Scope was populated which could lead to missing subject information downstream; this restores expected behavior.
Add a new `security permission update` command to create or modify access control entries for a subject on a securable resource.
Prevent accidental committing of .env and similar environment files by adding **/.env* to .gitignore
Add two VS Code debug configurations for running Go ACC tests. One configuration runs tests normally; the other sets HTTP(S) proxy environment variables. Both inject AZDO_ACC_TEST and AZDO_DEBUG and load additional env vars from .env.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes: #86