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

add org.matrix.login.jwt so that m.login.jwt can be deprecated #7675

Merged
merged 8 commits into from
Jun 24, 2020
Merged

add org.matrix.login.jwt so that m.login.jwt can be deprecated #7675

merged 8 commits into from
Jun 24, 2020

Conversation

Sorunome
Copy link
Contributor

@Sorunome Sorunome commented Jun 11, 2020 •
edited
Loading

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.
  • Pull request includes a sign off
  • Code style is correct (run the linters)

m.login.jwt is not part of the spec, and the m. namespace is reserved for things in the spec. When soru asked about this a few months ago, the reply was to not use this endpoint and that it might be removed at any point in time, due to not being in the spec. This PR namespaces the endpoint properly.

Signed-off-by: Sorunome mail@sorunome.de

@clokep clokep requested a review from a team June 11, 2020 13:59
@richvdh
Copy link
Member

richvdh commented Jun 11, 2020

related: https://github.com/matrix-org/matrix-doc/issues/1658

@Sorunome
Copy link
Contributor Author

Also related: matrix-org/matrix-spec-proposals#2416

richvdh
richvdh suggested changes Jun 18, 2020
View reviewed changes
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

Hi Soru, thanks for raising this.

We've discussed as a team what to do here. We agree that it's not good that Synapse is implementing login methods in the m. namespace without them being specced, but we don't think we can land this as it currently stands because we're pretty sure that at least a couple of users are (somehow) using it and it wouldn't be fair on them to break it without warning.

Instead, we think it would make sense to deprecate Synapse's implementation of m.login.jwt, with the intention of disabling it in the future and replacing it with org.matrix.login.jwt. In the meantime, we should allow both login methods to give users a chance to transition. (In the past, when we've deprecated things, we've allowed a couple of releases after the announcement before removing them.)

So, do you think you could modify this PR to support both login methods? We would be happy to land such a PR and would put a deprecation notice out in the next release.

@richvdh richvdh added the X-Awaiting-Changes A contributed PR which needs changes and re-review before it can be merged label Jun 18, 2020
@Sorunome
Copy link
Contributor Author

Done

richvdh
richvdh suggested changes Jun 18, 2020
View reviewed changes
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

thanks! could you update the changelog please?

@Sorunome
Copy link
Contributor Author

Done!

richvdh
richvdh approved these changes Jun 18, 2020
View reviewed changes
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

thanks!

@richvdh richvdh removed the X-Awaiting-Changes A contributed PR which needs changes and re-review before it can be merged label Jun 18, 2020
@richvdh richvdh changed the title rename m.login.jwt to org.matrix.login.jwt add org.matrix.login.jwt so that m.login.jwt can be deprecated Jun 24, 2020
@richvdh richvdh merged commit 6920e58 into matrix-org:develop Jun 24, 2020
babolivier pushed a commit that referenced this pull request Sep 1, 2021
@anoadragon453
Merge commit 'dc80a0762' into dinsic
87f941c 
* commit 'dc80a0762':
  1.16.0rc1
  Back out MSC2625 implementation (#7761)
  Additional configuration options for auto-join rooms (#7763)
  Add some metrics for inbound and outbound federation processing times (#7755)
  Explain the purpose of the "tests" conditional dependency requirement (#7751)
  Add another yield point to state res v2 (#7746)
  Move flake8 to end. Don't exit script on failure (#7738)
  Make tox actions work on Debian 10 (#7703)
  Yield during large v2 state res. (#7735)
  add org.matrix.login.jwt so that m.login.jwt can be deprecated (#7675)
  Set Content-Length for Metrics requests (#7730)
  Sync ignored table names in synapse_port_db to current database schema (#7717)
  Allow local media to be marked as safe from being quarantined. (#7718)
  Convert directory handler to async/await (#7727)
  Speed up state res v2 across large state differences. (#7725)
@clokep clokep mentioned this pull request Mar 23, 2022
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@richvdh richvdh richvdh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Sorunome @richvdh