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

MSC2264: Add an unstable feature flag to MSC2140 for clients to detect support #2264

Merged
merged 2 commits into from
Sep 4, 2019

Conversation

turt2live
Copy link
Member

Rendered

This proposal is an amendment to MSC2140 which was missed during review. A new proposal has been created as the change is major enough.

@turt2live turt2live added the proposal A matrix spec change proposal label Aug 29, 2019
@turt2live turt2live changed the title Add an unstable feature flag to MSC2140 for clients to detect support MSC2264: Add an unstable feature flag to MSC2140 for clients to detect support Aug 29, 2019
@turt2live
Copy link
Member Author

I'm hoping this is largely uncontroversial:

@mscbot fcp merge

@mscbot
Copy link
Collaborator

mscbot commented Aug 29, 2019

Team member @turt2live has proposed to merge this. The next step is review by the rest of the tagged people:

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@mscbot mscbot added proposed-final-comment-period Currently awaiting signoff of a majority of team members in order to enter the final comment period. disposition-merge labels Aug 29, 2019
@turt2live
Copy link
Member Author

Implementation: matrix-org/synapse#5930

@turt2live turt2live added this to Review stages in Client-server r0.6 proposals Aug 29, 2019
@ara4n
Copy link
Member

ara4n commented Aug 29, 2019

super confusing to have an msc which patches an existing msc...

@ara4n
Copy link
Member

ara4n commented Aug 29, 2019

the content of the patch however is ok

@turt2live
Copy link
Member Author

agreed, however this is what our process calls for :(

Something for the retro I guess

Copy link
Contributor

@jryans jryans left a comment

Choose a reason for hiding this comment

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

Looks great, thanks! 😁

@turt2live turt2live added phase:1 privacy-sprint Temporary label: privacy-related stuff labels Aug 29, 2019
@erikjohnston
Copy link
Member

Could you add a note at the top of the MSC to say that it was amended by MSC224 please?

@mscbot
Copy link
Collaborator

mscbot commented Aug 30, 2019

🔔 This is now entering its final comment period, as per the review above. 🔔

@mscbot mscbot added final-comment-period This MSC has entered a final comment period in interest to approval, postpone, or delete in 5 days. and removed proposed-final-comment-period Currently awaiting signoff of a majority of team members in order to enter the final comment period. labels Aug 30, 2019
In order to allow client implementations to determine if the homeserver they are developed
against supports `id_access_token`, an unstable feature flag of `m.id_access_token`
is to be added to `/versions`. When the flag is `false` or not present, clients must assume
that the homeserver does not support being given `id_access_token` and may receive an error
Copy link
Member

Choose a reason for hiding this comment

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

aiui it's not really the fact that the HS supports being given the token (it would just ignore it if not) but more that it supports passing the token on to the IS?

Copy link
Member Author

Choose a reason for hiding this comment

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

Synapse makes angry noises when you supply id_access_token when it doesn't want one. But yes, that's another way to say it.

Copy link
Member

Choose a reason for hiding this comment

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

why does it do that?

Copy link
Member Author

Choose a reason for hiding this comment

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

it probably shouldn't, but it's not exactly in the wrong here. Matrix doesn't say that a client can send unknown fields to endpoints, but it also doesn't say you can't. Synapse in this case I'd count as compliant, albeit annoying.

I can add some words about the flag being about supporting the field though, if wanted.

Copy link
Member

Choose a reason for hiding this comment

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

the field is in the body of a json post? I think it's fairly key to the extensibility of the protocol that servers don't complain about spurious fields. Anyway, this is OT here, I guess: I'm not really looking for a change if what it says is accurate currently.

### Unstable feature flag for transition

In order to allow client implementations to determine if the homeserver they are developed
against supports `id_access_token`, an unstable feature flag of `m.id_access_token`
Copy link
Member

Choose a reason for hiding this comment

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

I'm sure you've thought about this and I have no real desire to get involved with the bikeshed-painting, but it might be useful to record the rationale for choosing m.id_access_token rather than (say) m.authenticated_is_api or m.is_api_v2?

Copy link
Member Author

Choose a reason for hiding this comment

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

I've thought about it no more than copying the flag out of a synapse PR. I don't really know the rationale for picking the flag we did, but I also don't want this to be a bikeshed problem. The path of least resistance was very much abused here.

I'd definitely be in favour of m.identity_api_v2 or similar to avoid is.

Copy link
Member

@richvdh richvdh Aug 30, 2019

Choose a reason for hiding this comment

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

I'm sure you've thought about this

"you" = "someone on the privacy project

@turt2live turt2live moved this from Review stages to Implementation stages in Client-server r0.6 proposals Aug 30, 2019
@turt2live turt2live moved this from Implementation stages to Review stages in Client-server r0.6 proposals Aug 30, 2019
@mscbot mscbot added finished-final-comment-period and removed final-comment-period This MSC has entered a final comment period in interest to approval, postpone, or delete in 5 days. labels Sep 4, 2019
@mscbot
Copy link
Collaborator

mscbot commented Sep 4, 2019

The final comment period, with a disposition to merge, as per the review above, is now complete.

@turt2live turt2live merged commit a7c3c29 into master Sep 4, 2019
@turt2live turt2live added merged A proposal whose PR has merged into the spec! and removed finished-final-comment-period labels Sep 4, 2019
@turt2live
Copy link
Member Author

I'm jumping this straight to merged because there's no tangible requirement for this MSC to actually end up in the spec. We do not spec these flags because we expect that people will use the spec release as a detection for support.

MSC2140 has its own status to worry about independent of this MSC.

@turt2live turt2live deleted the travis/msc/feature-flag-2140 branch September 4, 2019 17:11
@turt2live turt2live moved this from Review stages to Done in Client-server r0.6 proposals Sep 4, 2019
anoadragon453 added a commit to matrix-org/synapse that referenced this pull request Oct 3, 2019
Synapse 1.4.0 (2019-10-03)
==========================

Bugfixes
--------

- Redact `client_secret` in server logs. ([\#6158](#6158))

Synapse 1.4.0rc2 (2019-10-02)
=============================

Bugfixes
--------

- Fix bug in background update that adds last seen information to the `devices` table, and improve its performance on Postgres. ([\#6135](#6135))
- Fix bad performance of censoring redactions background task. ([\#6141](#6141))
- Fix fetching censored redactions from DB, which caused APIs like initial sync to fail if it tried to include the censored redaction. ([\#6145](#6145))
- Fix exceptions when storing large retry intervals for down remote servers. ([\#6146](#6146))

Internal Changes
----------------

- Fix up sample config entry for `redaction_retention_period` option. ([\#6117](#6117))

Synapse 1.4.0rc1 (2019-09-26)
=============================

Note that this release includes significant changes around 3pid
verification. Administrators are reminded to review the [upgrade notes](UPGRADE.rst#upgrading-to-v140).

Features
--------

- Changes to 3pid verification:
  - Add the ability to send registration emails from the homeserver rather than delegating to an identity server. ([\#5835](#5835), [\#5940](#5940), [\#5993](#5993), [\#5994](#5994), [\#5868](#5868))
  - Replace `trust_identity_server_for_password_resets` config option with `account_threepid_delegates`, and make the `id_server` parameteter optional on `*/requestToken` endpoints, as per [MSC2263](matrix-org/matrix-spec-proposals#2263). ([\#5876](#5876), [\#5969](#5969), [\#6028](#6028))
  - Switch to using the v2 Identity Service `/lookup` API where available, with fallback to v1. (Implements [MSC2134](matrix-org/matrix-spec-proposals#2134) plus `id_access_token authentication` for v2 Identity Service APIs from [MSC2140](matrix-org/matrix-spec-proposals#2140)). ([\#5897](#5897))
  - Remove `bind_email` and `bind_msisdn` parameters from `/register` ala [MSC2140](matrix-org/matrix-spec-proposals#2140). ([\#5964](#5964))
  - Add `m.id_access_token` to `unstable_features` in `/versions` as per [MSC2264](matrix-org/matrix-spec-proposals#2264). ([\#5974](#5974))
  - Use the v2 Identity Service API for 3PID invites. ([\#5979](#5979))
  - Add `POST /_matrix/client/unstable/account/3pid/unbind` endpoint from [MSC2140](matrix-org/matrix-spec-proposals#2140) for unbinding a 3PID from an identity server without removing it from the homeserver user account. ([\#5980](#5980), [\#6062](#6062))
  - Use `account_threepid_delegate.email` and `account_threepid_delegate.msisdn` for validating threepid sessions. ([\#6011](#6011))
  - Allow homeserver to handle or delegate email validation when adding an email to a user's account. ([\#6042](#6042))
  - Implement new Client Server API endpoints `/account/3pid/add` and `/account/3pid/bind` as per [MSC2290](matrix-org/matrix-spec-proposals#2290). ([\#6043](#6043))
  - Add an unstable feature flag for separate add/bind 3pid APIs. ([\#6044](#6044))
  - Remove `bind` parameter from Client Server POST `/account` endpoint as per [MSC2290](matrix-org/matrix-spec-proposals#2290). ([\#6067](#6067))
  - Add `POST /add_threepid/msisdn/submit_token` endpoint for proxying submitToken on an `account_threepid_handler`. ([\#6078](#6078))
  - Add `submit_url` response parameter to `*/msisdn/requestToken` endpoints. ([\#6079](#6079))
  - Add `m.require_identity_server` flag to /version's unstable_features. ([\#5972](#5972))
- Enhancements to OpenTracing support:
  - Make OpenTracing work in worker mode. ([\#5771](#5771))
  - Pass OpenTracing contexts between servers when transmitting EDUs. ([\#5852](#5852))
  - OpenTracing for device list updates. ([\#5853](#5853))
  - Add a tag recording a request's authenticated entity and corresponding servlet in OpenTracing. ([\#5856](#5856))
  - Add minimum OpenTracing for client servlets. ([\#5983](#5983))
  - Check at setup that OpenTracing is installed if it's enabled in the config. ([\#5985](#5985))
  - Trace replication send times. ([\#5986](#5986))
  - Include missing OpenTracing contexts in outbout replication requests. ([\#5982](#5982))
  - Fix sending of EDUs when OpenTracing is enabled with an empty whitelist. ([\#5984](#5984))
  - Fix invalid references to None while OpenTracing if the log context slips. ([\#5988](#5988), [\#5991](#5991))
  - OpenTracing for room and e2e keys. ([\#5855](#5855))
  - Add OpenTracing span over HTTP push processing. ([\#6003](#6003))
- Add an admin API to purge old rooms from the database. ([\#5845](#5845))
- Retry well-known lookups if we have recently seen a valid well-known record for the server. ([\#5850](#5850))
- Add support for filtered room-directory search requests over federation ([MSC2197](matrix-org/matrix-spec-proposals#2197), in order to allow upcoming room directory query performance improvements. ([\#5859](#5859))
- Correctly retry all hosts returned from SRV when we fail to connect. ([\#5864](#5864))
- Add admin API endpoint for setting whether or not a user is a server administrator. ([\#5878](#5878))
- Enable cleaning up extremities with dummy events by default to prevent undue build up of forward extremities. ([\#5884](#5884))
- Add config option to sign remote key query responses with a separate key. ([\#5895](#5895))
- Add support for config templating. ([\#5900](#5900))
- Users with the type of "support" or "bot" are no longer required to consent. ([\#5902](#5902))
- Let synctl accept a directory of config files. ([\#5904](#5904))
- Increase max display name size to 256. ([\#5906](#5906))
- Add admin API endpoint for getting whether or not a user is a server administrator. ([\#5914](#5914))
- Redact events in the database that have been redacted for a week. ([\#5934](#5934))
- New prometheus metrics:
  - `synapse_federation_known_servers`: represents the total number of servers your server knows about (i.e. is in rooms with), including itself. Enable by setting `metrics_flags.known_servers` to True in the configuration.([\#5981](#5981))
  - `synapse_build_info`: exposes the Python version, OS version, and Synapse version of the running server. ([\#6005](#6005))
- Give appropriate exit codes when synctl fails. ([\#5992](#5992))
- Apply the federation blacklist to requests to identity servers. ([\#6000](#6000))
- Add `report_stats_endpoint` option to configure where stats are reported to, if enabled. Contributed by @Sorunome. ([\#6012](#6012))
- Add config option to increase ratelimits for room admins redacting messages. ([\#6015](#6015))
- Stop sending federation transactions to servers which have been down for a long time. ([\#6026](#6026))
- Make the process for mapping SAML2 users to matrix IDs more flexible. ([\#6037](#6037))
- Return a clearer error message when a timeout occurs when attempting to contact an identity server. ([\#6073](#6073))
- Prevent password reset's submit_token endpoint from accepting trailing slashes. ([\#6074](#6074))
- Return 403 on `/register/available` if registration has been disabled. ([\#6082](#6082))
- Explicitly log when a homeserver does not have the `trusted_key_servers` config field configured. ([\#6090](#6090))
- Add support for pruning old rows in `user_ips` table. ([\#6098](#6098))

Bugfixes
--------

- Don't create broken room when `power_level_content_override.users` does not contain `creator_id`. ([\#5633](#5633))
- Fix database index so that different backup versions can have the same sessions. ([\#5857](#5857))
- Fix Synapse looking for config options `password_reset_failure_template` and `password_reset_success_template`, when they are actually `password_reset_template_failure_html`, `password_reset_template_success_html`. ([\#5863](#5863))
- Fix stack overflow when recovering an appservice which had an outage. ([\#5885](#5885))
- Fix error message which referred to `public_base_url` instead of `public_baseurl`. Thanks to @aaronraimist for the fix! ([\#5909](#5909))
- Fix 404 for thumbnail download when `dynamic_thumbnails` is `false` and the thumbnail was dynamically generated. Fix reported by rkfg. ([\#5915](#5915))
- Fix a cache-invalidation bug for worker-based deployments. ([\#5920](#5920))
- Fix admin API for listing media in a room not being available with an external media repo. ([\#5966](#5966))
- Fix list media admin API always returning an error. ([\#5967](#5967))
- Fix room and user stats tracking. ([\#5971](#5971), [\#5998](#5998), [\#6029](#6029))
- Return a `M_MISSING_PARAM` if `sid` is not provided to `/account/3pid`. ([\#5995](#5995))
- `federation_certificate_verification_whitelist` now will not cause `TypeErrors` to be raised (a regression in 1.3). Additionally, it now supports internationalised domain names in their non-canonical representation. ([\#5996](#5996))
- Only count real users when checking for auto-creation of auto-join room. ([\#6004](#6004))
- Ensure support users can be registered even if MAU limit is reached. ([\#6020](#6020))
- Fix bug where login error was shown incorrectly on SSO fallback login. ([\#6024](#6024))
- Fix bug in calculating the federation retry backoff period. ([\#6025](#6025))
- Prevent exceptions being logged when extremity-cleanup events fail due to lack of user consent to the terms of service. ([\#6053](#6053))
- Remove POST method from password-reset `submit_token` endpoint until we implement `submit_url` functionality. ([\#6056](#6056))
- Fix logcontext spam on non-Linux platforms. ([\#6059](#6059))
- Ensure query parameters in email validation links are URL-encoded. ([\#6063](#6063))
- Fix a bug which caused SAML attribute maps to be overridden by defaults. ([\#6069](#6069))
- Fix the logged number of updated items for the `users_set_deactivated_flag` background update. ([\#6092](#6092))
- Add `sid` to `next_link` for email validation. ([\#6097](#6097))
- Threepid validity checks on msisdns should not be dependent on `threepid_behaviour_email`. ([\#6104](#6104))
- Ensure that servers which are not configured to support email address verification do not offer it in the registration flows. ([\#6107](#6107))

Updates to the Docker image
---------------------------

- Avoid changing `UID/GID` if they are already correct. ([\#5970](#5970))
- Provide `SYNAPSE_WORKER` envvar to specify python module. ([\#6058](#6058))

Improved Documentation
----------------------

- Convert documentation to markdown (from rst) ([\#5849](#5849))
- Update `INSTALL.md` to say that Python 2 is no longer supported. ([\#5953](#5953))
- Add developer documentation for using SAML2. ([\#6032](#6032))
- Add some notes on rolling back to v1.3.1. ([\#6049](#6049))
- Update the upgrade notes. ([\#6050](#6050))

Deprecations and Removals
-------------------------

- Remove shared-secret registration from `/_matrix/client/r0/register` endpoint. Contributed by Awesome Technologies Innovationslabor GmbH. ([\#5877](#5877))
- Deprecate the `trusted_third_party_id_servers` option. ([\#5875](#5875))

Internal Changes
----------------

- Lay the groundwork for structured logging output. ([\#5680](#5680))
- Retry well-known lookup before the cache expires, giving a grace period where the remote well-known can be down but we still use the old result. ([\#5844](#5844))
- Remove log line for debugging issue #5407. ([\#5860](#5860))
- Refactor the Appservice scheduler code. ([\#5886](#5886))
- Compatibility with v2 Identity Service APIs other than /lookup. ([\#5892](#5892), [\#6013](#6013))
- Stop populating some unused tables. ([\#5893](#5893), [\#6047](#6047))
- Add missing index on `users_in_public_rooms` to improve the performance of directory queries. ([\#5894](#5894))
- Improve the logging when we have an error when fetching signing keys. ([\#5896](#5896))
- Add support for database engine-specific schema deltas, based on file extension. ([\#5911](#5911))
- Update Buildkite pipeline to use plugins instead of buildkite-agent commands. ([\#5922](#5922))
- Add link in sample config to the logging config schema. ([\#5926](#5926))
- Remove unnecessary parentheses in return statements. ([\#5931](#5931))
- Remove unused `jenkins/prepare_sytest.sh` file. ([\#5938](#5938))
- Move Buildkite pipeline config to the pipelines repo. ([\#5943](#5943))
- Remove unnecessary return statements in the codebase which were the result of a regex run. ([\#5962](#5962))
- Remove left-over methods from v1 registration API. ([\#5963](#5963))
- Cleanup event auth type initialisation. ([\#5975](#5975))
- Clean up dependency checking at setup. ([\#5989](#5989))
- Update OpenTracing docs to use the unified `trace` method. ([\#5776](#5776))
- Small refactor of function arguments and docstrings in` RoomMemberHandler`. ([\#6009](#6009))
- Remove unused `origin` argument on `FederationHandler.add_display_name_to_third_party_invite`. ([\#6010](#6010))
- Add a `failure_ts` column to the `destinations` database table. ([\#6016](#6016), [\#6072](#6072))
- Clean up some code in the retry logic. ([\#6017](#6017))
- Fix the structured logging tests stomping on the global log configuration for subsequent tests. ([\#6023](#6023))
- Clean up the sample config for SAML authentication. ([\#6064](#6064))
- Change mailer logging to reflect Synapse doesn't just do chat notifications by email now. ([\#6075](#6075))
- Move last-seen info into devices table. ([\#6089](#6089))
- Remove unused parameter to `get_user_id_by_threepid`. ([\#6099](#6099))
- Refactor the user-interactive auth handling. ([\#6105](#6105))
- Refactor code for calculating registration flows. ([\#6106](#6106))
@turt2live turt2live added the kind:maintenance MSC which clarifies/updates existing spec label Apr 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge kind:maintenance MSC which clarifies/updates existing spec merged A proposal whose PR has merged into the spec! privacy-sprint Temporary label: privacy-related stuff proposal A matrix spec change proposal
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

6 participants