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

Enable guest access on new media endpoints, per MSC4189 #17675

Merged
merged 2 commits into from
Sep 10, 2024

Conversation

turt2live
Copy link
Member

I didn't see tests for other guest access endpoints in my 15 seconds of searching, but if there's guidance on how to set that up, happy to add the respective tests.

@turt2live turt2live requested a review from a team as a code owner September 5, 2024 20:03
Copy link
Member

@anoadragon453 anoadragon453 left a comment

Choose a reason for hiding this comment

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

It appears that we don't have any unit tests for guest access. Sytest appears to be where guest access was historically tested. If you're feeling adventurous, you could add a subtest to https://github.com/matrix-org/complement/blob/39733c1b2f8314800776748cc7164f9a34650686/tests/csapi/apidoc_content_test.go#L32-L56, which is where authenticated media appears to be primarily tested.

I don't believe this needs an experimental features option due to the simplicity. The MSC looks poised to go through and disabling this in production is easy given the small code size.

Feel free to add a test in Complement to verify, but for now this lgtm.

@anoadragon453 anoadragon453 merged commit a7fcac5 into develop Sep 10, 2024
37 of 39 checks passed
@anoadragon453 anoadragon453 deleted the travis/guest-access-msc4189 branch September 10, 2024 17:29
@turt2live
Copy link
Member Author

I am feeling adventurous, but unfortunately my bandwidth constraints prevent realizing that interest :(

maybe one day, though.

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.

2 participants