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

No session resumption on different phone number #56

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

MartinScharrer
Copy link
Contributor

@MartinScharrer MartinScharrer commented Mar 14, 2024

Added code to compare given phone number with the one in the credential file and disable websession resumption if so.
For this to work well I also added a command line option to select if credentials should be saved or not.
As a bonus a fix for corrupt cookie files got added.

This pull-request fixes #38.

…al file and start a new websession if so.

Added command line argument to manage saveing of the credential file without asking.
pytr/account.py Outdated
'''
If web is true, use web login method as else simulate app login.
Check if credentials file exists else create it.
If no parameters are set but are needed then ask for input
'''
log = get_logger(__name__)
save_cookies = True
save_credentials = (save_credentials == 'y')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just use a boolean directly?

pytr/main.py Outdated
@@ -50,6 +50,7 @@ def formatter(prog):
parser_login_args.add_argument('--applogin', help='Use app login instead of web login', action='store_true')
parser_login_args.add_argument('-n', '--phone_no', help='TradeRepublic phone number (international format)')
parser_login_args.add_argument('-p', '--pin', help='TradeRepublic pin')
parser_login_args.add_argument('-s', '--save_credentials', choices=['y', 'n'], help='Save credentials')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here. I'd just remove choices and only save the credentials if the argument is passed (with --save_credentials)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made the change to bool now.
Should I remove the pre-existing ask_for_save code as well and only save them if the switch was used?

Copy link
Collaborator

@marzzzello marzzzello left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your PR. I can merge after the argument is changed to bool

@MartinScharrer
Copy link
Contributor Author

Thanks for your PR. I can merge after the argument is changed to bool

Done.

I also now added support to credentials and cookies files arguments.

@Katzmann1983
Copy link
Collaborator

Katzmann1983 commented Jul 2, 2024

Using the current code, it seems that the active session is not correctly reused:

pytr portfolio
15:41:45 Found credentials file
15:41:45 Phone: +346********, PIN: ****
15:41:45 Credentials not saved
15:41:45 Web session resumed
15:41:45 Logged in
15:41:45 Found credentials file
15:41:45 Phone: +346********, PIN: ****
15:41:45 Phone number different from credential files. Assuming different account.
15:41:45 Credentials not saved
Enter the code you received to your mobile app as a notification.
Enter nothing if you want to receive the (same) code as SMS. (Countdown: 31)
Code: 8415
15:42:11 Logged in
15:42:11 Connecting to websocket ...
15:42:12 Connected to websocket ...

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.

Session is not invalidated when a new login with different number occurs
3 participants