From 529da34513fe0016aca22709119b3532b04c8d81 Mon Sep 17 00:00:00 2001 From: orientor Date: Wed, 1 Jul 2020 22:37:31 +0530 Subject: [PATCH] openapi: Use third-party validator for schema validation. Our previous OpenAPI schema validator that we implemented ourselves was useful training wheels for our understanding OpenAPI properly, and was mostly correct. But given that we've finally reached the point where our OpenAPI file accurately describes the API, it makes sense to switch to use an official OpenAPI validator. We lose some ability to do exclude rules for particular elements, but those were primarily important for us when we had a lot of them. As part of this change, we need to add `additionalProperties: false` for all of our dictonaries/objects where we've documented every parameter; otherwise the OpenAPI schema checker won't know that we expect every parameter to be documented. --- requirements/common.in | 3 + requirements/dev.txt | 57 +++++- requirements/prod.txt | 70 +++++++- version.py | 2 +- .../api_return_values_table_generator.py | 2 +- zerver/openapi/openapi.py | 170 ++++++------------ zerver/openapi/testing.yaml | 4 + zerver/openapi/zulip.yaml | 35 ++++ zerver/tests/test_openapi.py | 55 +++--- 9 files changed, 234 insertions(+), 164 deletions(-) diff --git a/requirements/common.in b/requirements/common.in index 910bb0c826..20da2b2e46 100644 --- a/requirements/common.in +++ b/requirements/common.in @@ -189,3 +189,6 @@ zxcvbn # Needed for sending HTTP requests requests[security] requests-oauthlib + +# For OpenAPI schema validation. +openapi-core diff --git a/requirements/dev.txt b/requirements/dev.txt index a6117a188b..797c4f7a8b 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -42,7 +42,7 @@ arrow==0.15.5 \ attrs==19.3.0 \ --hash=sha256:08a96c641c3a74e44eb59afb61a24f2cb9f4d7188748e76ba4bb5edfa3cb7d1c \ --hash=sha256:f7b7ce16570fe9965acd6d30101a28f62fb4a7f9e926b3bbc9b61f8b04247e72 \ - # via automat, jsonschema, service-identity, twisted + # via automat, jsonschema, openapi-core, service-identity, twisted automat==20.2.0 \ --hash=sha256:7979803c74610e11ef0c0d68a2942b152df52da55336e0c9d58daf1831cbdf33 \ --hash=sha256:b6feb6455337df834f6c9962d6ccf771515b7d939bca142b29c20c2376bc6111 \ @@ -416,7 +416,7 @@ ipython==7.15.0 \ isodate==0.6.0 \ --hash=sha256:2e364a3d5759479cdb2d37cce6b9376ea504db2ff90252a2e5b7cc89cc9ff2d8 \ --hash=sha256:aa4d33c06640f5352aca96e4b81afd8ab3b47337cc12089822d6f322ac772c81 \ - # via python3-saml + # via openapi-schema-validator, python3-saml https://github.com/timothycrosley/isort/archive/a26a58423b183217d0defdb6b67e5fd285be7186.zip#egg=isort==5.0.0+git \ --hash=sha256:3a634c45784c5ac73dc871efb0faf86320ab1f237f86f28ff55d8025e185016d \ # via -r requirements/dev.in @@ -454,7 +454,7 @@ jsonpointer==2.0 \ jsonschema==3.2.0 \ --hash=sha256:4e5b3cf8216f577bee9ce139cbe72eca3ea4f292ec60928ff24758ce626cd163 \ --hash=sha256:c8a85b28d377cc7737e46e2d9f2b4f44ee3c0e1deac6bf46ddefc7187d30797a \ - # via aws-sam-translator, cfn-lint + # via aws-sam-translator, cfn-lint, openapi-schema-validator, openapi-spec-validator jsx-lexer==0.0.8 \ --hash=sha256:1cb35102b78525aa3f587dc327f3208c0e1c76d5cdea64d4f9c3ced05d10c017 \ --hash=sha256:b879c7fafe974440a1dd9f9544dfb8629fa22078ada7f769c8fbb06149eac5d1 \ @@ -462,6 +462,26 @@ jsx-lexer==0.0.8 \ junit-xml==1.9 \ --hash=sha256:ec5ca1a55aefdd76d28fcc0b135251d156c7106fa979686a4b48d62b761b4732 \ # via cfn-lint +lazy-object-proxy==1.5.0 \ + --hash=sha256:0aef3fa29f7d1194d6f8a99382b1b844e5a14d3bc1ef82c3b1c4fb7e7e2019bc \ + --hash=sha256:159ae2bbb4dc3ba506aeba868d14e56a754c0be402d1f0d7fdb264e0bdf2b095 \ + --hash=sha256:161a68a427022bf13e249458be2cb8da56b055988c584d372a917c665825ae9a \ + --hash=sha256:2d58f0e6395bf41087a383a48b06b42165f3b699f1aa41ba201db84ab77be63d \ + --hash=sha256:311c9d1840042fc8e2dd80fc80272a7ea73e7646745556153c9cda85a4628b18 \ + --hash=sha256:35c3ad7b7f7d5d4a54a80f0ff5a41ab186237d6486843f8dde00c42cfab33905 \ + --hash=sha256:459ef557e669d0046fe2b92eb4822c097c00b5ef9d11df0f9bd7d4267acdfc52 \ + --hash=sha256:4a50513b6be001b9b7be2c435478fe9669249c77c241813907a44cda1fcd03f4 \ + --hash=sha256:51035b175740c44707694c521560b55b66da9d5a7c545cf22582bc02deb61664 \ + --hash=sha256:96f2cdb35bdfda10e075f12892a42cff5179bbda698992b845f36c5e92755d33 \ + --hash=sha256:a0aed261060cd0372abf08d16399b1224dbb5b400312e6b00f2b23eabe1d4e96 \ + --hash=sha256:a6052c4c7d95de2345d9c58fc0fe34fff6c27a8ed8550dafeb18ada84406cc99 \ + --hash=sha256:cbf1354292a4f7abb6a0188f74f5e902e4510ebad105be1dbc4809d1ed92f77e \ + --hash=sha256:da82b2372f5ded8806eaac95b19af89a7174efdb418d4e7beb0c6ab09cee7d95 \ + --hash=sha256:dd89f466c930d7cfe84c94b5cbe862867c88b269f23e5aa61d40945e0d746f54 \ + --hash=sha256:e3183fbeb452ec11670c2d9bfd08a57bc87e46856b24d1c335f995239bedd0e1 \ + --hash=sha256:e9a571e7168076a0d5ecaabd91e9032e86d815cca3a4bf0dafead539ef071aa5 \ + --hash=sha256:ec6aba217d0c4f71cbe48aea962a382dedcd111f47b55e8b58d4aaca519bd360 \ + # via openapi-core libthumbor==2.0.1 \ --hash=sha256:3c4e1a59c019d22f868d225315c06f97fad30fb5e78112d6a230b978e7d24e38 \ --hash=sha256:ed4fe5f27f8f90e7285b7e6dce99c1b67d43a140bf370e989080b43d80ce25f0 \ @@ -548,6 +568,10 @@ mock==4.0.2 \ --hash=sha256:3f9b2c0196c60d21838f307f5825a7b86b678cedc58ab9e50a8988187b4d81e0 \ --hash=sha256:dd33eb70232b6118298d516bbcecd26704689c386594f0f3c4f13867b2c56f72 \ # via moto +more-itertools==8.4.0 \ + --hash=sha256:68c70cc7167bdf5c7c9d8f6954a7837089c6a36bf565383919bb595efb8a17e5 \ + --hash=sha256:b78134b2063dd214000685165d81c154522c3ee0a1c0d4d113c80361c234c5a2 \ + # via openapi-core moto==1.3.14 \ --hash=sha256:2b3fa22778504b45715868cad95ad458fdea7227f9005b12e522fc9c2ae0cabc \ --hash=sha256:79aeaeed1592a24d3c488840065a3fcb3f4fa7ba40259e112482454c0e48a03a \ @@ -580,10 +604,28 @@ oauthlib==3.1.0 \ --hash=sha256:bee41cc35fcca6e988463cacc3bcb8a96224f470ca547e697b604cc697b2f889 \ --hash=sha256:df884cd6cbe20e32633f1db1072e9356f53638e4361bef4e8b03c9127c9328ea \ # via requests-oauthlib, social-auth-core +openapi-core==0.13.3 \ + --hash=sha256:57973b7383214a529012cf65ddac8c22b25a4df497366e588e88c627581c2928 \ + --hash=sha256:8c5e2053c51f8d43d241b54d322f6c1c32adf1a20617490d361d9c0d5a123448 \ + --hash=sha256:df53cc8b9d217616bbde4e6ef943c9213af810a01cd74f62e7885756c6080af4 \ + # via -r requirements/common.in +openapi-schema-validator==0.1.1 \ + --hash=sha256:3f0b0f9086e7d717a0413a462d3d9e6f82f7e80a744abf21943ee2e0d9e8c50d \ + --hash=sha256:8fc97a575393d179d70e7c7ebd30ed9fc46eb6c5013f2790736c2e50ea150f06 \ + --hash=sha256:b7afe93aff4a876781279b376c1ba5decb338483e9484af26b140ef215119e23 \ + # via openapi-core +openapi-spec-validator==0.2.8 \ + --hash=sha256:0caacd9829e9e3051e830165367bf58d436d9487b29a09220fa7edb9f47ff81b \ + --hash=sha256:d4da8aef72bf5be40cf0df444abd20009a41baf9048a8e03750c07a934f1bdd8 \ + --hash=sha256:e489c7a273284bc78277ac22791482e8058d323b4a265015e9fcddf6a8045bcd \ + # via openapi-core packaging==20.4 \ --hash=sha256:4357f74f47b9c12db93624a82154e9b120fa8293699949152b22065d556079f8 \ --hash=sha256:998416ba6962ae7fbd6596850b80e17859a5753ba17c32284f67bfff33784181 \ # via sphinx +parse==1.15.0 \ + --hash=sha256:a6d4e2c2f1fbde6717d28084a191a052950f758c0cbd83805357e6575c2b95c0 \ + # via openapi-core parsel==1.6.0 \ --hash=sha256:70efef0b651a996cceebc69e55a85eb2233be0890959203ba7c3a03c72725c79 \ --hash=sha256:9e1fa8db1c0b4a878bf34b35c043d89c9d1cbebc23b4d34dbc3c0ec33f2e087d \ @@ -790,7 +832,7 @@ pyyaml==5.3.1 \ --hash=sha256:b8eac752c5e14d3eca0e6dd9199cd627518cb5ec06add0de9d32baeee6fe645d \ --hash=sha256:cc8955cfbfc7a115fa81d85284ee61147059a753344bc51098f3ccd69b0d7e0c \ --hash=sha256:d13155f591e6fcc1ec3b30685d50bf0711574e2c0dfffd7644babf8b5102ca1a \ - # via cfn-lint, moto, yamole + # via cfn-lint, moto, openapi-spec-validator, yamole qrcode==6.1 \ --hash=sha256:3996ee560fc39532910603704c82980ff6d4d5d629f9c3f25f34174ce8606cf5 \ --hash=sha256:505253854f607f2abf4d16092c61d4e9d511a3b4392e60bff957a68592b04369 \ @@ -865,7 +907,7 @@ sh==1.12.14 \ six==1.15.0 \ --hash=sha256:30639c035cdb23534cd4aa2dd52c3bf48f06e5f4a941509c8bafd8ce11080259 \ --hash=sha256:8b74bedcbbbaca38ff6d7491d76f2b06b3592611af620f8426e82dddb04a5ced \ - # via -r requirements/common.in, argon2-cffi, automat, aws-sam-translator, cfn-lint, cryptography, django-bitfield, docker, ecdsa, hypchat, isodate, jsonschema, junit-xml, libthumbor, moto, packaging, parsel, pip-tools, protego, pyopenssl, python-dateutil, python-debian, python-jose, qrcode, responses, social-auth-app-django, social-auth-core, talon, traitlets, twilio, w3lib, websocket-client, zulip + # via -r requirements/common.in, argon2-cffi, automat, aws-sam-translator, cfn-lint, cryptography, django-bitfield, docker, ecdsa, hypchat, isodate, jsonschema, junit-xml, libthumbor, moto, openapi-core, openapi-schema-validator, openapi-spec-validator, packaging, parsel, pip-tools, protego, pyopenssl, python-dateutil, python-debian, python-jose, qrcode, responses, social-auth-app-django, social-auth-core, talon, traitlets, twilio, w3lib, websocket-client, zulip snakeviz==2.1.0 \ --hash=sha256:8ce375b18ae4a749516d7e6c6fbbf8be6177c53974f53534d8eadb646cd279b1 \ --hash=sha256:92ad876fb6a201a7e23a6b85ea96d9643a51e285667c253a8653643804f7cb68 \ @@ -966,6 +1008,9 @@ statsd==3.3.0 \ --hash=sha256:c610fb80347fca0ef62666d241bce64184bd7cc1efe582f9690e045c25535eaa \ --hash=sha256:e3e6db4c246f7c59003e51c9720a51a7f39a396541cb9b147ff4b14d15b5dd1f \ # via django-statsd-mozilla +strict-rfc3339==0.7 \ + --hash=sha256:5cad17bedfc3af57b399db0fed32771f18fc54bbd917e85546088607ac5e1277 \ + # via openapi-schema-validator stripe==2.48.0 \ --hash=sha256:515fe2cc915e639468f30150a39c162fc0fb090256ae9d6a04e5022925d136f1 \ --hash=sha256:bdbbea632b8faa983c670db61debbe0bdb5802ef98fd0613a03aa466e56cdade \ @@ -1085,7 +1130,7 @@ websocket-client==0.57.0 \ werkzeug==1.0.1 \ --hash=sha256:2de2a5db0baeae7b2d2664949077c2ac63fbd16d98da0ff71837f7d1dea3fd43 \ --hash=sha256:6c80b1e5ad3665290ea39320b91e1be1e0d5f60652b964a3070216de83d2e47c \ - # via moto + # via moto, openapi-core wheel==0.34.2 \ --hash=sha256:8788e9155fe14f54164c1b9eb0a319d98ef02c160725587ad60f14ddc57b6f96 \ --hash=sha256:df277cb51e61359aba502208d680f90c0493adec6f0e848af94948778aed386e \ diff --git a/requirements/prod.txt b/requirements/prod.txt index d752185a5b..b86d410377 100644 --- a/requirements/prod.txt +++ b/requirements/prod.txt @@ -31,6 +31,10 @@ argon2-cffi==20.1.0 \ --hash=sha256:d8029b2d3e4b4cea770e9e5a0104dd8fa185c1724a0f01528ae4826a6d25f97d \ --hash=sha256:da7f0445b71db6d3a72462e04f36544b0de871289b0bc8a7cc87c0f5ec7079fa \ # via -r requirements/common.in +attrs==19.3.0 \ + --hash=sha256:08a96c641c3a74e44eb59afb61a24f2cb9f4d7188748e76ba4bb5edfa3cb7d1c \ + --hash=sha256:f7b7ce16570fe9965acd6d30101a28f62fb4a7f9e926b3bbc9b61f8b04247e72 \ + # via jsonschema, openapi-core babel==2.8.0 \ --hash=sha256:1aac2ae2d0d8ea368fa90906567f5c08463d98ade155c0c4bfedd6a0f7160e38 \ --hash=sha256:d670ea0b10f8b723672d3a6abeb87b565b244da220d76b4dba1b66269ec152d4 \ @@ -287,7 +291,7 @@ ijson==3.0.4 \ importlib-metadata==1.6.1 ; python_version < "3.8" \ --hash=sha256:0505dd08068cfec00f53a74a0ad927676d7757da81b7436a6eefe4c7cf75c545 \ --hash=sha256:15ec6c0fd909e893e3a08b3a7c76ecb149122fb14b7efe1199ddd4c7c57ea958 \ - # via -r requirements/common.in, markdown + # via -r requirements/common.in, jsonschema, markdown ipython-genutils==0.2.0 \ --hash=sha256:72dd37233799e619666c9f639a9da83c34013a73e8bbc79a7a6348d93c61fab8 \ --hash=sha256:eb2e116e75ecef9d4d228fdc66af54269afa26ab4463042e33785b887c628ba8 \ @@ -299,7 +303,7 @@ ipython==7.15.0 \ isodate==0.6.0 \ --hash=sha256:2e364a3d5759479cdb2d37cce6b9376ea504db2ff90252a2e5b7cc89cc9ff2d8 \ --hash=sha256:aa4d33c06640f5352aca96e4b81afd8ab3b47337cc12089822d6f322ac772c81 \ - # via python3-saml + # via openapi-schema-validator, python3-saml jedi==0.17.1 \ --hash=sha256:1ddb0ec78059e8e27ec9eb5098360b4ea0a3dd840bedf21415ea820c21b40a22 \ --hash=sha256:807d5d4f96711a2bcfdd5dfa3b1ae6d09aa53832b182090b222b5efb81f52f63 \ @@ -312,10 +316,34 @@ jmespath==0.10.0 \ --hash=sha256:b85d0567b8666149a93172712e68920734333c0ce7e89b78b3e987f71e5ed4f9 \ --hash=sha256:cdf6525904cc597730141d61b36f2e4b8ecc257c420fa2f4549bac2c2d0cb72f \ # via boto3, botocore +jsonschema==3.2.0 \ + --hash=sha256:4e5b3cf8216f577bee9ce139cbe72eca3ea4f292ec60928ff24758ce626cd163 \ + --hash=sha256:c8a85b28d377cc7737e46e2d9f2b4f44ee3c0e1deac6bf46ddefc7187d30797a \ + # via openapi-schema-validator, openapi-spec-validator jsx-lexer==0.0.8 \ --hash=sha256:1cb35102b78525aa3f587dc327f3208c0e1c76d5cdea64d4f9c3ced05d10c017 \ --hash=sha256:b879c7fafe974440a1dd9f9544dfb8629fa22078ada7f769c8fbb06149eac5d1 \ # via -r requirements/common.in +lazy-object-proxy==1.5.0 \ + --hash=sha256:0aef3fa29f7d1194d6f8a99382b1b844e5a14d3bc1ef82c3b1c4fb7e7e2019bc \ + --hash=sha256:159ae2bbb4dc3ba506aeba868d14e56a754c0be402d1f0d7fdb264e0bdf2b095 \ + --hash=sha256:161a68a427022bf13e249458be2cb8da56b055988c584d372a917c665825ae9a \ + --hash=sha256:2d58f0e6395bf41087a383a48b06b42165f3b699f1aa41ba201db84ab77be63d \ + --hash=sha256:311c9d1840042fc8e2dd80fc80272a7ea73e7646745556153c9cda85a4628b18 \ + --hash=sha256:35c3ad7b7f7d5d4a54a80f0ff5a41ab186237d6486843f8dde00c42cfab33905 \ + --hash=sha256:459ef557e669d0046fe2b92eb4822c097c00b5ef9d11df0f9bd7d4267acdfc52 \ + --hash=sha256:4a50513b6be001b9b7be2c435478fe9669249c77c241813907a44cda1fcd03f4 \ + --hash=sha256:51035b175740c44707694c521560b55b66da9d5a7c545cf22582bc02deb61664 \ + --hash=sha256:96f2cdb35bdfda10e075f12892a42cff5179bbda698992b845f36c5e92755d33 \ + --hash=sha256:a0aed261060cd0372abf08d16399b1224dbb5b400312e6b00f2b23eabe1d4e96 \ + --hash=sha256:a6052c4c7d95de2345d9c58fc0fe34fff6c27a8ed8550dafeb18ada84406cc99 \ + --hash=sha256:cbf1354292a4f7abb6a0188f74f5e902e4510ebad105be1dbc4809d1ed92f77e \ + --hash=sha256:da82b2372f5ded8806eaac95b19af89a7174efdb418d4e7beb0c6ab09cee7d95 \ + --hash=sha256:dd89f466c930d7cfe84c94b5cbe862867c88b269f23e5aa61d40945e0d746f54 \ + --hash=sha256:e3183fbeb452ec11670c2d9bfd08a57bc87e46856b24d1c335f995239bedd0e1 \ + --hash=sha256:e9a571e7168076a0d5ecaabd91e9032e86d815cca3a4bf0dafead539ef071aa5 \ + --hash=sha256:ec6aba217d0c4f71cbe48aea962a382dedcd111f47b55e8b58d4aaca519bd360 \ + # via openapi-core libthumbor==2.0.1 \ --hash=sha256:3c4e1a59c019d22f868d225315c06f97fad30fb5e78112d6a230b978e7d24e38 \ --hash=sha256:ed4fe5f27f8f90e7285b7e6dce99c1b67d43a140bf370e989080b43d80ce25f0 \ @@ -395,10 +423,32 @@ matrix-client==0.3.2 \ --hash=sha256:2855a2614a177db66f9bc3ba38cbd2876041456f663c334f72a160ab6bb11c49 \ --hash=sha256:dce3ccb8665df0d519f08e07a16e6d3f9fab3a947df4b7a7c4bb26573d68f2d5 \ # via zulip +more-itertools==8.4.0 \ + --hash=sha256:68c70cc7167bdf5c7c9d8f6954a7837089c6a36bf565383919bb595efb8a17e5 \ + --hash=sha256:b78134b2063dd214000685165d81c154522c3ee0a1c0d4d113c80361c234c5a2 \ + # via openapi-core oauthlib==3.1.0 \ --hash=sha256:bee41cc35fcca6e988463cacc3bcb8a96224f470ca547e697b604cc697b2f889 \ --hash=sha256:df884cd6cbe20e32633f1db1072e9356f53638e4361bef4e8b03c9127c9328ea \ # via requests-oauthlib, social-auth-core +openapi-core==0.13.3 \ + --hash=sha256:57973b7383214a529012cf65ddac8c22b25a4df497366e588e88c627581c2928 \ + --hash=sha256:8c5e2053c51f8d43d241b54d322f6c1c32adf1a20617490d361d9c0d5a123448 \ + --hash=sha256:df53cc8b9d217616bbde4e6ef943c9213af810a01cd74f62e7885756c6080af4 \ + # via -r requirements/common.in +openapi-schema-validator==0.1.1 \ + --hash=sha256:3f0b0f9086e7d717a0413a462d3d9e6f82f7e80a744abf21943ee2e0d9e8c50d \ + --hash=sha256:8fc97a575393d179d70e7c7ebd30ed9fc46eb6c5013f2790736c2e50ea150f06 \ + --hash=sha256:b7afe93aff4a876781279b376c1ba5decb338483e9484af26b140ef215119e23 \ + # via openapi-core +openapi-spec-validator==0.2.8 \ + --hash=sha256:0caacd9829e9e3051e830165367bf58d436d9487b29a09220fa7edb9f47ff81b \ + --hash=sha256:d4da8aef72bf5be40cf0df444abd20009a41baf9048a8e03750c07a934f1bdd8 \ + --hash=sha256:e489c7a273284bc78277ac22791482e8058d323b4a265015e9fcddf6a8045bcd \ + # via openapi-core +parse==1.15.0 \ + --hash=sha256:a6d4e2c2f1fbde6717d28084a191a052950f758c0cbd83805357e6575c2b95c0 \ + # via openapi-core parso==0.7.0 \ --hash=sha256:158c140fc04112dc45bca311633ae5033c2c2a7b732fa33d0955bad8152a8dd0 \ --hash=sha256:908e9fae2144a076d72ae4e25539143d40b8e3eafbaeae03c1bfe226f4cdf12c \ @@ -514,6 +564,9 @@ pyopenssl==19.1.0 \ --hash=sha256:621880965a720b8ece2f1b2f54ea2071966ab00e2970ad2ce11d596102063504 \ --hash=sha256:9a24494b2602aaf402be5c9e30a0b82d4a5c67528fe8fb475e3f3bc00dd69507 \ # via requests +pyrsistent==0.16.0 \ + --hash=sha256:28669905fe725965daa16184933676547c5bb40a5153055a8dee2a4bd7933ad3 \ + # via jsonschema python-dateutil==2.8.1 \ --hash=sha256:73ebfe9dbf22e832286dafa60473e4cd239f8592f699aa5adaf10050e6e1823c \ --hash=sha256:75bb3f31ea686f1197762692a9ee6a7550b59fc6ca3a1f4b5d7e32fb98e2da2a \ @@ -557,7 +610,7 @@ pyyaml==5.3.1 \ --hash=sha256:b8eac752c5e14d3eca0e6dd9199cd627518cb5ec06add0de9d32baeee6fe645d \ --hash=sha256:cc8955cfbfc7a115fa81d85284ee61147059a753344bc51098f3ccd69b0d7e0c \ --hash=sha256:d13155f591e6fcc1ec3b30685d50bf0711574e2c0dfffd7644babf8b5102ca1a \ - # via yamole + # via openapi-spec-validator, yamole qrcode==6.1 \ --hash=sha256:3996ee560fc39532910603704c82980ff6d4d5d629f9c3f25f34174ce8606cf5 \ --hash=sha256:505253854f607f2abf4d16092c61d4e9d511a3b4392e60bff957a68592b04369 \ @@ -604,7 +657,7 @@ s3transfer==0.3.3 \ six==1.15.0 \ --hash=sha256:30639c035cdb23534cd4aa2dd52c3bf48f06e5f4a941509c8bafd8ce11080259 \ --hash=sha256:8b74bedcbbbaca38ff6d7491d76f2b06b3592611af620f8426e82dddb04a5ced \ - # via -r requirements/common.in, argon2-cffi, cryptography, django-bitfield, hypchat, isodate, libthumbor, pyopenssl, python-dateutil, qrcode, social-auth-app-django, social-auth-core, talon, traitlets, twilio, zulip + # via -r requirements/common.in, argon2-cffi, cryptography, django-bitfield, hypchat, isodate, jsonschema, libthumbor, openapi-core, openapi-schema-validator, openapi-spec-validator, pyopenssl, python-dateutil, qrcode, social-auth-app-django, social-auth-core, talon, traitlets, twilio, zulip social-auth-app-django==4.0.0 \ --hash=sha256:2c69e57df0b30c9c1823519c5f1992cbe4f3f98fdc7d95c840e091a752708840 \ --hash=sha256:567ad0e028311541d7dfed51d3bf2c60440a6fd236d5d4d06c5a618b3d6c57c5 \ @@ -661,6 +714,9 @@ statsd==3.3.0 \ --hash=sha256:c610fb80347fca0ef62666d241bce64184bd7cc1efe582f9690e045c25535eaa \ --hash=sha256:e3e6db4c246f7c59003e51c9720a51a7f39a396541cb9b147ff4b14d15b5dd1f \ # via django-statsd-mozilla +strict-rfc3339==0.7 \ + --hash=sha256:5cad17bedfc3af57b399db0fed32771f18fc54bbd917e85546088607ac5e1277 \ + # via openapi-schema-validator stripe==2.48.0 \ --hash=sha256:515fe2cc915e639468f30150a39c162fc0fb090256ae9d6a04e5022925d136f1 \ --hash=sha256:bdbbea632b8faa983c670db61debbe0bdb5802ef98fd0613a03aa466e56cdade \ @@ -709,6 +765,10 @@ wcwidth==0.2.5 \ --hash=sha256:beb4802a9cebb9144e99086eff703a642a13d6a0052920003a230f3294bbe784 \ --hash=sha256:c4d647b99872929fdb7bdcaa4fbe7f01413ed3d98077df798530e5b04f116c83 \ # via prompt-toolkit +werkzeug==1.0.1 \ + --hash=sha256:2de2a5db0baeae7b2d2664949077c2ac63fbd16d98da0ff71837f7d1dea3fd43 \ + --hash=sha256:6c80b1e5ad3665290ea39320b91e1be1e0d5f60652b964a3070216de83d2e47c \ + # via openapi-core xmlsec==1.3.8 \ --hash=sha256:13cf2c9a82df9f19ddc6c3d9baa1a92aba8011645c29bb4e7df4e3b6a51a86f3 \ --hash=sha256:13e8f99d88b3b630f2b72a7f32edab051dd490e8a070e4b9138f8335a981eea8 \ @@ -748,4 +808,4 @@ pip==20.1.1 \ setuptools==47.3.1 \ --hash=sha256:4ba6f9789ea243a6b8ba57da81f75a53494456117810436fd9277a74d1c915d1 \ --hash=sha256:843037738d1e34e8b326b5e061f474aca6ef9d7ece41329afbc8aac6195a3920 \ - # via ipython + # via ipython, jsonschema diff --git a/version.py b/version.py index 2a694029d0..2b6cd82dcf 100644 --- a/version.py +++ b/version.py @@ -44,4 +44,4 @@ API_FEATURE_LEVEL = 23 # historical commits sharing the same major version, in which case a # minor version bump suffices. -PROVISION_VERSION = '87.1' +PROVISION_VERSION = '87.2' diff --git a/zerver/lib/markdown/api_return_values_table_generator.py b/zerver/lib/markdown/api_return_values_table_generator.py index 93ee1b35ca..6a4779d13a 100644 --- a/zerver/lib/markdown/api_return_values_table_generator.py +++ b/zerver/lib/markdown/api_return_values_table_generator.py @@ -68,7 +68,7 @@ class APIReturnValuesTablePreprocessor(Preprocessor): ans.append(self.render_desc(description, spacing, return_value)) if 'properties' in return_values[return_value]: ans += self.render_table(return_values[return_value]['properties'], spacing + 4) - if 'additionalProperties' in return_values[return_value]: + if return_values[return_value].get('additionalProperties', False): ans.append(self.render_desc(return_values[return_value]['additionalProperties'] ['description'], spacing + 4)) if 'properties' in return_values[return_value]['additionalProperties']: diff --git a/zerver/openapi/openapi.py b/zerver/openapi/openapi.py index 0a54e2e1c1..7479689c38 100644 --- a/zerver/openapi/openapi.py +++ b/zerver/openapi/openapi.py @@ -4,39 +4,18 @@ import os import re from typing import Any, Dict, List, Optional, Set +from openapi_schema_validator import OAS30Validator + OPENAPI_SPEC_PATH = os.path.abspath(os.path.join( os.path.dirname(__file__), '../openapi/zulip.yaml')) -# A list of exceptions we allow when running validate_against_openapi_schema. -# The validator will ignore these keys when they appear in the "content" -# passed. -EXCLUDE_PROPERTIES = { - '/events': { - 'get': { - # Array with opaque object - '200': ['events'] - } - }, - '/register': { - 'post': { - '200': ['max_message_id', 'realm_emoji'], - }, - }, - '/settings/notifications': { - 'patch': { - # Some responses contain undocumented keys - '200': ['notification_sound', 'enable_login_emails', - 'enable_stream_desktop_notifications', 'wildcard_mentions_notify', - 'pm_content_in_desktop_notifications', 'desktop_icon_count_display', - 'realm_name_in_notifications', 'presence_enabled'], - }, - }, -} - # A list of endpoint-methods such that the endpoint # has documentation but not with this particular method. -EXCLUDE_ENDPOINTS = ["/realm/emoji/{emoji_name}:delete"] +EXCLUDE_UNDOCUMENTED_ENDPOINTS = {"/realm/emoji/{emoji_name}:delete"} +# Consists of endpoints with some documentation remaining. +# These are skipped but return true as the validator cannot exclude objects +EXCLUDE_DOCUMENTED_ENDPOINTS = {"/events:get", "/register:post", "/settings/notifications:patch"} class OpenAPISpec(): def __init__(self, path: str) -> None: self.path = path @@ -200,6 +179,10 @@ def validate_against_openapi_schema(content: Dict[str, Any], endpoint: str, """Compare a "content" dict with the defined schema for a specific method in an endpoint. Return true if validated and false if skipped. """ + + # This first set of checks are primarily training wheels that we + # hope to eliminate over time as we improve our API documentation. + # No 500 responses have been documented, so skip them if response.startswith('5'): return False @@ -210,121 +193,74 @@ def validate_against_openapi_schema(content: Dict[str, Any], endpoint: str, return False endpoint = match # Excluded endpoint/methods - if endpoint + ':' + method in EXCLUDE_ENDPOINTS: + if endpoint + ':' + method in EXCLUDE_UNDOCUMENTED_ENDPOINTS: return False - + # Return true for endpoints with only response documentation remaining + if endpoint + ':' + method in EXCLUDE_DOCUMENTED_ENDPOINTS: + return True # Check if the response matches its code if response.startswith('2') and (content.get('result', 'success').lower() != 'success'): raise SchemaError("Response is not 200 but is validating against 200 schema") - # In a single response schema we do not have two keys with the same name. - # Hence exclusion list is declared globally - exclusion_list = (EXCLUDE_PROPERTIES.get(endpoint, {}).get(method.lower(), {}).get(response, [])) - # Code is not declared but appears in various 400 responses. If common, it can be added - # to 400 response schema + # Code is not declared but appears in various 400 responses. If + # common, it can be added to 400 response schema if response.startswith('4'): - exclusion_list.append('code') - # This return statement should ideally be not here. But since we have not defined 400 - # responses for various paths this has been added as all 400 have the same schema. - # When all 400 response have been defined this should be removed. + # This return statement should ideally be not here. But since + # we have not defined 400 responses for various paths this has + # been added as all 400 have the same schema. When all 400 + # response have been defined this should be removed. return True + + # The actual work of validating that the response matches the + # schema is done via the third-party OAS30Validator. schema = get_schema(endpoint, method, response) - validate_object(content, schema, exclusion_list) + validator = OAS30Validator(schema) + validator.validate(content) return True -def validate_array(content: List[Any], schema: Dict[str, Any], exclusion_list: List[str]) -> None: - valid_types: List[type] = [] - object_schema: Optional[Dict[str, Any]] = None - array_schema: Optional[Dict[str, Any]] = None +def validate_schema_array(schema: Dict[str, Any]) -> None: + """ + Helper function for validate_schema + """ if 'oneOf' in schema['items']: for oneof_schema in schema['items']['oneOf']: if oneof_schema['type'] == 'array': - array_schema = oneof_schema + validate_schema_array(oneof_schema) elif oneof_schema['type'] == 'object': - object_schema = oneof_schema - valid_types.append(to_python_type(oneof_schema['type'])) + validate_schema(oneof_schema) else: - valid_types.append(to_python_type(schema['items']['type'])) if schema['items']['type'] == 'array': - array_schema = schema['items'] + validate_schema_array(schema['items']) elif schema['items']['type'] == 'object': - object_schema = schema['items'] + validate_schema(schema['items']) - for item in content: - if type(item) not in valid_types: - raise SchemaError('Wrong data type in array') - # We can directly check for objects and arrays as - # there are no mixed arrays consisting of objects - # and arrays. - if type(item) == dict: - assert object_schema is not None - if 'properties' not in object_schema: - raise SchemaError('Opaque object in array') - validate_object(item, object_schema, exclusion_list) - if type(item) == list: - assert(array_schema is not None) - validate_array(item, array_schema, exclusion_list) +def validate_schema(schema: Dict[str, Any]) -> None: + """Check if opaque objects are present in the OpenAPI spec; this is an + important part of our policy for ensuring every detail of Zulip's + API responses is correct. -def validate_object(content: Dict[str, Any], schema: Dict[str, Any], exclusion_list: List[str]) -> None: - for key, value in content.items(): - object_schema: Optional[Dict[str, Any]] = None - array_schema: Optional[Dict[str, Any]] = None - if key in exclusion_list: - continue - # Check that the key is defined in the schema - if key not in schema['properties']: - raise SchemaError('Extraneous key "{}" in the response\'s ' - 'content'.format(key)) - # Check that the types match - expected_type: List[type] = [] + This is done by checking for the presence of the + `additionalProperties` attribute for all objects (dictionaries). + """ + if 'additionalProperties' not in schema: + raise SchemaError('additionalProperties needs to be defined for objects to make' + + 'sure they have no additional properties left to be documented.') + for key in schema.get('properties', dict()): if 'oneOf' in schema['properties'][key]: for types in schema['properties'][key]['oneOf']: - expected_type.append(to_python_type(types['type'])) if types['type'] == 'object': - object_schema = types + validate_schema(types) elif types['type'] == 'array': - array_schema = types + validate_schema_array(types) else: - expected_type.append(to_python_type(schema['properties'][key]['type'])) if schema['properties'][key]['type'] == 'object': - object_schema = schema['properties'][key] + validate_schema(schema['properties'][key]) elif schema['properties'][key]['type'] == 'array': - array_schema = schema['properties'][key] - - actual_type = type(value) - # We have only define nullable property if it is nullable - if value is None and 'nullable' in schema['properties'][key]: - continue - if actual_type not in expected_type: - raise SchemaError('Expected type {} for key "{}", but actually ' - 'got {}'.format(expected_type, key, actual_type)) - if actual_type == list: - assert array_schema is not None - validate_array(value, array_schema, exclusion_list) - if actual_type == dict: - assert object_schema is not None - if 'properties' in object_schema: - validate_object(value, object_schema, exclusion_list) - continue - if 'additionalProperties' in schema['properties'][key]: - for child_keys in value: - if type(value[child_keys]) == list: - validate_array(value[child_keys], - schema['properties'][key]['additionalProperties'], exclusion_list) - continue - validate_object(value[child_keys], - schema['properties'][key]['additionalProperties'], exclusion_list) - continue - # If the object is not opaque then continue statements - # will be executed above and this will be skipped - if actual_type == dict: - raise SchemaError(f'Opaque object "{key}"') - # Check that at least all the required keys are present - if 'required' in schema: - for req_key in schema['required']: - if req_key in exclusion_list: - continue - if req_key not in content.keys(): - raise SchemaError(f'Expected to find the "{req_key}" required key') + validate_schema_array(schema['properties'][key]) + if schema['additionalProperties']: + if schema['additionalProperties']['type'] == 'array': + validate_schema_array(schema['additionalProperties']) + elif schema['additionalProperties']['type'] == 'object': + validate_schema(schema['additionalProperties']) def to_python_type(py_type: str) -> type: """Transform an OpenAPI-like type to a Python one. diff --git a/zerver/openapi/testing.yaml b/zerver/openapi/testing.yaml index beec44dbf4..ebdaf0240d 100644 --- a/zerver/openapi/testing.yaml +++ b/zerver/openapi/testing.yaml @@ -4,6 +4,7 @@ test1: content: application/json: schema: + additionalProperties: false properties: top_array: type: array @@ -50,6 +51,7 @@ test2: content: application/json: schema: + additionalProperties: false properties: top_array: type: array @@ -63,6 +65,7 @@ test2: items: type: string - type: object + additionalProperties: false properties: str3: type: string @@ -97,6 +100,7 @@ test3: content: application/json: schema: + additionalProperties: false properties: top_array: type: array diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 947a03fc16..0a4ff23bc7 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -157,6 +157,7 @@ paths: consecutive. items: type: object + additionalProperties: false - example: { "events": [ @@ -365,6 +366,7 @@ paths: details about a file uploaded by the user. items: type: object + additionalProperties: false properties: id: type: integer @@ -403,6 +405,7 @@ paths: uploaded file. items: type: object + additionalProperties: false properties: date_sent: type: integer @@ -979,6 +982,7 @@ paths: type: array items: type: object + additionalProperties: false properties: topic: type: string @@ -1273,6 +1277,7 @@ paths: with message IDs as keys and search rendering data as values. additionalProperties: type: object + additionalProperties: false properties: match_content: type: string @@ -1831,6 +1836,7 @@ paths: logged into. additionalProperties: type: object + additionalProperties: false properties: timestamp: type: integer @@ -2092,6 +2098,7 @@ paths: An array of `topic` objects. items: type: object + additionalProperties: false properties: max_id: description: | @@ -2165,6 +2172,7 @@ paths: information about one of the subscribed streams. items: type: object + additionalProperties: false properties: stream_id: type: integer @@ -2559,6 +2567,7 @@ paths: type: array items: type: object + additionalProperties: false properties: name: type: string @@ -2862,6 +2871,7 @@ paths: emoji ID as the key. additionalProperties: type: object + additionalProperties: false description: | `{emoji_id}`: Object containing details about the emoji with the specified ID. It has the following properties: @@ -2946,6 +2956,7 @@ paths: type: array items: type: object + additionalProperties: false properties: property: type: string @@ -2973,6 +2984,16 @@ paths: *`email_notifications`: Whether to trigger an email notification for all messages sent to the stream. + + *`in_home_view`: Whether to mute the stream (Legacy property) + + *`wildcard_mentions_notify`: whether wildcard mentions trigger notifications + as though they were personal mentions in this stream. + + A null value means the value of this setting + should be inherited from the user-level default + setting, wildcard_mentions_notify, for + this stream. enum: - color - push_notifications @@ -2982,6 +3003,8 @@ paths: - audible_notifications - push_notifications - email_notifications + - in_home_view + - wildcard_mentions_notify value: description: | The desired value of the property @@ -3301,6 +3324,7 @@ paths: - properties: authentication_methods: type: object + additionalProperties: false deprecated: true description: | Each key-value pair in the object indicates whether the authentication @@ -3366,6 +3390,7 @@ paths: **Changes**: New in Zulip 2.1. items: type: object + additionalProperties: false properties: name: type: string @@ -3786,6 +3811,7 @@ paths: type: array items: type: object + additionalProperties: false properties: stream_id: type: integer @@ -4274,6 +4300,7 @@ paths: type: array items: type: object + additionalProperties: false properties: description: type: string @@ -4365,6 +4392,7 @@ paths: content: application/json: schema: + additionalProperties: false properties: data: type: string @@ -4465,6 +4493,7 @@ components: schemas: Messages: type: object + additionalProperties: false properties: avatar_url: type: string @@ -4499,6 +4528,7 @@ components: - type: array items: type: object + additionalProperties: false properties: id: type: integer @@ -4545,6 +4575,7 @@ components: Data on any reactions to the message. items: type: object + additionalProperties: false properties: emoji_code: type: string @@ -4578,6 +4609,7 @@ components: object is deprecated and will be removed in the future. user: type: object + additionalProperties: false deprecated: true description: | Dictionary with data on the user who added the reaction, including @@ -4698,6 +4730,7 @@ components: search keywords. User: type: object + additionalProperties: false description: | A dictionary containing basic data on a given Zulip user. properties: @@ -4802,6 +4835,7 @@ components: supporting markdown, a `rendered_value` key will also be present. additionalProperties: type: object + additionalProperties: false description: | '{id}': Object with data about what value user filled in the custom profile field with id `id`. @@ -4821,6 +4855,7 @@ components: as are used for message content. JsonResponse: type: object + additionalProperties: false properties: result: type: string diff --git a/zerver/tests/test_openapi.py b/zerver/tests/test_openapi.py index 36742de0ec..2615a7418f 100644 --- a/zerver/tests/test_openapi.py +++ b/zerver/tests/test_openapi.py @@ -21,10 +21,10 @@ from unittest.mock import MagicMock, patch import yaml from django.http import HttpResponse +from jsonschema.exceptions import ValidationError from zerver.lib.request import _REQ, arguments_map from zerver.lib.test_classes import ZulipTestCase -from zerver.openapi import openapi as openapi from zerver.openapi.markdown_extension import ( generate_curl_example, parse_language_and_options, @@ -41,6 +41,7 @@ from zerver.openapi.openapi import ( openapi_spec, to_python_type, validate_against_openapi_schema, + validate_schema, ) TEST_ENDPOINT = '/messages/{message_id}' @@ -98,9 +99,9 @@ class OpenAPIToolsTest(ZulipTestCase): assert(expected_item in actual) def test_validate_against_openapi_schema(self) -> None: - with self.assertRaises(SchemaError, - msg=('Extraneous key "foo" in ' - 'the response\'scontent')): + with self.assertRaises(ValidationError, + msg=("Additional properties are not" + + " allowed ('foo' was unexpected)")): bad_content: Dict[str, object] = { 'msg': '', 'result': 'success', @@ -111,10 +112,8 @@ class OpenAPIToolsTest(ZulipTestCase): TEST_METHOD, TEST_RESPONSE_SUCCESS) - with self.assertRaises(SchemaError, - msg=("Expected type for key " - "\"msg\", but actually got " - "")): + with self.assertRaises(ValidationError, + msg=("42 is not of type string")): bad_content = { 'msg': 42, 'result': 'success', @@ -124,7 +123,7 @@ class OpenAPIToolsTest(ZulipTestCase): TEST_METHOD, TEST_RESPONSE_SUCCESS) - with self.assertRaises(SchemaError, + with self.assertRaises(ValidationError, msg='Expected to find the "msg" required key'): bad_content = { 'result': 'success', @@ -145,26 +144,6 @@ class OpenAPIToolsTest(ZulipTestCase): TEST_RESPONSE_SUCCESS) # Overwrite the exception list with a mocked one - exclude_properties = openapi.EXCLUDE_PROPERTIES - openapi.EXCLUDE_PROPERTIES = { - TEST_ENDPOINT: { - TEST_METHOD: { - TEST_RESPONSE_SUCCESS: ['foo'], - }, - }, - } - try: - good_content = { - 'msg': '', - 'result': 'success', - 'foo': 'bar', - } - validate_against_openapi_schema(good_content, - TEST_ENDPOINT, - TEST_METHOD, - TEST_RESPONSE_SUCCESS) - finally: - openapi.EXCLUDE_PROPERTIES = exclude_properties test_dict: Dict[str, Any] = {} # Check that validate_against_openapi_schema correctly @@ -181,14 +160,14 @@ class OpenAPIToolsTest(ZulipTestCase): validate_against_openapi_schema((test_dict['test1']['responses']['200']['content'] ['application/json']['example']), 'testing', 'test1', '200') - with self.assertRaises(SchemaError, msg = 'Extraneous key "str4" in response\'s content'): + with self.assertRaises(ValidationError, msg = 'Extraneous key "str4" in response\'s content'): validate_against_openapi_schema((test_dict['test2']['responses']['200'] ['content']['application/json']['example']), 'testing', 'test2', '200') with self.assertRaises(SchemaError, msg = 'Opaque object "obj"'): - validate_against_openapi_schema((test_dict['test3']['responses']['200'] - ['content']['application/json']['example']), - 'testing', 'test3', '200') + # Checks for opaque objects + validate_schema((test_dict['test3']['responses']['200'] + ['content']['application/json']['schema'])) finally: openapi_spec.spec()['paths'].pop('testing', None) @@ -1032,7 +1011,13 @@ class TestCurlExampleGeneration(ZulipTestCase): class OpenAPIAttributesTest(ZulipTestCase): def test_attributes(self) -> None: - EXCLUDE = ["/real-time"] + """ + Checks: + * All endpoints have `operationId` and `tag` attributes. + * All example responses match their schema. + * That no opaque object exists. + """ + EXCLUDE = ["/real-time", "/register", "/events"] VALID_TAGS = ["users", "server_and_organizations", "authentication", "real_time_events", "streams", "messages", "users", "webhooks"] @@ -1052,10 +1037,12 @@ class OpenAPIAttributesTest(ZulipTestCase): if 'oneOf' in response_schema: cnt = 0 for entry in response_schema['oneOf']: + validate_schema(entry) assert(validate_against_openapi_schema(entry['example'], path, method, response + '_' + str(cnt))) cnt += 1 continue + validate_schema(response_schema) assert(validate_against_openapi_schema(response_schema['example'], path, method, response))