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

Fix Login OTP #199

Merged
merged 3 commits into from
Jan 11, 2025
Merged

Fix Login OTP #199

merged 3 commits into from
Jan 11, 2025

Conversation

Winding6636
Copy link
Contributor

Check the redirect URL and see if the OTP is valid.
Changed because the username was no longer displayed on the OTP screen.

Added OTP check
Changed because the username was no longer displayed on the OTP screen.
@AlexAplin
Copy link
Owner

div.pageMainMsg span.userAccount appears to be present still when I see the OTP page. Do you have a screenshot/log output showing what changed?

@Winding6636
Copy link
Contributor Author

I will send you a screenshot and the error.

catsxp_vdGDxHtC6x

Error message:

Logging in...
AttributeError: 'NoneType' object has no attribute 'text'
Traceback (most recent call last):
  File "nndownload.py", line 3, in <module>
    cli()
  File "/mnt/t/Develop/Git/niconico/orig_nndownload/nndownload/nndownload.py", line 2193, in cli
    main()
  File "/mnt/t/Develop/Git/niconico/orig_nndownload/nndownload/nndownload.py", line 2156, in main
    session = login(account_username, account_password, session_cookie)
  File "/mnt/t/Develop/Git/niconico/orig_nndownload/nndownload/nndownload.py", line 1990, in login
    otp_code_account = otp_code_page.select_one("div.pageMainMsg span.userAccount").text
AttributeError: 'NoneType' object has no attribute 'text'
print(otp_code_page.select_one("div.pageMainMsg"))

WindowsTerminal_vRVBEaFHkv

I'm accessing this from Japan, but it may be different overseas.

Is it better to add a check if "span.userAccount" exists?

@AlexAplin
Copy link
Owner

AlexAplin commented Dec 11, 2024

That's interesting, based on the message I guess it has to do if whether you set up 2FA on your account?

Either way I think the message needs to change then. Probably what we should do is capture the text of div.pageMainMsg and display that when prompting for the code, instead of Enter the OTP code sent to the email/telephone on file for your account ({}):

Branch by identifying two types of authentication applications and email transmissions.
@Winding6636
Copy link
Contributor Author

Sorry, I checked the NicoNicohelp again and it says it right....
https://qa.nicovideo.jp/faq/show/4500?site_domain=default

There was a way to verify the OTP code via the authentication app and email. (I thought it was only app authentication)
*It seems that email is mandatory overseas?

After confirming the migration of the 2-step verification page, I changed the message to confirm the code from the email if 'span.userAccount' is available and from the authentication app if not.

Please let me know if there is a smarter way to write this.

I verified and confirmed three authentication patterns.
・2FA not set (password only)
・2FA set by email
・2FA set with an authentication app

@AlexAplin
Copy link
Owner

AlexAplin commented Dec 22, 2024

If you can, change the two URL checks to use urlparse:

from urllib.parse import urlparse
[...]
            parsed_login_request_url = urlparse(login_request.url)

            if "message=cant_login" in parsed_login_request_url.query:
                raise AuthenticationException("Incorrect email/telephone or password. Please verify your login details")
            if parsed_login_request_url.path == "/mfa":
                otp_code_request = session.get(login_request.url)
                [...]

Thanks for investigating!

@AlexAplin AlexAplin merged commit e6814fe into AlexAplin:master Jan 11, 2025
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.

2 participants