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

Improve logon with 2FA (#114) #115

Closed
wants to merge 6 commits into from
Closed

Conversation

infamousjoeg
Copy link
Owner

Co-authored-by: Quincy Cheng [email protected]
Co-authored-by: Joe Garcia [email protected]

  • Improve logon with 2FA

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.

  • Fix lint errors and add missing file from previous commit

This commit fixes the go lint error related to key type in context and it
adds a missing file from previous commit not uploaded by error.

  • Fix type error in comment

  • Missing comment on Cookie function

Added missing comment

  • Fix panic when body response is nil

Try to catch nil response body on httpjson.SendRequestRaw function

Co-authored-by: Andrew Copeland [email protected]
Co-authored-by: Quincy Cheng [email protected]
Co-authored-by: Joe Garcia [email protected]

infamousjoeg and others added 5 commits April 16, 2021 15:07
- Fixes #108 add authn ldap support (#109)
* Resolves #106

Co-authored-by: Quincy Cheng <[email protected]>
* V0.1.3 beta (#113)

* Fixes #108 add authn ldap support (#109)
* Fixes #106 Adding 6 CEM sub-commands (#105) (#111)

Co-authored-by: Quincy Cheng <[email protected]>
Co-authored-by: Joe Garcia <[email protected]>

* Improve logon with 2FA

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.

* Fix lint errors and add missing file from previous commit

This commit fixes the go  lint error related to key type in context and it
adds a missing file from previous commit not uploaded by error.

* Fix type error in comment

* Missing comment on Cookie function

Added missing comment

* Fix panic when body response is nil

Try to catch nil response body on httpjson.SendRequestRaw function

Co-authored-by: Andrew Copeland <[email protected]>
Co-authored-by: Quincy Cheng <[email protected]>
Co-authored-by: Joe Garcia <[email protected]>
@infamousjoeg infamousjoeg added enhancement New feature or request customer This issue submitted by customer labels May 7, 2021
@sonarcloud
Copy link

sonarcloud bot commented May 7, 2021

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

@infamousjoeg infamousjoeg deleted the mdaguete-improve-2fa branch February 27, 2022 03:54
@Infraded
Copy link

Infraded commented Nov 7, 2023

Hi @infamousjoeg, I was trying to get cybr-cli working with an MFA/RADIUS protected instance and found this and #114 while troubleshooting. The issues fixed in the original PR seem to still be present and prompts for MFA challenges just get 403s. It's been a while, and I'm not sure why this didn't make the original merge, but any chance on getting this actually merged in?

@infamousjoeg infamousjoeg restored the mdaguete-improve-2fa branch November 8, 2023 13:14
@infamousjoeg
Copy link
Owner Author

@Infraded, extremely interesting. It seems like it was staged for release in an early beta version, somehow got untracked from the project and orphaned, then 2 years later when I was cleaning branches, it was deleted. Let me reopen this, restore the branch, and re-investigate.

Once I get the branch synchronized with main, I'll ask for a quick test to ensure the fixes on the branch are still good to go.

@infamousjoeg infamousjoeg reopened this Nov 8, 2023
Copy link

sonarcloud bot commented Nov 8, 2023

Kudos, SonarCloud Quality Gate passed!    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

@infamousjoeg
Copy link
Owner Author

@Infraded,

Let's continue this over on #214 where I added this code into the current code base. I am hoping you can help test the build.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customer This issue submitted by customer enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants