-
Notifications
You must be signed in to change notification settings - Fork 16
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
Integrate #114 to improve logon with 2FA #214
base: main
Are you sure you want to change the base?
Conversation
Can you please build and test this locally to see if it fixes the problems you're experiencing?
|
Tested and got a failure after password input.
|
Okay, I fixed the nil pointer dereference. Please give it another shot, @Infraded. |
No more nil pointer dereference, but getting this now after password:
On the older builds I was getting the "expected" 500 responses and I've confirmed that I can get through via the API directly |
Added some debug outputs and it seems like here: https://github.com/infamousjoeg/cybr-cli/pull/214/files#diff-34ff7a443be24db6bbf225c6913a8c0f097b382c1f5dc6f6de688935e09e5a88R173-R175 is where it dies. It gets a 500 response back for ErrorCode |
Thanks @Infraded. I apologize that I don't have a radius server setup to test this on. I've implemented a conditional in an attempt to handle status code 500 and the existence of ITATS542I. I'm hoping this is the ticket: cybr-cli/pkg/cybr/helpers/httpjson/httpjson.go Lines 183 to 189 in 8e7635a
|
Partial success, with some more tweaking. cybr-cli/pkg/cybr/helpers/httpjson/httpjson.go Lines 173 to 176 in 8e7635a
getResponse which looks to be doing the same checking sendRequestRaw was doing cybr-cli/pkg/cybr/helpers/httpjson/httpjson.go Lines 135 to 137 in 8e7635a
When I bypass that first err check in |
I want to avoid removing that error check that you bypassed. The 500 status code shouldn't be causing it to error there. That should just be for network and protocol errors of the net/http Do method. What I did find, however, is in the logic of cmd/logon.go a conditional that wasn't checking for the error code to prompt for OTP. So, I made changes to that so that it prompts for OTP when the code is detected. If you need to, please manually bypass the |
I wouldn't want to remove that check either, but I'm hacking through it locally, but am far from proficient in Go. Bypassing the error catch still and making sure |
@Infraded I got it! I got my hands on a self-hosted PAM lab with Radius configured. I needed to implement AND/OR as needed for when ITATS542I was contained in the response body. This should be prompting for one-time passcode now: The problem now is handling the OTP code afterwards. Troubleshooting that portion now... PROGRESS! |
Used gorilla/sessions instead of net/http and it is working using that. I don't know that I want to switch everything over to using gorilla/sessions, though. Going to continue to investigate and use that as a last resort. |
I was definitely able to get a token back from the API manually with challenge/response. Our usage has two sets of challenges and even that was OK when manually hitting with Insomnia. Sounds like you had some success though, is it in a state to test or still working on it? |
* Update ci.yml for Pineapple * Fix indentation error * Revert to previous checkout/install * Add /api to CC URL * move to ubuntu-latest hosted runner * switch from ldap to identity * Change test account to cyberark * Change PAS_ADDRESS to PAS_HOSTNAME * removed sleep * Update IDs for SaaS * accountSafeName _ to - * Create new test account * Add Content-Length to request header * Add empty struct for body on POST * Add emptyBody to logoff * Change ListSafeMembers.Members.value.MemberId to interface{} * Update RemoveSafeMember from v1 to v2 API * Add MemberType to Add Safe Member test * Removed early DeleteSafe * Add emptyBody to UnsuspendUser * Add Create Temp PEM Files step * Update CCP variable paths * Base64 decode CCP Certs * Add CCP_HOSTNAME env var * Move test workflow to self-hosted runner * Update CCP_CLIENT_PRIVATE_KEY
Quality Gate passedThe SonarCloud Quality Gate passed, but some issues were introduced. 3 New issues |
@infamousjoeg Any further thoughts on this? Is the current state of this branch worth testing or is there more to do still? |
@Infraded there's something with the way Go needs to handle the cookies when dealing with the challenge/response that I just cannot figure out. PowerShell does it natively, so it's not something that anyone with experience there has encountered before. I did go to the Discord Community in hopes that a customer or partner may have ran into this in the past and they had. Unfortunately, no one had any answers or workarounds outside of "Use PowerShell". This is still on my radar, but it's going to take time for me to learn what is going on. Any community contributions in the meantime are welcomed! |
From my testing, it just needs to keep cookies during the challenge/response calls, until it gets the token. And also not have any old ones when it starts that process. I've got a script now just using a cookie jar w/ curl that works ok. I don't know the mechanisms for that in Go (yet), but will keep poking around too. Appreciate your work on this! |
This should fix 403 issues when attempting a challenge/response via MFA on self-hosted PAS.