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

Filter destination rooms in admin API #16882

Conversation

MisguidedEmails
Copy link

@MisguidedEmails MisguidedEmails commented Feb 1, 2024

No filtering was done, so all rooms were returned when hitting /_synapse/admin/v1/federation/destinations/<destination>/rooms

Fixes: #16883

Note: feel free to re-create this, or amend it in any way - I'm not willing to provide a legal name for the CLA

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
    • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
  • Code style is correct
    (run the linters)

No filtering existed on the rooms that were returned. We were returning
all rooms, but still showing the counter as accurate.
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@MisguidedEmails MisguidedEmails force-pushed the fix/filter-destination-rooms-admin-api branch from 8e557c7 to ba875b4 Compare February 1, 2024 22:20
@MisguidedEmails MisguidedEmails marked this pull request as ready for review February 1, 2024 22:21
@MisguidedEmails MisguidedEmails requested a review from a team as a code owner February 1, 2024 22:21
@reivilibre
Copy link
Contributor

Note: feel free to re-create this, or amend it in any way - I'm not willing to provide a legal name for the CLA

Thanks for your interest in contributing a patch to Synapse and sorry that this (seems like it) came as an unwelcome surprise when you tried to do so.

In the past we have accepted common-law names for sign-off, I don't know whether we still can with the CLA but I will check with those who do know.

@MisguidedEmails
Copy link
Author

sorry that this (seems like it) came as an unwelcome surprise when you tried to do so

I knew of the CLA, though I was expecting/hoping that I could say "yeah sure whatever, I don't care, take my stuff", but it seems it's more complex/formal than that. Also to clarify, it's not so much that it's a legal vs common name problem, but more that I'd rather not have any personal information associated with this account. (I understand it's not going to make it publicly visible, but it would be trusting whatever parties I give it to that the data will always be secure. I'd prefer not to have to do that).

@reivilibre
Copy link
Contributor

I completely understand your reasoning.

I have checked and I'm afraid the lawyers win this time: I'm afraid I can't take your contribution. Sorry.

@reivilibre reivilibre closed this Mar 21, 2024
@MisguidedEmails
Copy link
Author

Unfortunate, but I understand. Though what happens with this specific issue? The fix is small, potentially small/trivial enough that it isn't actually copyrightable code. Though if it is indeed code that is copyrighted (against my will, really), then does that mean it cannot be fixed? As I'd imagine someone else would fix it in the same manner, due to it's simplicity (unless of course there is a more elaborate fix that doesn't use the same code).

It's a bit of a bind if there are bug fixes - or I guess features too - down the line that are submitted (by me or otherwise) and that fix is then "claimed" since there is no way for the person to relinquish the copyright to Element without the CLA.

@wrjlewis
Copy link
Contributor

wrjlewis commented Apr 4, 2024

Since this is a small and trivial change as you say, we can make an exception and raise this as a PR ourselves.
Though a small caveat that we can't make guarantees on how soon we can doit, being a time constrained team.

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.

Admin API v1/federation/destinations/<destination>/rooms returns all rooms
4 participants