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

Enables direct supply of a valid accessToken via the playbook for VCP… #456

Merged
merged 1 commit into from
Apr 12, 2024

Conversation

BeardedPrincess
Copy link
Collaborator

…. also Improves errors on expired access token. (Previously this error was unhandled and failed at parsing an empty response body with an unexpected end of json error.

Finally, improves playbook validation for VCP credentials, and prevents more than one kind of credential to be set. Like highlander, there can now only be one! This should keep us from having to make a (wrong) prioritization decision about which credential should be used when multiple are provided.

…. Improves errors on expired access token. Improves playbook validation for VCP credentials
@rvelaVenafi rvelaVenafi merged commit 1a5a8db into Venafi:master Apr 12, 2024
}

if !apikey && !svcaccount {
return false, ErrNoCredentials
// If there's just an API key provided, that's good

Choose a reason for hiding this comment

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

Your logic here is a bit too complex.
I think what you're trying for here (after line 142) is:

if !svcaccount && !apikey && !accesstoken {
	// none of the credential options were provided
	return false, ErrNoCredentials
}
// if we got here then at least one of the credential options was provided
if (svcaccount && apikey) || (svcaccount && accesstoken) || (apikey && accesstoken) {
	// more than one credential option is not acceptable
	return false, ErrAmbiguousVCPCreds
}
// if we got here then only one credential option was provided (which is what we want)
return true, nil

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

much better, thanks

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.

5 participants