-
Notifications
You must be signed in to change notification settings - Fork 25
feat: Initial work on CyberArk Identity client #655
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
Conversation
970cb9a
to
cbb3aeb
Compare
eadca4b
to
402ea1f
Compare
While digging into that, I discovered a very new Golang SDK (1 month old), written by the same author: It seems to have an auth / identity package. |
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.
Looks good to me to merge and build upon in future PRs, unless we decide we can use the new CyberArk SDK: https://github.com/cyberark/ark-sdk-golang
My overall comments are:
- I was a bit confused about the password / secret handling, and I would like to read more about the vision for that.
- The new files should all have copyright / apache license headers, as per the LICENSE file.
- I'd like to see an example of how you run the
cmd/testidentity
tool.
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.
I wanted to try this out, but it's not obvious what URL or credentials to use.
Suggest adding an explanatory comment with an example of usage.
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.
I can provide credentials in another place - because this PR is public I don't want to share any details of our private test env for this. I'm sure it would be fine to do so, but I'm being super careful about what I share and don't share in public.
😭 This is the pain of integrating with a new company - nobody I've spoken to so far mentioned this. Definitely that repo will be the longer-term way to go, although I suspect we should merge a version of this here and then iterate from there. First I'll fix up based on your comments! |
402ea1f
to
e64600a
Compare
This makes it easier to test the identity client Signed-off-by: Ashley Davis <[email protected]>
Signed-off-by: Ashley Davis <[email protected]>
This includes a bunch of testing data obtained experimentally from testing against an integration-environment tenant. I've confirmed that I'm able to log in using this code. Currently, this code stores the login token and requires exactly one username/password challenge to complete successfully. Signed-off-by: Ashley Davis <[email protected]>
Signed-off-by: Ashley Davis <[email protected]>
Signed-off-by: Ashley Davis <[email protected]>
e64600a
to
c633902
Compare
@wallrj I've updated it now. Also added a retry for the login process, and as part of that I migrated the other use of the backoff library to the latest and greatest version |
Going to merge this now so we can build on it later! |
This includes a bunch of testing data obtained experimentally from testing against an integration-environment tenant.
I've confirmed that I'm able to log in using this code in some scenarios; more work is needed on this client before it can be used but this is quite a lot of lines added as-is and it's worth getting an initial review of the logic so far.
Most docs are linked in comments, but the most important are:
The open source Identity SDK in Python may also be helpful: https://github.com/cyberark/ark-sdk-python/blob/main/ark_sdk_python/auth/identity/ark_identity.py