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

Use request scheme in server url #121

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

NoelDeMartin
Copy link

Hey, I've been playing with this in a local Nextcloud instance and could make it work after commenting a couple of things.

I think most of the issues were related to networking, so I don't think there was anything broken in the plugin (my app's clientid wasn't reachable from the Nextcloud server, etc.).

But something I found to improve is that it's assuming that instances run with https://, which was not true in my case. Hopefully everyone uses https:// in production though :).

@Potherca
Copy link
Member

Potherca commented May 3, 2023

Thank you for taking the time to report this!

Originally, the hard-coded https was to satisfy the Solid spec, specifically:

2.1 HTTP Server

[..] When both http and https URI schemes are supported, the server MUST redirect all http URIs to their https counterparts using a response with a 301 status code and a Location header.

This was in part to protect unknowing users against themselves.

At some point, the issue of being able to run the solid pod over http did come up. In the Standalone PHP Server, this was resolved by allowing http by explicitly setting PROXY_MODE (see the "Usage" section).

It looks like a similar solution was omitted in this plugin 😞

I'll discuss this internally and see what resolution we come up with.

@Potherca
Copy link
Member

I've moved the internal discussion into the open at https://github.com/orgs/pdsinterop/discussions/2
I'll add an update here once the discussion has been resolved.

@Potherca
Copy link
Member

The consensus reached in the linked discussion is to remove the HTTP(S) check completely and let the server / environment decide.

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