Skip to content

Conversation

@tobiasKaminsky
Copy link
Member

@AndyScherzinger
Copy link
Member

I thought INVALID_SESSION_TOKEN is the indicator for using and old login, is it not?

@mario
Copy link
Contributor

mario commented Sep 20, 2017 via email

@AndyScherzinger
Copy link
Member

Then imho this needs to be the other way round:

if (isInvalidSessionToken(response)) {
    result = new RemoteOperationResult(false, status, post.getResponseHeaders());
} else {
     result = new
RemoteOperationResult(RemoteOperationResult.ResultCode.ACCOUNT_USES_OLD_LOGIN);
}

@mario
Copy link
Contributor

mario commented Sep 20, 2017 via email

@mario
Copy link
Contributor

mario commented Sep 20, 2017

@tobiasKaminsky it is, but the issue is not in the name but where it should go. Also this will NOT trigger if you use old-login with app password. So maybe: ACCOUNT_USES_STANDARD_PASSWORD instead?

@tobiasKaminsky
Copy link
Member Author

Thanks @AndyScherzinger, I fixed it.

@mario
Copy link
Contributor

mario commented Sep 20, 2017

@AndyScherzinger @tobiasKaminsky thoughts on replacing the name to better signify what the actual problem is?

@tobiasKaminsky
Copy link
Member Author

Also this will NOT trigger if you use old-login with app password.

I tried this, I logged into NC12 server with old login method (wait 60s on login and choose fallback). And then this was triggered.
Do I miss something?

@mario
Copy link
Contributor

mario commented Sep 20, 2017

Well what I'm saying is ... if you login with an old login method to NC12 with the app password you generated yourself, Push should work and you should not get this error AFAIK.

@tobiasKaminsky
Copy link
Member Author

Now I got it :-)
I will change the name to ACCOUNT_USES_STANDARD_PASSWORD

@AndyScherzinger AndyScherzinger added this to the NC lib 1.0.31 milestone Sep 20, 2017
@AndyScherzinger
Copy link
Member

AndyScherzinger commented Sep 20, 2017

👍 all comments implemented afaik + needs rebase now :)

Approved with PullApprove

@AndyScherzinger
Copy link
Member

let's get this merged 🚀 @mario 😃

@mario
Copy link
Contributor

mario commented Sep 21, 2017

👍

Approved with PullApprove

1 similar comment
@AndyScherzinger
Copy link
Member

AndyScherzinger commented Sep 21, 2017

👍

Approved with PullApprove

@AndyScherzinger AndyScherzinger merged commit 2f5ddc4 into master Sep 21, 2017
@AndyScherzinger AndyScherzinger deleted the pushWarning branch September 21, 2017 09:41
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