Skip to content

Wizard: Resolve url/ redirects only if url/status.php not found#6106

Merged
ckamm merged 1 commit into2.4from
base-url-redirect-fix-5954
Oct 24, 2017
Merged

Wizard: Resolve url/ redirects only if url/status.php not found#6106
ckamm merged 1 commit into2.4from
base-url-redirect-fix-5954

Conversation

@ckamm
Copy link
Copy Markdown
Contributor

@ckamm ckamm commented Oct 19, 2017

Unfortunately checking the base-url for redirects in all cases lead
to incorrect behavior in some SAML/OAuth2 edge cases.

This new iteration checks the base url for redirects only if the
standard CheckServerJob can't reach the server. That way the 2.3
behavior is only changed in cases that would have lead to errors.

See #5954

@ckamm ckamm added this to the 2.4.0 milestone Oct 19, 2017
@ckamm ckamm self-assigned this Oct 19, 2017
@ckamm ckamm requested a review from ogoffart October 19, 2017 09:09
@SamuAlfageme
Copy link
Copy Markdown
Contributor

SamuAlfageme commented Oct 20, 2017

I think:

https://github.com/owncloud/client/blob/224d9194256c91631ea63adb1ec7fd35974d1335/src/gui/owncloudsetupwizard.cpp#L153

is causing:

10-20 10:16:01:080 [ warning default ]:	QMetaObject::invokeMethod: No such method OCC::OwncloudSetupWizard::slotContinueDetermineAuth()

on every wizard run.

Copy link
Copy Markdown
Contributor

@ogoffart ogoffart left a comment

Choose a reason for hiding this comment

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

👎 because of what samuel said. Looks good otherwise

Unfortunately checking the base-url for redirects in all cases lead
to incorrect behavior in some SAML/OAuth2 edge cases.

This new iteration checks the base url for redirects only if the
standard CheckServerJob can't reach the server. That way the 2.3
behavior is only changed in cases that would have lead to errors.

See #5954
@ckamm ckamm force-pushed the base-url-redirect-fix-5954 branch from 224d919 to 0869e8c Compare October 24, 2017 07:41
@ckamm
Copy link
Copy Markdown
Contributor Author

ckamm commented Oct 24, 2017

Fixed (and double-checked the other renames)

@ckamm ckamm merged commit f3ea375 into 2.4 Oct 24, 2017
@ckamm ckamm deleted the base-url-redirect-fix-5954 branch October 24, 2017 07:42
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.

3 participants