-
Notifications
You must be signed in to change notification settings - Fork 13
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
Enable SP to retrieve and provide German umlaut chars #500
Conversation
8d7ae2d
to
3f6384b
Compare
Uses the Go API function client.RetrieveBatchSecretsSafe, which requests base64 encoded secrets from Conjur, and returns a map of variable IDs to decoded values. Secrets Provider can now inject binary secret values, including strings with special characters.
3f6384b
to
9c027b9
Compare
Code Climate has analyzed commit 9c027b9 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 89.1% (0.0% change). View more on Code Climate. |
@@ -35,6 +35,7 @@ done | |||
conjur variable values add secrets/test_secret "some-secret" | |||
conjur variable values add "secrets/var with spaces" "some-secret" | |||
conjur variable values add "secrets/var+with+pluses" "some-secret" | |||
conjur variable values add "secrets/umlaut" "some-secret" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you not need an umlaut in this secret value? Or is this just a placeholder that gets replaced later on?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a placeholder that's replaced when a test case runs [test case][CLI variable load].
*/ | ||
type ConjurClient interface { | ||
RetrieveBatchSecrets([]string) (map[string][]byte, error) | ||
RetrieveBatchSecretsSafe([]string) (map[string][]byte, error) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure it's totally safe now or are there other corner cases that might sneak in? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disclaimer: did not come up with the naming here. Having trouble thinking of a better variable name that distinguishes the two without getting into unseen details.
Also, RetrieveBatchSecretsSafe
has the same interface as RetrieveBatchSecrets
, but doesn't fail on special characters/binary values - maybe RetrieveBatchSecretsSafe
should be renamed to RetrieveBatchSecrets
, and RetrieveBatchSecrets
shouldn't be used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh right, you're just using a different conjur-api-go
method? Nah, that's fine then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Desired Outcome
Enable Secrets Provider to deliver secrets with special characters/binary values.
Implemented Changes
In conjur#1989, the
/secrets
endpoint was updated to allow for the batch retrieval of secrets with binary values - adding theAccept-Encoding
header set tobase64
means Conjur will base64 encode secret values before attempting to render response JSON.We can use that functionality here to enable Secrets Provider to deliver binary/special character secrets by switching
client.RetrieveBatchSecrets()
toclient.RetrieveBatchSecretsSafe()
.Connected Issue/Story
CNJR-466
Definition of Done
At least 1 todo must be completed in the sections below for the PR to be
merged.
Changelog
CHANGELOG update
Test coverage
changes, or
Documentation
README
s) were updated in this PRBehavior
Security