)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"dfde0e65c5bf9c59ca1c62255814dbac829dc977","unresolved":true,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2025-05-14 20:56:19 -0700"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"s3token: Ensure check_signature gets called when caching secrets"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: Ife37041c1e83eea3e108ad80101d84a5ebf70f2b"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"e205a8b9_2807d3a4","line":8,"updated":"2025-05-29 10:06:05.000000000","message":"would be useful to have\n\n``Related-Change: I7be1ce9eb5dba7b17bdf3e53b0d05d25ac0a05b0``\n\ni.e. aws-chunked transfer support","commit_id":"5d8a12e998df4924b178a74099ba51df839cee64"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"64d2af659f7bab979cde97253893c86b85b1fcd3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"7b6185c9_233c3d63","updated":"2025-05-29 16:52:53.000000000","message":"I just realized -- we don\u0027t have anything to configure `secret_cache_duration` for the DSVM jobs... so I\u0027m really confused about how tests like `test_strm_sgnd_pyld_good_signatures_ok` are passing with https://review.opendev.org/c/openstack/swift/+/942133 ... Should that be getting a 501?","commit_id":"5d8a12e998df4924b178a74099ba51df839cee64"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"f790727c71de8f74c531d22621c2b37368c8218c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"61abb581_d9622a88","updated":"2025-05-28 18:34:50.000000000","message":"Pretty small and tame -- WDYT?","commit_id":"5d8a12e998df4924b178a74099ba51df839cee64"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"dfde0e65c5bf9c59ca1c62255814dbac829dc977","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"f52d8eec_667e00a0","updated":"2025-05-29 10:06:05.000000000","message":"This should probably be rebased so it has the context of chunked transfer support.\n\nIIUC this only works if ``secret_cache_duration`` is \u003e 0, which is a pain because I think it means the second keystone request is necessary even if caching is disabled.\n\nThis test illustrates the lack of ``check_signature`` callback:\n\n```\ndiff --git a/test/unit/common/middleware/s3api/test_s3token.py b/test/unit/common/middleware/s3api/test_s3token.py\nindex 34033d61b..1d25da098 100644\n--- a/test/unit/common/middleware/s3api/test_s3token.py\n+++ b/test/unit/common/middleware/s3api/test_s3token.py\n@@ -239,6 +239,27 @@ class S3TokenMiddlewareTestGood(S3TokenMiddlewareTestBase):\n         req.get_response(self.middleware)\n         self._assert_authorized(req)\n \n+    def test_authorized_calls_check_signature(self):\n+        # note: default conf has no secret_cache_duration\n+        fake_check_signature \u003d mock.Mock()\n+        req \u003d Request.blank(\u0027/v1/AUTH_cfa/c/o\u0027)\n+        req.environ[\u0027s3api.auth_details\u0027] \u003d {\n+            \u0027access_key\u0027: u\u0027access\u0027,\n+            \u0027signature\u0027: u\u0027signature\u0027,\n+            \u0027string_to_sign\u0027: u\u0027token\u0027,\n+            \u0027check_signature\u0027: fake_check_signature,\n+        }\n+        req.environ[\u0027swift.cache\u0027] \u003d mock.Mock()\n+        middleware \u003d self.make_middleware({\n+            \u0027auth_uri\u0027: self.TEST_AUTH_URI,\n+            \u0027secret_cache_duration\u0027: 0,\n+        })\n+        req.get_response(middleware)\n+        self._assert_authorized(req)\n+        # XXX no secret fetched from keystone, no secret passed to the\n+        # SigChecker :(\n+        self.assertEqual([], fake_check_signature.call_args_list)\n+\n     def test_tolerate_missing_token_id(self):\n         resp \u003d copy.deepcopy(GOOD_RESPONSE_V2)\n         del resp[\u0027access\u0027][\u0027token\u0027][\u0027id\u0027]\n\n```","commit_id":"5d8a12e998df4924b178a74099ba51df839cee64"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"5ffba1c42538e6c0e6a117f39fd3539b4913383b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"f6d276c9_f291e34c","in_reply_to":"7b6185c9_233c3d63","updated":"2025-05-29 21:40:49.000000000","message":"Turns out, the same (tempauth) test users were getting used for both cross-compat runs :-(\n\nhttps://review.opendev.org/c/openstack/swift/+/951339 fixes that, but doesn\u0027t pass yet (even once I *do* start setting `secret_cache_duration`)","commit_id":"5d8a12e998df4924b178a74099ba51df839cee64"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"94758928dcdb1f9d1fb53f249cec670473b480c0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"ca0c4e8f_9ba7f31e","in_reply_to":"f52d8eec_667e00a0","updated":"2025-05-29 16:40:05.000000000","message":"\u003e IIUC this only works if `secret_cache_duration` is \u003e 0, which is a pain because I think it means the second keystone request is necessary even if caching is disabled.\n\nI\u0027m increasingly of the opinion that deploying s3token with secret caching disabled makes about as much sense as deploying proxy servers without a memcache client. Sure, maybe you can kind of make it work, but performance is going to suck and your backend servers are going to melt.\n\nI should probably just update the default to something like 60s and call out in the sample config that ops might prefer something in the 5-15 min range.","commit_id":"5d8a12e998df4924b178a74099ba51df839cee64"}],"swift/common/middleware/s3api/s3token.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"dfde0e65c5bf9c59ca1c62255814dbac829dc977","unresolved":true,"context_lines":[{"line_number":365,"context_line":"                        cred_ref \u003d self.keystoneclient.ec2.get("},{"line_number":366,"context_line":"                            user_id\u003duser_id,"},{"line_number":367,"context_line":"                            access\u003daccess)"},{"line_number":368,"context_line":"                        if not s3_auth_details[\u0027check_signature\u0027]("},{"line_number":369,"context_line":"                                cred_ref.secret):"},{"line_number":370,"context_line":"                            # We shouldn\u0027t get in this block (keystone *just*"},{"line_number":371,"context_line":"                            # told us the signature was valid), but we wanted"}],"source_content_type":"text/x-python","patch_set":2,"id":"ecdbdb41_309ea1cc","line":368,"updated":"2025-05-29 10:06:05.000000000","message":"there\u0027s a bunch of tests that do not have ``\u0027check_signature\u0027`` in their s3api.auth_details e.g. test.unit.common.middleware.s3api.test_s3token.S3TokenMiddlewareTestGood.test_authorized\n\n...which suggests this is not always reached...which is because it is conditional on there being memcache_client, which in turn on there being non zero ``secret_cache_duration``.\n\nSo, I think that when the secret is not returned from memcache we\u0027re going to have to always call ``self.keystoneclient.ec2.get`` in order to call ``check_signature`` with a secret.\n\nMaybe we should add a ``check_signature_required`` flag to auth_details which is only True for signed streaming requests?","commit_id":"5d8a12e998df4924b178a74099ba51df839cee64"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"94758928dcdb1f9d1fb53f249cec670473b480c0","unresolved":true,"context_lines":[{"line_number":365,"context_line":"                        cred_ref \u003d self.keystoneclient.ec2.get("},{"line_number":366,"context_line":"                            user_id\u003duser_id,"},{"line_number":367,"context_line":"                            access\u003daccess)"},{"line_number":368,"context_line":"                        if not s3_auth_details[\u0027check_signature\u0027]("},{"line_number":369,"context_line":"                                cred_ref.secret):"},{"line_number":370,"context_line":"                            # We shouldn\u0027t get in this block (keystone *just*"},{"line_number":371,"context_line":"                            # told us the signature was valid), but we wanted"}],"source_content_type":"text/x-python","patch_set":2,"id":"a32394e2_e3f284d1","line":368,"in_reply_to":"ecdbdb41_309ea1cc","updated":"2025-05-29 16:40:05.000000000","message":"Alternative perspective: `secret_cache_duration \u003d 0` indicates that ops don\u0027t want swift fetching \u0026 caching secrets from keystone -- and then IDK that swift memory should be treated differently from memcache. In that case 501 is the only option.","commit_id":"5d8a12e998df4924b178a74099ba51df839cee64"}]}