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

Improve setup checks and url handling with separate callback url #3315

Merged
merged 5 commits into from
Dec 1, 2023

Conversation

juliushaertl
Copy link
Member

@juliushaertl juliushaertl commented Nov 23, 2023

Improve handling of different URLs. From now on we will allow to configure:

  • wopi_url Used by Nextcloud to connect to Collabora in the backend
  • wop_callback_url Passed to collabora to connect back to Nextcloud (optional, determined from the browser URL if not set)

The public_wopi_url which was only partly working is no longer ment to be manually set and will be overwritten depending on the /hosting/discovery response.

Further this PR improves:

  • Add setup check on occ and admin page
  • Give proper error on individual failures of the setup check
  • Display configured and detected URLs to make setup issue debugging easier
  • Refactor services to have a cleaner structure for setup checks, discovery and capabilities fetching
  • Add frontend connectivity check in the admin settings

Note: One thing we cannot check as of right now is if the Collabora server is properly configured. This could be achieved continuing the implementation effort on CollaboraOnline/online#4353

Fix #3313
Fix #3262
Fix #1497
Fix #1835

occ command

occ rich:setup -w "http://collabora:9980" -c "https://nextcloud.local"
✓ Set WOPI url to http://collabora:9980
✓ Set callback url to https://nextcloud.local
Checking configuration
🛈 Configured WOPI URL: http://collabora:9980
🛈 Configured public WOPI URL: https://collabora.local
🛈 Configured callback URL: https://nextcloud.local

✓ Fetched /hosting/discovery endpoint
✓ Valid mimetype response
✓ Valid capabilities entry
✓ Fetched /hosting/capabilities endpoint
✓ Detected WOPI server: Collabora Online Development Edition 23.05.5.4

Collabora URL (used for Nextcloud to contact the Collabora server):
  http://collabora:9980
Collabora public URL (used in the browser to open Collabora):
  https://collabora.local
Callback URL (used by Collabora to connect back to Nextcloud):
  https://nextcloud.local

Screenshots of new admin settings hints

Screenshot 2023-11-23 at 11 32 21 Screenshot 2023-11-23 at 11 32 35 Screenshot 2023-11-23 at 11 34 27 Screenshot 2023-11-23 at 10 57 51

@juliushaertl juliushaertl force-pushed the fix/url-handling branch 2 times, most recently from 1afbfcb to 2ac99ec Compare November 23, 2023 10:38
@juliushaertl juliushaertl added enhancement New feature or request 3. to review Ready to be reviewed labels Nov 23, 2023
@juliushaertl juliushaertl mentioned this pull request Nov 23, 2023
6 tasks
@juliushaertl
Copy link
Member Author

@joshtrichards I'm unsure if you have a development setup, but I'd be curious about your feedback around this given that you did quite some issue triaging efforts. This PR aims to introduce one additional setting but the core part is to improve error reporting for admins. Maybe you have a way to give it a try? Or just give some feedback on further improving the error messages?

Improve handling of different URLs. From now on we will allow to
configure:

- wopi_url Used by Nextcloud to connect to Collabora in the backend
- wop_callback_url Passed to collabora to connect back to Nextcloud
  (optional, determined from the browser URL if not set)

The public_wopi_url which was only partly working is no longer ment to
be manually set and will be overwritten depending on the
/hosting/discovery response.

Further this PR improves:
- Add setup check on occ and admin page
- Give proper error on individual failures of the setup check
- Display configured and detected URLs to make setup issue debugging
  easier
- Refactor services to have a cleaner structure for setup checks,
  discovery and capabilities fetching

Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Copy link

@Sprinterfreak Sprinterfreak left a comment

Choose a reason for hiding this comment

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

Tested on unicode domain using server 27.1.4. With commit 4aecf93 applied, richdocuments doesn't break nextcloud anymore.

Fixes #3086 again

@juliushaertl juliushaertl merged commit db7610b into main Dec 1, 2023
57 checks passed
@juliushaertl juliushaertl deleted the fix/url-handling branch December 1, 2023 14:58
@juliushaertl
Copy link
Member Author

/backport to stable28

joshtrichards added a commit that referenced this pull request Dec 25, 2023
Fixes #3363. The refactor in #3315 swapped around a few methods and this updates Application.php to use the appropriate ones.

Signed-off-by: Josh Richards <josh.t.richards@gmail.com>
backportbot-nextcloud bot pushed a commit that referenced this pull request Dec 27, 2023
Fixes #3363. The refactor in #3315 swapped around a few methods and this updates Application.php to use the appropriate ones.

Signed-off-by: Josh Richards <josh.t.richards@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Ready to be reviewed enhancement New feature or request
Projects
None yet
3 participants