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

Challenge selection fix #1286

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

aaronthebaron
Copy link
Contributor

No description provided.

@aaronthebaron aaronthebaron force-pushed the challenge-selection-fix branch from 2e190c2 to 2f94cb4 Compare June 6, 2024 17:43
@aaronthebaron
Copy link
Contributor Author

@edwardrf I don't know if this helps you at all, but in this branch I put some basic work into fixing the issue you're running into with change of selection.

When you enter googleapps.go:83 you're failing out to the bottom at googleapps.go:449

Instead, I think we need to catch the error and try to look at this selection page, so in this branch I added a new function and an if block to try and catch your new selection page.

The new selection page is the html that is added to tests in this PR. I think the key is figuring out which of this attributes needs to be passed to the page:

<div class="mimsib SmR8" jsname="EBHGs" data-challengeid=11 data-action=selectchallenge data-accountrecovery="false" data-challengetype="39">

@edwardrf
Copy link
Contributor

edwardrf commented Jun 6, 2024

Thanks @aaronthebaron, I did took a look at the page and tried to select the one-time code method, but it seems it always ends up sending me to the SMS challenge page. I'll have to circle back to see what could be the reason.
Code change here: edwardrf@8644170

@aaronthebaron aaronthebaron force-pushed the challenge-selection-fix branch from 2f94cb4 to e0e9694 Compare June 7, 2024 21:24
@aaronthebaron
Copy link
Contributor Author

@edwardrf Thanks for taking a swing at it. I fixed captcha's today and will take a look at your changes on Monday if you don't beat me to it.

@codecov-commenter
Copy link

codecov-commenter commented Jun 7, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 0% with 22 lines in your changes missing coverage. Please review.

Project coverage is 42.05%. Comparing base (99d6fe4) to head (5a90874).
Report is 39 commits behind head on master.

Files Patch % Lines
pkg/provider/googleapps/googleapps.go 0.00% 22 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1286      +/-   ##
==========================================
- Coverage   42.19%   42.05%   -0.15%     
==========================================
  Files          54       54              
  Lines        6456     6478      +22     
==========================================
  Hits         2724     2724              
- Misses       3283     3305      +22     
  Partials      449      449              
Flag Coverage Δ
unittests 42.05% <0.00%> (-0.15%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@aaronthebaron
Copy link
Contributor Author

@edwardrf I think I see what's happening with your code. I think two elements are required, a challengeId and a challengeType. You have one but not the other.

The types also appear to be in the HTML

<div class="mimsib SmR8 RDPZE" jsname="EBHGs" data-challengeid=10 data-action=selectchallenge data-challengeunavailable="true" data-accountrecovery="false" data-challengetype="2">

So it seems like you are on the right track?

@mapkon
Copy link
Contributor

mapkon commented Jul 8, 2024

What is the plan for this PR?

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