Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Synapse accepts "user" instead of "username" in application service /register #9547

Closed
neilalexander opened this issue Mar 4, 2021 · 8 comments · Fixed by #15928
Closed
Assignees
Labels
A-Application-Service Related to AS support A-Spec-Compliance places where synapse does not conform to the spec O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Blocks non-critical functionality, workarounds exist. T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. Z-Future-Maintenance Things that can't yet be done, but will need cleaning up in a couple of months/releases

Comments

@neilalexander
Copy link

Description

When an AS registers a user in Synapse, it seems happy to accept "user" instead of the specced "username" in the request body.

This is related to matrix-org/sytest#1017.

Steps to reproduce

Up until matrix-org/sytest@ab7b8f0, Sytest was testing this way and the tests passed.

Version information

  • Version: Seemingly all versions
@anoadragon453
Copy link
Member

This may be related to old client behaviour, in which case changing it would be hard.

@anoadragon453 anoadragon453 added S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. labels Mar 4, 2021
@richvdh
Copy link
Member

richvdh commented Mar 4, 2021

possibly, but it would have to be very old clients, and tbh we should be doing a better job at deprecating and sunsetting these things.

@clokep clokep added the A-Spec-Compliance places where synapse does not conform to the spec label Mar 8, 2021
@clokep
Copy link
Member

clokep commented Mar 8, 2021

I'm assuming that this accepts both since sytest still passes? So this is really a matter of deprecating the old user field?

@anoadragon453
Copy link
Member

anoadragon453 commented Mar 8, 2021

@clokep Yep. We need to do a similar thing to #9559 / #9548 here. Create a deprecation notice in the changelog and then remove the old support once enough time has passed.

Someone needs to do the investigation on whether anyone is still using user as well (a good way to do so is via grepping matrix.org logs). Nevermind, this is in the body of the request, which is not logged.

@anoadragon453 anoadragon453 added T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. and removed S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. labels Mar 8, 2021
@clokep
Copy link
Member

clokep commented May 26, 2023

Note that #927 seems to have added a fallback to "username". I think this is really Synapse accepts "user" in addition to "username" FWIW (at least since #927).

@clokep
Copy link
Member

clokep commented May 26, 2023

@Half-Shot / @tulir Any idea if we can deprecate and remove this (similar to the legacy AS paths)?

@clokep
Copy link
Member

clokep commented Jun 1, 2023

And for all avoidance of doubt that this is really old:

So this was way before r0.1.0 of the appservice spec.

I'm going to do a PR to deprecate this and we can remove it in the future.

@clokep clokep added S-Minor Blocks non-critical functionality, workarounds exist. O-Uncommon Most users are unlikely to come across this or unexpected workflow labels Jun 1, 2023
@clokep clokep changed the title Synapse accepts "user" instead of "username" in AS /register Synapse accepts "user" instead of "username" in application service /register Jun 1, 2023
@clokep
Copy link
Member

clokep commented Jun 1, 2023

This is deprecated and can be removed in v1.88.0. Since this was never specced I think we can just remove it.

@clokep clokep added A-Application-Service Related to AS support Z-Future-Maintenance Things that can't yet be done, but will need cleaning up in a couple of months/releases labels Jun 1, 2023
@clokep clokep added this to the Revisit: Next Month milestone Jun 1, 2023
@clokep clokep self-assigned this Jul 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Application-Service Related to AS support A-Spec-Compliance places where synapse does not conform to the spec O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Blocks non-critical functionality, workarounds exist. T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. Z-Future-Maintenance Things that can't yet be done, but will need cleaning up in a couple of months/releases
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants