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.

Sign up for GitHub

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

Jump to bottom

MSC4127: Removal of query string auth #4127

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

MSC4127: Removal of query string auth #4127

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions proposals/4127-remove-query-string-auth.md
View file
Open in desktop
Copy link
Member Author

Choose a reason for hiding this comment

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

Implementation requirements:

  • None, in my opinion. The MSC is not feasible to implement beyond what has already been described in the proposal itself.

Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# MSC4127: Removal of query string auth

See [MSC4126](https://github.com/matrix-org/matrix-spec-proposals/pull/4126) for details on query
string authentication and the prerequisite deprecation.

## Proposal

Query string authentication becomes *removed* from the [Client-Server API](https://spec.matrix.org/v1.10/client-server-api/#using-access-tokens)
and [Identity Service API](https://spec.matrix.org/v1.10/identity-service-api/#authentication).
Comment on lines +8 to +9
Copy link
Member

Choose a reason for hiding this comment

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

An issue that Synapse ran into with MSC2832 which removed query parameter auth for app services is that we did not want to drop support for old spec versions, but did want to follow this guidance. (matrix-org/synapse#15379 has some of the discussion.)

We ended up saying Synapse was still compatible with 1.4, but had a configuration flag for re-enabling the old behavior (disabling it by default in the name of secure defaults).

I'm not sure if I have a recommendation, but wanting to be both backwards compatible and secure by default was difficult in this situation. I guess one option would have been to only declare support for the older versions if the config flag was flipped, but I think various clients would have stopped working if we did that.


The prior deprecation required by the [deprecation policy](https://spec.matrix.org/v1.10/#deprecation-policy)
is described by [MSC4126](https://github.com/matrix-org/matrix-spec-proposals/pull/4126).

As a process note, it was considered to bundle the deprecation and removal into MSC4126 instead of
having two separate MSCs, breaking the deprecation policy slightly, though this introduces consequences
which are, in the author's opinion, undesirable. Specifically, the spec release changelog for the
deprecation would say "as per MSC4126" and imply the MSC being moved to the [merged](https://spec.matrix.org/proposals/#process)
state, however the second half of the MSC (the feature removal) would not have landed yet. This would
leave the MSC stuck in an awkward `spec-pr-missing` state because half of it had not been written up
or landed yet. To maintain a clearer changelog, ease process considerations, and give clients another
opportunity to object to the removal, the two MSC approach required by the deprecation policy is best.

## Potential issues

See [MSC4126](https://github.com/matrix-org/matrix-spec-proposals/pull/4126).

## Alternatives

See [MSC4126](https://github.com/matrix-org/matrix-spec-proposals/pull/4126).

More of a process note, during drafting of this proposal the author considered combining the deprecation
and removal into a single MSC, against the advice of the deprecation policy. The aim was to reduce
process burden and more quickly get the security risk removed from the specification, though merging
both operations into a single MSC potentially makes the spec release changelog unclear and leaves the
MSC in an awkward released-but-not-merged state. To avoid figuring out how to navigate the process
states, increasing the process burden rather than decreasing it, the deprecation policy's two MSC
requirement has been followed here. However, there are still other process efficiencies which can
take place - see the "Dependencies" section of this MSC for details.

## Security considerations

See [MSC4126](https://github.com/matrix-org/matrix-spec-proposals/pull/4126).

## Unstable prefix

This proposal cannot feasibly have an unstable prefix. Clients are already discouraged from using
query string authentication and should switch to `Authorization` as soon as possible, regardless of
this MSC.

See [MSC4126](https://github.com/matrix-org/matrix-spec-proposals/pull/4126) for related details.

## Dependencies

[MSC4126](https://github.com/matrix-org/matrix-spec-proposals/pull/4126) must be *released* in the
specification before this proposal can be merged, though this MSC *may* progress through Final Comment
Period concurrent to MSC4126 if desirable.