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

Integration Manager: Wrong URL to review terms if URL in config contains path #1606

Closed
dontub opened this issue Jul 2, 2020 · 2 comments
Closed
Assignees

Comments

@dontub
Copy link

dontub commented Jul 2, 2020

My homeserver uses Dimension as integration manager. So RiotX fetches https://<dimension>/riot as integrtionUIUrl and https://<dimension>/api/v1/scalar as integrationRestUrl via Integration manager discovery MSC1957.

If the terms of service are not signed, the URL https://<dimension>/api/v1/scalar gets passed as baseUrl to
https://github.com/vector-im/riotX-android/blob/e878821df2fdad04279a1f1d9b6c012095b674cf/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/terms/DefaultTermsService.kt#L103
This method adds /_matrix/integrations/v1/ to the URL which ends up in a request to https://<dimension>/api/v1/scalar/_matrix/integrations/v1/terms instead of https://<dimension>/_matrix/integrations/v1/terms to review the terms.

In my understanding of MSC2140 the path /_matrix/integrations/v1 is meant to be absolute. Thus the path in baseUrl should be dropped before appending this path. I think this could be done in the method linked above or at
https://github.com/vector-im/riotX-android/blob/e878821df2fdad04279a1f1d9b6c012095b674cf/vector/src/main/java/im/vector/riotx/features/widgets/WidgetViewModel.kt#L239

@ganfra
Copy link
Member

ganfra commented Jul 8, 2020

Thanks for the catch!

@bmarty
Copy link
Member

bmarty commented Jul 11, 2020

Fixed in #1665

@bmarty bmarty closed this as completed Jul 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants
@bmarty @dontub @ganfra and others