Repository navigation
[beyondinsight_password_safe] Handle null password ion authentication - #17411
Conversation
|
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
…n when password is null
751fd99 to
49b09d2
Compare
🚀 Benchmarks reportTo see the full report comment with |
efd6
left a comment
There was a problem hiding this comment.
Suggest the following commit message
beyondinsight_password_safe: handle optional password in authentication
The BeyondInsight API does not always require a password for
authentication. Whether one is needed depends on the "User Password
Required" setting on the API registration in BeyondInsight. When no
password was configured, the integration failed because it assumed
the password field was always present in state.
ref: https://docs.beyondtrust.com/bips/docs/bi-cloud-configure-api
| url: http://{{Hostname}}:{{Port}}/BeyondTrust/api/public/v3 | ||
| apikey: test_api_key | ||
| username: testuser2 | ||
| password: null |
There was a problem hiding this comment.
This has no default in the manifest, so it can be omitted here.
| password: null |
There was a problem hiding this comment.
The test is specifically for when the password is null. Should we ever add a default in the manifest then this test will no longer test the condition for which it is written.
There was a problem hiding this comment.
In the state construction I think the password field should be conditionally included
{{#if password}}
password: {{escape_string password}}
{{/if}}
There was a problem hiding this comment.
The password always exists. Sometimes it is null.
There was a problem hiding this comment.
I discussed this with @andrewkroh before I did the fix as I was also confused about the password existing and being null. This fix will allow users to not have to update their policies when the integration is updated.
| "Authorization": [ | ||
| sprintf("PS-Auth key=%s; runas=%s;", [state.apikey, state.username]) + | ||
| ((state.password != "") ? (sprintf(" pwd=[%s];", [state.password])) : ""), | ||
| ((state.?password.orValue("") != "") ? (sprintf(" pwd=[%s];", [state.password])) : ""), |
There was a problem hiding this comment.
With conditional rendering into the config, this becomes has(state.password) ? …
There was a problem hiding this comment.
The password always exists. sometimes it is null.
There was a problem hiding this comment.
Can you confirm that password: {{escape_string password}} gives {"password": ""} if the password var is null?
There was a problem hiding this comment.
password: {{escape_string password}} gives null if the password is null. I added in the 'if' block and added some policy tests for null and empty string to verify that no password is set if the variable is null or empty and changed the system tests to test for a missing password.
There was a problem hiding this comment.
Can you confirm that a password state update from "something" to falsey will remove the password from the profile's configuration state?
There was a problem hiding this comment.
If I update a policy that had a password, then leave the password field blank when I update the policy, it generates a policy with no password:
original:
state:
limit: 1000
apikey: ${SECRET_0}
password: ${SECRET_1}
username: amiauser
updated:
state:
limit: 1000
apikey: ${SECRET_0}
username: amiauser
…ing using if statement in template.
efd6
left a comment
There was a problem hiding this comment.
There are some whitespace changes that should be reverted and a query. After that LGTM.
| } | ||
| ] | ||
| `}} | ||
|
|
There was a problem hiding this comment.
These deleted blank lines (also below) are stanza breaks for clarity. They should not be removed.
| "Name": "Test User" | ||
| } | ||
| `}} | ||
|
|
There was a problem hiding this comment.
elastic-package format made those changes. That needs to be fixed.
| "Name": "Test User" | ||
| } | ||
| `}} | ||
|
|
| "Name": "Test User" | ||
| } | ||
| `}} | ||
|
|
| "Name": "Test User" | ||
| } | ||
| `}} | ||
|
|
| "Authorization": [ | ||
| sprintf("PS-Auth key=%s; runas=%s;", [state.apikey, state.username]) + | ||
| ((state.password != "") ? (sprintf(" pwd=[%s];", [state.password])) : ""), | ||
| ((state.?password.orValue("") != "") ? (sprintf(" pwd=[%s];", [state.password])) : ""), |
There was a problem hiding this comment.
Can you confirm that a password state update from "something" to falsey will remove the password from the profile's configuration state?
…k lines that had been removed by elastic-package format
|
/test |
💚 Build Succeeded
History
|
|
Package beyondinsight_password_safe - 0.12.2 containing this change is available at https://epr.elastic.co/package/beyondinsight_password_safe/0.12.2/ |
…elastic#17411) beyondinsight_password_safe: handle optional password in authentication The BeyondInsight API does not always require a password for authentication. The password will be null when it is not supplied. Whether one is needed depends on the "User Password Required" setting on the API registration in BeyondInsight. When no password was configured, the integration failed because it assumed the password field was always present in state. ref: https://docs.beyondtrust.com/bips/docs/bi-cloud-configure-api
Proposed commit message
beyondinsight_password_safe: handle optional password in authentication
The BeyondInsight API does not always require a password for
authentication. The password will be null when it is not supplied.
Whether one is needed depends on the "User Password
Required" setting on the API registration in BeyondInsight. When no
password was configured, the integration failed because it assumed
the password field was always present in state.
ref: https://docs.beyondtrust.com/bips/docs/bi-cloud-configure-api
Checklist
changelog.ymlfile.