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

Force re-auth if cached credentials are invalid #34

Merged
merged 7 commits into from
Jan 26, 2024

Conversation

ccapurso
Copy link
Contributor

@ccapurso ccapurso commented Jan 23, 2024

Overview

Attempting to run hcp connect will fail if a prior successful connection has been made but the associated credentials have expired or been invalidated. The underlying issue is rooted in how hcp-sdk-go caches credentials. An attempt to list HCP organizations using invalid credentials will fail with a 401 response. The following error is provided to the user:

Failed to get HCP organization information: [GET /resource-manager/2019-12-10/organizations][401] OrganizationService_List default &{Code:0 Details:[] Message:}

This issue can currently be resolved by running hcp disconnect which will perform a logout within hcp-sdk-go. This PR introduces a change to validate the obtained credentials using the IAM service GetCallerIdentity API endpoint. A logout (similar to hcp disconnect) is performed which will clear the credentials cache on disk in ~/.config/hcp/creds-cache.json. A re-authentication is initiated once the logout succeeds.

IAM Service mocks were added for testing which accounts for a large portion of the diff.

This PR also introduces a couple unrelated changes:

  • A slight refactor such that the client members of HCPConnectCommand are no longer passed as method arguments and are instead accessed from the receiver
  • Improved the error message provided when a cluster identified by -cluster-id does not exist

Related Issues/Pull Requests

This will be incorporated with a Vault PR once approved and merged

@ccapurso ccapurso requested a review from a team as a code owner January 23, 2024 22:00
Copy link
Contributor

@VioletHynes VioletHynes 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 to me! A few small comments. Note though that it's harder for me to speak for the veracity of the business logic as I'm less familiar with the subject matter.

connect.go Outdated Show resolved Hide resolved
connect_test.go Outdated Show resolved Hide resolved
@ccapurso ccapurso merged commit 473e9a4 into main Jan 26, 2024
2 checks passed
@ccapurso ccapurso deleted the hvt-4984-force-re-auth branch January 26, 2024 20:01
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.

4 participants