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.

Sign up for GitHub

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

Jump to bottom

OAuth account wizard breaks when back button is used #6574

Closed
ckamm opened this issue Jun 11, 2018 · 6 comments
Closed

OAuth account wizard breaks when back button is used #6574

ckamm opened this issue Jun 11, 2018 · 6 comments
Assignees
@ogoffart
Labels
ReadyToTest QA, please validate the fix/enhancement type:bug
Milestone
2.5.0

Comments

@ckamm
Copy link
Contributor

ckamm commented Jun 11, 2018

Steps:

  • Add a new oauth account
  • In the advanced setup page hit "Back"
  • Click "Re-open browser" and "Authorize" in the browser
  • The browser will show "Login Error, Error returned from the server: invalid_request"

Note that the back button starts out as disabled but can be enabled by changing the folder sync type.

@ogoffart @SamuAlfageme
@ckamm ckamm added the type:bug label Jun 11, 2018
@ckamm ckamm added this to the 2.5.0 milestone Jun 11, 2018
@ckamm ckamm self-assigned this Jun 11, 2018
@SamuAlfageme
Copy link
Contributor

Note that the back button starts out as disabled but can be enabled by changing the folder sync type.

Great catch @ckamm - disabling some navigation options was part of #5813 but since the wizard rework happened this might have emerged

@ckamm
Copy link
Contributor Author

ckamm commented Jun 11, 2018 •
edited
Loading

Huh. I cannot reproduce it anymore. I'm genuinely confused as to why it got reenabled for me.

@ckamm ckamm closed this as completed Jun 11, 2018
@ogoffart
Copy link
Contributor

I can reproduce! It gets re-enabled when there is an error at the last step.

@ogoffart ogoffart reopened this Jun 18, 2018
@ogoffart ogoffart assigned ogoffart and unassigned ckamm Jun 18, 2018
@ogoffart
Copy link
Contributor

ogoffart commented Jun 18, 2018 •
edited
Loading

When there is an error, OwncloudAdvancedSetupPage::updateStatus and others call completeChanged() , which is connected to QWizardPrivate::_q_updateButtonStates which will re-enable the back button

We re-open the browser, but the account's credentials already have a oauth token set. So the call to the API to get a new token fails because we use the previous token instead of using the client's secret_id.

Edit: Even if we fix to send properly the secrer_id, then, the next PROPFIND will still fails with a 401. Probably because the server is confused by all the session cookies.

ogoffart added a commit that referenced this issue Jun 18, 2018
@ogoffart
Wizard + OAuth: Make opening a new browser after clicking back works …
e838b94 
…again

Issue #6574

When there is an error in the advanced page, OwncloudAdvancedSetupPage::updateStatus
(and others) call completeChanged(), which is connected to
QWizardPrivate::_q_updateButtonStates which will re-enable the back button from the
last page.

When the user click "back" and re-open the browser, the account's credentials
already have a oauth token set. So the call to the API to get a new token fails
because we use the previous token instead of using the client's secret_id.
Fix this with the HttpCredentials::DontAddCredentialsAttribute.

Now, this is still not working because the session cookies are confusing the
server.  So we'll clear the cookies when re-opening the browser
@ogoffart ogoffart mentioned this issue Jun 18, 2018
Merged
@ogoffart ogoffart added the ReadyToTest QA, please validate the fix/enhancement label Jun 19, 2018
ogoffart added a commit that referenced this issue Jun 19, 2018
@ogoffart
Wizard + OAuth: Make opening a new browser after clicking back works …
6c2c40d 
…again

Issue #6574

When there is an error in the advanced page, OwncloudAdvancedSetupPage::updateStatus
(and others) call completeChanged(), which is connected to
QWizardPrivate::_q_updateButtonStates which will re-enable the back button from the
last page.

When the user click "back" and re-open the browser, the account's credentials
already have a oauth token set. So the call to the API to get a new token fails
because we use the previous token instead of using the client's secret_id.
Fix this with the HttpCredentials::DontAddCredentialsAttribute.

Now, this is still not working because the session cookies are confusing the
server.  So we'll clear the cookies when re-opening the browser
ogoffart added a commit that referenced this issue Jun 19, 2018
@ogoffart
Wizard + OAuth: Make opening a new browser after clicking back works …
850dc67 
…again

Issue #6574

When there is an error in the advanced page, OwncloudAdvancedSetupPage::updateStatus
(and others) call completeChanged(), which is connected to
QWizardPrivate::_q_updateButtonStates which will re-enable the back button from the
last page.

When the user click "back" and re-open the browser, the account's credentials
already have a oauth token set. So the call to the API to get a new token fails
because we use the previous token instead of using the client's secret_id.
Fix this with the HttpCredentials::DontAddCredentialsAttribute.

Now, this is still not working because the session cookies are confusing the
server.  So we'll clear the cookies when re-opening the browser
@guruz
Copy link
Contributor

guruz commented Jul 23, 2018

2.5.0 beta1 is out :-)
https://central.owncloud.org/t/desktop-sync-client-2-5-0-beta1-released/14667
Everyone, please comment here if we can close this issue. Thank you.

@jnweiger
Copy link
Contributor

jnweiger commented Aug 8, 2018 •
edited
Loading

linux mint tara

testpilotcloud version 2.5.0daily20180801 (build 9974)

Add a new oauth account
In the advanced setup page hit "Back"
Click "Re-open browser" and "Authorize" in the browser
The browser will show "Login Error, Error returned from the server: invalid_request"

The client window disappears when the browser is shown. Left-Click on the tray icon brings it back.

The first backbutton is always enabled
clicking there, visit the url again and press next, opens another browser window,

OK: connect successful

Choose what to sync. Select one folder, visit the web interface, and rename that folder
change the sync folder to some folder that has contents, ignore the warnings, press cancel.
(some other error may be needed too, to enable the second back button. It worked only once for me)

Switch to 'Manually create folder sync connections" -> the second back button gets enabled.
-> click re-open browser and auhtorize,

OK: connect successful.

@jnweiger jnweiger closed this as completed Aug 8, 2018
@camilasan camilasan mentioned this issue Sep 9, 2018
Closed
TheOneRing added a commit to TheOneRing/client that referenced this issue Sep 14, 2020
@TheOneRing
Don't clear cookies during oauth
b9ae374 
The cookies are required by the load balancer and there for a reason.
Since owncloud#6574 we fixed the back button in owncloud#8056 .
@TheOneRing TheOneRing mentioned this issue Sep 14, 2020
Merged
TheOneRing added a commit to TheOneRing/client that referenced this issue Sep 14, 2020
@TheOneRing
Don't clear cookies during oauth
31aeaf7 
The cookies are required by the load balancer and there for a reason.
Since owncloud#6574 we fixed the back button in owncloud#8056 .

Fixes: owncloud#8072
TheOneRing added a commit that referenced this issue Sep 14, 2020
@TheOneRing
Don't clear cookies during oauth
e2ce146 
The cookies are required by the load balancer and there for a reason.
Since #6574 we fixed the back button in #8056 .

Fixes: #8072
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ogoffart ogoffart

Labels
ReadyToTest QA, please validate the fix/enhancement type:bug
Projects
None yet
Milestone
2.5.0
Development

No branches or pull requests
5 participants
@ckamm @ogoffart @guruz @jnweiger @SamuAlfageme