infamousjoeg / cybr-cli

A "Swiss Army Knife" command-line interface (CLI) for easy human and non-human interaction with @CyberArk suite of products.
Apache License 2.0
71 stars 15 forks source link

Improve logon with 2FA #114

Closed mdaguete closed 3 years ago

mdaguete commented 3 years ago

Currenly when trying to autenticate against a CyberArk with 2FA enabled the request request fails with the following error:

Failed to Logon to the PVWA. Failed to authenticate to the PAS REST API.
Received non-200 status code '500'

In function httpjson.SendRequestRay the response body isn't returned, so the caller function never finds the correct error code.

After the previous fix, the second call, the one with the otp code still fails with 403. Debugging it seems that we need to store the cookies between the two logon calls. So a go context is added to the functions.

infamousjoeg commented 3 years ago

@mdaguete Really loving this PR! I did a super quick review of the code changes and then allowed GitHub Actions to run its workflow. Lint failed, so if you can commit a fix for go-lint to pass, both @AndrewCopeland and I will begin our formal review.

Thanks for contributing and improving this project!

infamousjoeg commented 3 years ago

@AndrewCopeland after this is merged, we'll release v0.1.3-beta.

mdaguete commented 3 years ago

Hi @infamousjoeg ,

Thank you for your comments !

I'm glad to contribute to this project, it makes my life easier to use cyberark.

If you condider it necessary to sign the commits, I can do that and make a new pr wiith signed ones.

Regards.

infamousjoeg commented 3 years ago

@mdaguete signing is not required at all.

infamousjoeg commented 3 years ago

@AndrewCopeland Can you take a look at the tests failing when you get a chance, please? I don't believe they are related to this PR.

AndrewCopeland commented 3 years ago

@mdaguete Some of the unit tests are returning with failure, the failure message being returned is the following:

--- FAIL: TestListAccountSuccess (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
    panic: runtime error: invalid memory address or nil pointer dereference [recovered]
    panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x4e529d]

I would imagine this error is related to the context that has been added.

The panic is being raised here: https://github.com/mdaguete/cybr-cli/blob/improve-2fa/pkg/cybr/helpers/httpjson/httpjson.go#L168

sonarcloud[bot] commented 3 years ago

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

mdaguete commented 3 years ago

Hi @AndrewCopeland,

I think the issue was related to an empty response body when authenticating. Unfortunately I can't access a non production instance to test it but I think that my latest commit can fix it.

Thanks.