Added the ability to log in when using one time passwords. #13
Conversation
|
I forgot to compile, so please use squash commit to complete this PR in one commit. |
| @@ -44,12 +45,17 @@ export function loginRequest(params: LoginRequestParamsType): Promise<LoginRespo | |||
| debug(`Making request to ${BASE_API_URL + LOGIN_URL_PATH} with options:`) | |||
| debug(JSON.stringify(requestOptions, null, 2)) | |||
| fetch(BASE_API_URL + LOGIN_URL_PATH, requestOptions) | |||
| .then((res) => { | |||
| if (!payload.oneTimePassword) return res | |||
There was a problem hiding this comment.
I think this could be a break point because res is not a promise that is going to be result so the next then step maybe is not going to be triggered never??
There was a problem hiding this comment.
It should not be, but I'll test it out.
Hello Daniel!!
First pull the last changes of master branch.
Your changes break one login test
Provide a new custom test for your new awesome feature please to ensure this lib always works
After that I will merge your pull request for sure!Thank you
Unfortunately tests do not work out of the box. What I still had to do to get them working:
- npm i -g mocha ts-node
- I had to install something else but unfortunately I forgot what it was.
Returning from within a .then() should always allow the chain to continue.
Once I got the tests working, unfortunately my account got blocked. Reason being that a one time password can be used only once, and the tests fire numerous times with the same OTP. To make this work, the tests should only log in once, and then reuse that session. For tests like the logout, it should then prompt for a new OTP.
For now, once I get my account reactivated, I will be suspending my own testing, and simply keep using my own fork, as that one does work for me.
src/DeGiro.ts
Outdated
| @@ -101,7 +104,7 @@ export class DeGiro implements DeGiroClassInterface { | |||
| login(): Promise<AccountDataType> { | |||
| if (this.jsessionId) return this.loginWithJSESSIONID(this.jsessionId) | |||
| return new Promise((resolve, reject) => { | |||
| loginRequest({ username: this.username, pwd: this.pwd }) | |||
| loginRequest({ username: this.username, pwd: this.pwd, oneTimePassword: this.oneTimePassword }) | |||
There was a problem hiding this comment.
Maybe should put in 3 new lines instead of once
There was a problem hiding this comment.
Agreed, resolved in upcoming commit.
|
Okay, you are right. Re-use session on test set is a great nice-to-have. I'm going to merge your code and check if pass the tests with my account that has not OTP. Good job and thank you! |
|
Why do you perform two requests with OTP -- first the one without the OTP, followed by the one with OTP? It appears sending the one with OTP directly works as well. |
|
Could be a great pull requests! You're right! |
|
Hi, I won't be creating PRs as I'm working on a competing implementation (WIP for Rust), but I'm happy to share my findings as I go along :) On that note, from my testing, it appears the Would you like me to file issues whenever I stumble upon things? |
No description provided.