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

add another regex for finding state tokens #1064

Merged
merged 3 commits into from
May 31, 2023

Conversation

lizduty
Copy link
Contributor

@lizduty lizduty commented May 29, 2023

I've been having a weird intermittent issue, where trying to log in would sometimes fail with an opaque message:

Error authenticating to IdP.: failed to getStateToken: error retrieving saml response: cannot find state token

After a bunch of digging, I discovered that sometimes Okta will request "extra verification" in the form of another 2fa request. Not a big deal, but for whatever reason that page doesn't store the state token as a single variable. Instead, it's burried deep in a blob of almost-JSON.

This change lets us dig up state tokens from those pages as well, which mostly fixes the "cannot find state token" message

@codecov-commenter
Copy link

Codecov Report

Merging #1064 (fda8a76) into master (ba48cfb) will increase coverage by 0.05%.
The diff coverage is 100.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##           master    #1064      +/-   ##
==========================================
+ Coverage   36.23%   36.29%   +0.05%     
==========================================
  Files          53       53              
  Lines        7934     7941       +7     
==========================================
+ Hits         2875     2882       +7     
  Misses       4686     4686              
  Partials      373      373              
Flag Coverage Δ
unittests 36.29% <100.00%> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/provider/okta/okta.go 22.19% <100.00%> (+0.44%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@mapkon mapkon merged commit c00663f into Versent:master May 31, 2023
@gempesaw
Copy link

gempesaw commented Jun 8, 2023

woohoo thank goodness for this PR, i could only auth in once before getting this error and getting stuck. but i built latest locally and now it's working like a charm, i can login as many times as i want :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants