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.

Sign up for GitHub

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

Jump to bottom

Rotate logged in host api key #143

Merged
merged 4 commits into from
Aug 25, 2023
Merged

Rotate logged in host api key #143

merged 4 commits into from
Aug 25, 2023

Conversation

doodlesbykumbi
Copy link
Contributor

@doodlesbykumbi doodlesbykumbi commented Aug 24, 2023 •
edited by szh
Loading

Desired Outcome

Rotating your own role's API key in Conjur requires that the request include basic authentication credentials to verify that the user has positive control of either the current password or API key.

Currently there is no way to configure the Golang CLI to send basic authentication for the rotate API key request, so it is not possible to rotate your own API key with the CLI.

Note, it is possible to rotate another role's API key that your role has update privilege for.

Implemented Changes

Change command behavior of conjur host rotate-api-key without parameters to rotate the currently logged in host's api key, the same as conjur user rotate-api-key without parameters

Connected Issue/Story

CyberArk internal issue ID: CNJR-2078

Definition of Done

At least 1 todo must be completed in the sections below for the PR to be
merged.

Changelog

  • The CHANGELOG has been updated, or
  • This PR does not include user-facing changes and doesn't require a
    CHANGELOG update

Test coverage

  • This PR includes new unit and integration tests to go with the code
    changes, or
  • The changes in this PR do not require tests

Documentation

  • Docs (e.g. READMEs) were updated in this PR
  • A follow-up issue to update official docs has been filed here: CNJR-2078
  • This PR does not require updating any documentation

Behavior

  • This PR changes product behavior and has been reviewed by a PO, or
  • These changes are part of a larger initiative that will be reviewed later, or
  • No behavior was changed with this PR

Security

  • Security architect has reviewed the changes in this PR,
  • These changes are part of a larger initiative with a separate security review, or
  • There are no security aspects to these changes

@doodlesbykumbi doodlesbykumbi requested a review from a team as a code owner August 24, 2023 12:55
@doodlesbykumbi doodlesbykumbi force-pushed the rotate-logged-in-host-api-key branch 2 times, most recently from 262d4a8 to e572160 Compare August 24, 2023 15:34
szh
szh reviewed Aug 24, 2023
View reviewed changes
pkg/cmd/host.go
newAPIKey, err := client.RotateHostAPIKey(hostID)
var newAPIKey []byte
if hostID == "" {
newAPIKey, err = client.RotateCurrentUserAPIKey()
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems if a user is logged in and ran 'conjur host rotate-api-key' it would also rotate the user's api key. I'm thinking in this case maybe we should fail with an error since that's probably not what the user wants to happen. They may be expecting usage info or something instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, the same is also true for conjur host rotate-api-key. This PR is addressing the particular reported bug. Perhaps we can address that in a separate PR

szh
szh suggested changes Aug 24, 2023
View reviewed changes
Copy link
Contributor

@szh szh left a comment

Choose a reason for hiding this comment

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

Almost there, just one tiny typo ;)

pkg/cmd/host_test.go Outdated Show resolved Hide resolved
@doodlesbykumbi doodlesbykumbi force-pushed the rotate-logged-in-host-api-key branch from e572160 to 9fd4c32 Compare August 25, 2023 12:06
@szh szh added the kind/bug label Aug 25, 2023
@szh
Copy link
Contributor

szh commented Aug 25, 2023

@doodlesbykumbi I committed the typo fix and filled in the PR description. Can you please add a changelog entry?

@doodlesbykumbi doodlesbykumbi force-pushed the rotate-logged-in-host-api-key branch from 43bc2ff to e9e6d01 Compare August 25, 2023 16:20
@codeclimate
Copy link

codeclimate bot commented Aug 25, 2023

Code Climate has analyzed commit e9e6d01 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 82.3% (0.0% change).

View more on Code Climate.

szh
szh approved these changes Aug 25, 2023
View reviewed changes
Copy link
Contributor

@szh szh left a comment

Choose a reason for hiding this comment

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

LGTM

@szh szh merged commit 3a3202b into master Aug 25, 2023
@szh szh deleted the rotate-logged-in-host-api-key branch August 25, 2023 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@szh szh szh approved these changes

Assignees

@doodlesbykumbi doodlesbykumbi

Labels
kind/bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@doodlesbykumbi @szh