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 degiro keys setup and improve 2FA authentication #2234

Merged
merged 8 commits into from
Aug 3, 2022
Merged

Conversation

montezdesousa
Copy link
Contributor

@montezdesousa montezdesousa commented Aug 2, 2022

Description

This PR fixes the issue with setting up username and password for Degiro api in keys raised in #2143 by @JerBouma. It also adds a new way to login for users with 2FA authentication enabled on Degiro through login command in degiro sub-menu.

  • Summary of the change / bug fix.
  • Relevant motivation and context.

How has this been tested?

  • Go to keys and type credentials degiro -u USERNAME -p PASSWORD
  • Go to /portfolio/bro/degiro and type login if no 2FA
  • Go to /portfolio/bro/degiro and type login -otp ONETIMEPASSWORD if 2FA enabled

Others

  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • My code passes all the checks pylint, flake8, black, ... To speed up development you should run pre-commit install.
  • New and existing unit tests pass locally with my changes. You can test this locally using pytest tests/....

@montezdesousa montezdesousa added bug Fix bug enhancement Enhancement labels Aug 2, 2022
@JerBouma
Copy link
Contributor

JerBouma commented Aug 2, 2022

It doesn't matter for me:

2022 Aug 02, 09:47 (🦋) /portfolio/bro/degiro/ $ login
Error: 400 Client Error: Bad Request for url: https://trader.degiro.nl/login/secure/login

You are now logged in !
2022 Aug 02, 09:47 (🦋) /portfolio/bro/degiro/ $ hold
Error: Connection required.

0 position found.
2022 Aug 02, 09:47 (🦋) /portfolio/bro/degiro/ $

Even after a completely fresh installation of the terminal.

@montezdesousa
Copy link
Contributor Author

It doesn't matter for me:

2022 Aug 02, 09:47 (🦋) /portfolio/bro/degiro/ $ login
Error: 400 Client Error: Bad Request for url: https://trader.degiro.nl/login/secure/login

You are now logged in !
2022 Aug 02, 09:47 (🦋) /portfolio/bro/degiro/ $ hold
Error: Connection required.

0 position found.
2022 Aug 02, 09:47 (🦋) /portfolio/bro/degiro/ $

Even after a completely fresh installation of the terminal.

Can you try leaving and reentering the terminal after setting the keys and then login on degiro menu pls? Just to understand if it's a similar issue I run into today.

@JerBouma
Copy link
Contributor

JerBouma commented Aug 2, 2022

It doesn't matter for me:

2022 Aug 02, 09:47 (🦋) /portfolio/bro/degiro/ $ login
Error: 400 Client Error: Bad Request for url: https://trader.degiro.nl/login/secure/login

You are now logged in !
2022 Aug 02, 09:47 (🦋) /portfolio/bro/degiro/ $ hold
Error: Connection required.

0 position found.
2022 Aug 02, 09:47 (🦋) /portfolio/bro/degiro/ $

Even after a completely fresh installation of the terminal.

Can you try leaving and reentering the terminal after setting the keys and then login on degiro menu pls? Just to understand if it's a similar issue I run into today.

Unfortunately doesn't change a thing. I still receive the same error. Could it be my password? It contains all types of symbols (e.g. #!@)

@montezdesousa
Copy link
Contributor Author

It doesn't matter for me:

2022 Aug 02, 09:47 (🦋) /portfolio/bro/degiro/ $ login
Error: 400 Client Error: Bad Request for url: https://trader.degiro.nl/login/secure/login

You are now logged in !
2022 Aug 02, 09:47 (🦋) /portfolio/bro/degiro/ $ hold
Error: Connection required.

0 position found.
2022 Aug 02, 09:47 (🦋) /portfolio/bro/degiro/ $

Even after a completely fresh installation of the terminal.

Can you try leaving and reentering the terminal after setting the keys and then login on degiro menu pls? Just to understand if it's a similar issue I run into today.

Unfortunately doesn't change a thing. I still receive the same error. Could it be my password? It contains all types of symbols (e.g. #!@)

If don't mind temporarily changing your password just to rule that out... I actually did tests with a password with special character so should not be an issue. Any ideas on what could be the problem @Chavithra ?

@JerBouma
Copy link
Contributor

JerBouma commented Aug 2, 2022

It doesn't matter for me:

2022 Aug 02, 09:47 (🦋) /portfolio/bro/degiro/ $ login
Error: 400 Client Error: Bad Request for url: https://trader.degiro.nl/login/secure/login

You are now logged in !
2022 Aug 02, 09:47 (🦋) /portfolio/bro/degiro/ $ hold
Error: Connection required.

0 position found.
2022 Aug 02, 09:47 (🦋) /portfolio/bro/degiro/ $

Even after a completely fresh installation of the terminal.

Can you try leaving and reentering the terminal after setting the keys and then login on degiro menu pls? Just to understand if it's a similar issue I run into today.

I tried and it doesn't change a thing unfortunately.

Copy link
Contributor

@Chavithra Chavithra left a comment

Choose a reason for hiding this comment

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

Looks good to me.

We need to check @JerBouma issue in another PR

@Chavithra Chavithra merged commit 950d53c into main Aug 3, 2022
@montezdesousa montezdesousa deleted the degiro_login branch August 3, 2022 08:01
@JerBouma
Copy link
Contributor

JerBouma commented Aug 3, 2022

@Chavithra @montezdesousa

Found the issue, I input my email instead of username. However, there needs to be a test that checks if it actually works. This can simply be calling login (and logging off as well) otherwise troubleshooting is tough.

│ degiro DEGIRO defined, not tested

@montezdesousa
Copy link
Contributor Author

montezdesousa commented Aug 3, 2022

@Chavithra @montezdesousa

Found the issue, I input my email instead of username. However, there needs to be a test that checks if it actually works. This can simply be calling login (and logging off as well) otherwise troubleshooting is tough.

│ degiro DEGIRO defined, not tested

Nice!! I can look into that, but if user has 2FA it will need to input it to test the keys on keys. And do the same when entering degiro menu login. Feels a bit repetitive on one hand, but on the other the user would immediately know keys are not correct.

Meanwhile this #2143 is already testable with your own ETFs then.

@xapel
Copy link

xapel commented Sep 13, 2022

I am still not able to login to Degiro. I have 2FA set up. Perhaps it is because originally I used degiro -u USERNAME -p PASSWORD -s '', because I thought I had to provide the s parameter. How do I clear the degiro information so I can start from scratch?

@montezdesousa
Copy link
Contributor Author

montezdesousa commented Sep 13, 2022

I am still not able to login to Degiro. I have 2FA set up. Perhaps it is because originally I used degiro -u USERNAME -p PASSWORD -s '', because I thought I had to provide the s parameter. How do I clear the degiro information so I can start from scratch?

Hi @xapel, API keys are stored in a .env file in your installation folder. E.g. "C:\Users\username\OpenBB.env".

We recently uploaded instructions on Degiro login with 2FA here: https://openbb-finance.github.io/OpenBBTerminal/terminal/portfolio/bro/degiro/

@xapel
Copy link

xapel commented Sep 13, 2022

Do you know where I find this file on macOS?

@xapel
Copy link

xapel commented Sep 13, 2022

I managed to find the file at /Users/<username>/.env.
I deleted the degiro entries and then restarted openBB and went into keys menu and did degiro -u USERNAME -p PASSWORD. This time there were no error messages. It just said defined, not tested
Then I went to the portfolio/bro/degiro menu.

2022 Sep 13, 04:57 (🦋) /portfolio/bro/degiro/ $ login -otp 880352
Error: 'NoneType' object is not callable

You are now logged in !
2022 Sep 13, 04:58 (🦋) /portfolio/bro/degiro/ $ hold
Error: 'NoneType' object is not callable

0 position found.
2022 Sep 13, 04:58 (🦋) /portfolio/bro/degiro/ $

@montezdesousa
Copy link
Contributor Author

I managed to find the file at /Users/<username>/.env. I deleted the degiro entries and then restarted openBB and went into keys menu and did degiro -u USERNAME -p PASSWORD. This time there were no error messages. It just said defined, not tested Then I went to the portfolio/bro/degiro menu.

2022 Sep 13, 04:57 (🦋) /portfolio/bro/degiro/ $ login -otp 880352
Error: 'NoneType' object is not callable

You are now logged in !
2022 Sep 13, 04:58 (🦋) /portfolio/bro/degiro/ $ hold
Error: 'NoneType' object is not callable

0 position found.
2022 Sep 13, 04:58 (🦋) /portfolio/bro/degiro/ $

I can reproduce your error on 1.8. installer, but seems to work fine in the main branch for next release. If this issue still persists we get back to it.

@xapel
Copy link

xapel commented Oct 11, 2022

I have upgraded to v1.9 now, but the issue is still the same for me. I cannot login to Degiro.

2022 Oct 11, 10:19 (🦋) /portfolio/bro/degiro/ $ login -otp 866218
Error: 'NoneType' object is not callable

You are now logged in !
2022 Oct 11, 10:19 (🦋) /portfolio/bro/degiro/ $ hold
Error: 'NoneType' object is not callable

0 position found.

@JerBouma
Copy link
Contributor

JerBouma commented Oct 11, 2022

I have the same issue, entered keys and entered both username and password as follows:

2022 Oct 11, 10:40 (🦋) /keys/ $ degiro -u MY_USERNAME -p MY_PASSWORD
defined, not tested

Then went into portfolio/bro/degiro:

2022 Oct 11, 10:40 (🦋) /portfolio/bro/degiro/ $ login
Error: 'NoneType' object is not callable

You are now logged in !
2022 Oct 11, 10:40 (🦋) /portfolio/bro/degiro/ $ hold
Error: 'NoneType' object is not callable

0 position found.
2022 Oct 11, 10:41 (🦋) /portfolio/bro/degiro/ $

Could you look into this @montezdesousa?

@montezdesousa
Copy link
Contributor Author

I have the same issue, entered keys and entered both username and password as follows:

2022 Oct 11, 10:40 (🦋) /keys/ $ degiro -u MY_USERNAME -p MY_PASSWORD
defined, not tested

Then went into portfolio/bro/degiro:

2022 Oct 11, 10:40 (🦋) /portfolio/bro/degiro/ $ login
Error: 'NoneType' object is not callable

You are now logged in !
2022 Oct 11, 10:40 (🦋) /portfolio/bro/degiro/ $ hold
Error: 'NoneType' object is not callable

0 position found.
2022 Oct 11, 10:41 (🦋) /portfolio/bro/degiro/ $

Could you look into this @montezdesousa?

@hjoaquim is fixing some stuff here #2747, we can try to reproduce

@hjoaquim
Copy link
Contributor

Hello @JerBouma @xapel we've now pushed new developments to main regarding the described issues related with degiro.

Would you mind try again and give us feedback, pls?

Hint: you might need to do r / reset / relaunch the terminal in order to get the defined, test passed.

@xapel
Copy link

xapel commented Oct 17, 2022

I don't know how to test from main. Is there instructions for how to do this?

@hjoaquim
Copy link
Contributor

To update your main branch:

  1. git remote --v
    image

  2. git fetch OpenBB-finance

Note that OpenBB-finance is my alias, you should look for yours with the described on (1)

image

  1. Within your fork you can do: git merge OpenBB-finance/main

Once again, OpenBB-finance is my alias!

image

After:

Just try the instructions you described above 😊

@xapel
Copy link

xapel commented Nov 9, 2022

I have the code checked out in a folder on my machine now. How do I proceed? Do I need to build it first? Or can I just run something from the terminal?

@hjoaquim
Copy link
Contributor

hjoaquim commented Nov 9, 2022

@xapel in the meantime degiro seems to have the issues fixed. But if you have some improvements on it, feel free to add a Pull Request!

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

Successfully merging this pull request may close these issues.

5 participants