Skip to content

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.

@ylebre
Copy link
Member

ylebre commented Mar 3, 2025

Updated this patch in 4c7b29d.

@ylebre
Copy link
Member

ylebre commented Mar 3, 2025

Merged in #173

@ylebre ylebre closed this Mar 3, 2025
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