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

Fix permission and 404 response for alias deletion - #654 #706

Merged
merged 8 commits into from Aug 7, 2019

Conversation

@Cnly
Copy link
Collaborator

commented May 28, 2019

This PR should fix #654. Checklist from the issue:

  • Keep track of an alias's owner (someone else shouldn't be able to delete an alias I set)
  • DELETE /directory/room/{roomAlias} should 404 if the alias doesn't exist

Signed-off-by: Alex Chen minecnly@gmail.com

Pull Request Checklist

  • I have added any new tests that need to pass to testfile as specified in docs/sytest.md
  • Pull request includes a sign off
Fix permission and 404 response for alias deletion - #654
Signed-off-by: Alex Chen <minecnly@gmail.com>

@Cnly Cnly force-pushed the Cnly:fix-alias-deletion-654 branch from 4216ac2 to aea0b36 Jun 22, 2019

@Cnly Cnly changed the title [WIP] Fix permission and 404 response for alias deletion - #654 Fix permission and 404 response for alias deletion - #654 Jun 25, 2019

@anoadragon453 anoadragon453 added this to In progress in Homeserver Task Board via automation Jul 9, 2019

@anoadragon453 anoadragon453 moved this from In progress to Community PRs in Homeserver Task Board Jul 9, 2019

@anoadragon453 anoadragon453 self-assigned this Jul 9, 2019

@anoadragon453 anoadragon453 requested a review from matrix-org/dendrite-core Jul 11, 2019

@anoadragon453

This comment has been minimized.

Copy link
Member

commented Jul 11, 2019

I'd like to get an answer to this before reviewing.

@anoadragon453
Copy link
Member

left a comment

There's less bookkeeping we need to do if we're allowing room admins to delete room aliases, so this PR can be cut down quite considerably.

clientapi/routing/directory.go Outdated Show resolved Hide resolved
clientapi/routing/directory.go Outdated Show resolved Hide resolved
Apply suggestions from code review
Co-Authored-By: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
@Cnly

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 25, 2019

There's less bookkeeping we need to do if we're allowing room admins to delete room aliases, so this PR can be cut down quite considerably.

I think that's still a TODO here? This PR only concerns about users deleting aliases they set.

@anoadragon453
Copy link
Member

left a comment

looks good besides some tiny nits

roomserver/api/alias.go Outdated Show resolved Hide resolved
roomserver/storage/room_aliases_table.go Outdated Show resolved Hide resolved

Cnly and others added some commits Aug 6, 2019

Apply suggestions from code review
Co-Authored-By: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
Fix alias_test.go
Signed-off-by: Alex Chen <minecnly@gmail.com>

@Cnly Cnly merged commit 94ea325 into matrix-org:master Aug 7, 2019

8 checks passed

buildkite/dendrite Build #221 passed (2 minutes, 56 seconds)
Details
buildkite/dendrite/build-slash-go-1-dot-11 Passed (52 seconds)
Details
buildkite/dendrite/build-slash-go-1-dot-12 Passed (49 seconds)
Details
buildkite/dendrite/lint-slash-go-1-dot-12 Passed (1 minute, 41 seconds)
Details
buildkite/dendrite/pipeline Passed (8 seconds)
Details
buildkite/dendrite/unit-tests-slash-go-1-dot-11 Passed (54 seconds)
Details
buildkite/dendrite/unit-tests-slash-go-1-dot-12 Passed (53 seconds)
Details
ci/circleci: dendrite Your tests passed on CircleCI!
Details

Homeserver Task Board automation moved this from Community PRs to Done Aug 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.