Skip to content

Secrets Manager app rename and share permission update#171

Open
mtyagi-ks wants to merge 3 commits into
secrets-manager-new-commandsfrom
secrets-manager-new-commands-int
Open

Secrets Manager app rename and share permission update#171
mtyagi-ks wants to merge 3 commits into
secrets-manager-new-commandsfrom
secrets-manager-new-commands-int

Conversation

@mtyagi-ks
Copy link
Copy Markdown

Align keeper-cli KSM with Commander by supporting application rename via vault/records_update and toggling record/shared-folder editable vs read-only on existing app shares (remove + re-add).

  • SDK: update_secrets_manager_app, update_secrets_manager_app_shares
  • CLI: secrets-manager-app update --name; secrets-manager-share update

Align keeper-cli KSM with Commander by supporting application rename
via vault/records_update and toggling record/shared-folder editable
vs read-only on existing app shares (remove + re-add).
- SDK: update_secrets_manager_app, update_secrets_manager_app_shares
- CLI: secrets-manager-app update --name; secrets-manager-share update
@sgaddala-ks
Copy link
Copy Markdown

LGTM. Only issue I have is that string comparison using direct strings instead of a constant. using constants/ enum will make it easier to change later if needed.

if response is None:
raise KeeperApiError('unknown', 'vault/records_update returned no response')

status = next((x for x in response.records if record_uid_bytes == x.record_uid), None)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

validate the status
if not status:
raise KeeperApiError('unknown', f'No status returned for record {app_uid}')

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

addressed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants