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

Add riotchat to rootUrlApps #28226

Closed
wants to merge 1 commit into from
Closed

Conversation

gary-kim
Copy link
Member

@Sorunome is working on a Matrix integration for Nextcloud but it requires root url routes for some of the Matrix routes so this PR adds riotchat to the allowed root url apps.

gary-kim/riotchat#369

Signed-off-by: Gary Kim <gary@garykim.dev>
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Not a huge fan but 🤷

@juliushaertl
Copy link
Member

Can you maybe elaborate a bit further why those are needed in the root and why having them under the app url-subspace doesn't work for that use case?

@Sorunome
Copy link

Sure!

The idea was to easily share media (images etc.) from nextcloud into matrix. (Matrix is a federated chat protocol.) For that, nextcloud could act as a matrix media repository. The matrix protocol, however, requires those endpoints to start with /_matrix/media, sub-paths are unfortunately not possible. Thus, root URLs are required so that nextcloud can act as a matrix media repository. Other, non-matrix-protocol-related endpoints ofc still use the app namespace.

@LukasReschke
Copy link
Member

Is there any chance Matrix can add support for .well-known URLs for discovery? I can see the use-case here, but this seems a tad hacky :)

@Sorunome
Copy link

Is there any chance Matrix can add support for .well-known URLs for discovery? I can see the use-case here, but this seems a tad hacky :)

Matrix already has .well-known for discovering server domains, but not for paths. There is a semi-related issue open to this already, about pushers (push notification targets). There does not seem to be an issue to add such configurability to all endpoints, and thus the path forward for that would be very slow etc., having to go through an MSC (Matrix Spec Change) process, etc.

@szaimen szaimen added the 3. to review Waiting for reviews label Aug 31, 2021
@skjnldsv skjnldsv mentioned this pull request Oct 13, 2021
@skjnldsv skjnldsv modified the milestones: Nextcloud 23, Nextcloud 24 Oct 21, 2021
@skjnldsv
Copy link
Member

Matrix already has .well-known for discovering server domains, but not for paths. There is a semi-related issue open to this already, about pushers (push notification targets).

then I think this is an issue with matrix. Implementing workarounds here does not seems to be the right solution imho. I'm in favour of closing :)

@skjnldsv skjnldsv mentioned this pull request Mar 24, 2022
@blizzz blizzz mentioned this pull request Mar 31, 2022
This was referenced Apr 7, 2022
@blizzz blizzz modified the milestones: Nextcloud 24, Nextcloud 25 Apr 21, 2022
This was referenced Aug 12, 2022
This was referenced Aug 24, 2022
This was referenced Sep 6, 2022
@skjnldsv skjnldsv mentioned this pull request Sep 15, 2022
This was referenced Sep 20, 2022
@blizzz blizzz modified the milestones: Nextcloud 25, Nextcloud 26 Sep 22, 2022
@blizzz blizzz mentioned this pull request Feb 1, 2023
@skjnldsv skjnldsv mentioned this pull request Feb 23, 2023
@blizzz blizzz mentioned this pull request Mar 7, 2023
@blizzz blizzz modified the milestones: Nextcloud 26, Nextcloud 27 Mar 9, 2023
This was referenced May 3, 2023
@ChristophWurst
Copy link
Member

Pushed over for two years. I'm assuming we can at least unassign from the milestone. @gary-kim is this change still required?

@ChristophWurst ChristophWurst added 0. Needs triage Pending check for reproducibility or if it fits our roadmap and removed 3. to review Waiting for reviews labels May 10, 2023
@skjnldsv skjnldsv removed this from the Nextcloud 27 milestone May 10, 2023
@juliushaertl juliushaertl removed their request for review May 28, 2023 15:28
@Thatoo
Copy link

Thatoo commented Jun 26, 2023

We'd really like to be able to share media (files, images etc.) from nextcloud into matrix.
I read the thread and didn't understand the reason it has been rejected till now.
I only read a mention of not "liking the idea" but no real argument. Why? For what reason? Security?

Yes, this issue is still needed unless a Matrix MSC is established : matrix-org/matrix-spec#693

Can't riotchat be added to rootUrlApps along spreed? At least untill Matrix handle the .well-known solution to manage subpath of /_matrix/ ?

@skjnldsv skjnldsv added 2. developing Work in progress and removed 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Feb 21, 2024
@skjnldsv
Copy link
Member

Matrix already has .well-known for discovering server domains, but not for paths. There is a semi-related issue open to this already, about pushers (push notification targets).

then I think this is an issue with matrix. Implementing workarounds here does not seems to be the right solution imho. I'm in favour of closing :)

still no attraction in 3 years. Closing 😶‍🌫️

@skjnldsv skjnldsv closed this Feb 27, 2024
@skjnldsv skjnldsv deleted the enh/noid/add-riotchat-root-apps branch February 27, 2024 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants