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

Use JSON when querying the IS's /store-invite endpoint #5638

Merged
merged 4 commits into from
Jul 9, 2019

Conversation

babolivier
Copy link
Contributor

Tries to send requests to the IS's /store-invite endpoint using the application/json type as the spec mandates it. This might not work because of matrix-org/sydent#170, so if that failed fallback to the previous behaviour, which is sending it as application/x-www-form-urlencoded.

Fixes #5634

@babolivier babolivier requested a review from a team July 8, 2019 13:27
@babolivier
Copy link
Contributor Author

FTR I've tested these changes on a personal box, and they work both with Sydent's current master and with the patch at matrix-org/sydent#170.

@codecov
Copy link

codecov bot commented Jul 8, 2019

Codecov Report

Merging #5638 into master will decrease coverage by <.01%.
The diff coverage is 16.66%.

@@            Coverage Diff             @@
##           master    #5638      +/-   ##
==========================================
- Coverage   63.16%   63.15%   -0.01%     
==========================================
  Files         328      328              
  Lines       35944    35929      -15     
  Branches     5919     5908      -11     
==========================================
- Hits        22703    22692      -11     
+ Misses      11612    11611       -1     
+ Partials     1629     1626       -3

1 similar comment
@codecov
Copy link

codecov bot commented Jul 8, 2019

Codecov Report

Merging #5638 into master will decrease coverage by <.01%.
The diff coverage is 16.66%.

@@            Coverage Diff             @@
##           master    #5638      +/-   ##
==========================================
- Coverage   63.16%   63.15%   -0.01%     
==========================================
  Files         328      328              
  Lines       35944    35929      -15     
  Branches     5919     5908      -11     
==========================================
- Hits        22703    22692      -11     
+ Misses      11612    11611       -1     
+ Partials     1629     1626       -3

@babolivier babolivier changed the base branch from master to develop July 8, 2019 14:48
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.

lgtm otherwise

data = yield self.simple_http_client.post_json_get_json(
is_url, invite_config
)
except HttpResponseException as e:
Copy link
Member

Choose a reason for hiding this comment

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

can we restrict this to only happen on a 400 response?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we restrict it then we need to restrict it to 500s, because matrix-org/sydent#170 fixes it 500ing on a JSON request.

Copy link
Member

Choose a reason for hiding this comment

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

good point, ignore me

@babolivier babolivier merged commit 65434da into develop Jul 9, 2019
anoadragon453 added a commit that referenced this pull request Jul 22, 2019
v1.2.0rc1

Features
--------

- Add support for opentracing. ([\#5544](#5544), [\#5712](#5712))
- Add ability to pull all locally stored events out of synapse that a particular user can see. ([\#5589](#5589))
- Add a basic admin command app to allow server operators to run Synapse admin commands separately from the main production instance. ([\#5597](#5597))
- Add `sender` and `origin_server_ts` fields to `m.replace`. ([\#5613](#5613))
- Add default push rule to ignore reactions. ([\#5623](#5623))
- Include the original event when asking for its relations. ([\#5626](#5626))
- Implement `session_lifetime` configuration option, after which access tokens will expire. ([\#5660](#5660))
- Return "This account has been deactivated" when a deactivated user tries to login. ([\#5674](#5674))
- Enable aggregations support by default ([\#5714](#5714))

Bugfixes
--------

- Fix 'utime went backwards' errors on daemonization. ([\#5609](#5609))
- Various minor fixes to the federation request rate limiter. ([\#5621](#5621))
- Forbid viewing relations on an event once it has been redacted. ([\#5629](#5629))
- Fix requests to the `/store_invite` endpoint of identity servers being sent in the wrong format. ([\#5638](#5638))
- Fix newly-registered users not being able to lookup their own profile without joining a room. ([\#5644](#5644))
- Fix bug in #5626 that prevented the original_event field from actually having the contents of the original event in a call to `/relations`. ([\#5654](#5654))
- Fix 3PID bind requests being sent to identity servers as `application/x-form-www-urlencoded` data, which is deprecated. ([\#5658](#5658))
- Fix some problems with authenticating redactions in recent room versions. ([\#5699](#5699), [\#5700](#5700), [\#5707](#5707))
- Ignore redactions of m.room.create events. ([\#5701](#5701))

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

- Base Docker image on a newer Alpine Linux version (3.8 -> 3.10). ([\#5619](#5619))
- Add missing space in default logging file format generated by the Docker image. ([\#5620](#5620))

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

- Add information about nginx normalisation to reverse_proxy.rst. Contributed by @skalarproduktraum - thanks! ([\#5397](#5397))
- --no-pep517 should be --no-use-pep517 in the documentation to setup the development environment. ([\#5651](#5651))
- Improvements to Postgres setup instructions. Contributed by @Lrizika - thanks! ([\#5661](#5661))
- Minor tweaks to postgres documentation. ([\#5675](#5675))

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

- Remove support for the `invite_3pid_guest` configuration setting. ([\#5625](#5625))

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

- Move logging code out of `synapse.util` and into `synapse.logging`. ([\#5606](#5606), [\#5617](#5617))
- Add a blacklist file to the repo to blacklist certain sytests from failing CI. ([\#5611](#5611))
- Make runtime errors surrounding password reset emails much clearer. ([\#5616](#5616))
- Remove dead code for persiting outgoing federation transactions. ([\#5622](#5622))
- Add `lint.sh` to the scripts-dev folder which will run all linting steps required by CI. ([\#5627](#5627))
- Move RegistrationHandler.get_or_create_user to test code. ([\#5628](#5628))
- Add some more common python virtual-environment paths to the black exclusion list. ([\#5630](#5630))
- Some counter metrics exposed over Prometheus have been renamed, with the old names preserved for backwards compatibility and deprecated. See `docs/metrics-howto.rst` for details. ([\#5636](#5636))
- Unblacklist some user_directory sytests. ([\#5637](#5637))
- Factor out some redundant code in the login implementation. ([\#5639](#5639))
- Update ModuleApi to avoid register(generate_token=True). ([\#5640](#5640))
- Remove access-token support from `RegistrationHandler.register`, and rename it. ([\#5641](#5641))
- Remove access-token support from `RegistrationStore.register`, and rename it. ([\#5642](#5642))
- Improve logging for auto-join when a new user is created. ([\#5643](#5643))
- Remove unused and unnecessary check for FederationDeniedError in _exception_to_failure. ([\#5645](#5645))
- Fix a small typo in a code comment. ([\#5655](#5655))
- Clean up exception handling around client access tokens. ([\#5656](#5656))
- Add a mechanism for per-test homeserver configuration in the unit tests. ([\#5657](#5657))
- Inline issue_access_token. ([\#5659](#5659))
- Update the sytest BuildKite configuration to checkout Synapse in `/src`. ([\#5664](#5664))
- Add a `docker` type to the towncrier configuration. ([\#5673](#5673))
- Convert `synapse.federation.transport.server` to `async`. Might improve some stack traces. ([\#5689](#5689))
- Documentation for opentracing. ([\#5703](#5703))
@babolivier babolivier deleted the babolivier/invite-json branch September 27, 2019 09:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants