)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7414,"name":"David Wilde","email":"dwilde@redhat.com","username":"d34dh0r53"},"change_message_id":"bfc673b731d8baa1c1fec344eff56d9d04b72bb7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"64f7a55b_32470ea4","updated":"2025-01-10 14:34:14.000000000","message":"recheck","commit_id":"095059b32a970c37c354b803e7b57041d3717146"},{"author":{"_account_id":7414,"name":"David Wilde","email":"dwilde@redhat.com","username":"d34dh0r53"},"change_message_id":"b2891b4fa741ecb5bbba4eac98798e055ecaf82b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"462c4628_62398dba","updated":"2025-03-07 14:33:27.000000000","message":"A couple of nits but overall it\u0027s just about ready, I agree with Greg on adding a maximum to prevent leaking the entire hash.","commit_id":"f72b1aaec7fc6d8ac2186e7215ae991be8368b1f"},{"author":{"_account_id":7414,"name":"David Wilde","email":"dwilde@redhat.com","username":"d34dh0r53"},"change_message_id":"6705d50f4a4105f780f48c368ae3ad36e9e13cdc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"9353e82d_34c4daae","in_reply_to":"462c4628_62398dba","updated":"2025-03-07 14:38:07.000000000","message":"Nevermind, I remember the conversation regarding the max length now, we don\u0027t have to specify a max and should leave that up to the CSP.","commit_id":"f72b1aaec7fc6d8ac2186e7215ae991be8368b1f"},{"author":{"_account_id":27900,"name":"Artem Goncharov","email":"artem.goncharov@gmail.com","username":"gtema"},"change_message_id":"84590797b6704a66734e5abaea2b2614f6cc5a2d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"a5e14be5_654fbcea","updated":"2025-03-21 14:53:40.000000000","message":"I think it is worth of leaving few words about that feature in the documentation. Simply the config is not that helpful. We have already something on https://docs.openstack.org/keystone/latest/admin/event_notifications.html and I would suggest to extend it with a dedicated chapter on that describing how to configure it and show how the events would look like","commit_id":"7db8d5fe58cb719b15d63d3f74bfb8535eda179c"},{"author":{"_account_id":37374,"name":"Stanislav Zaprudskiy","email":"s.zaprudskiy@sap.com","username":"stanislav-z"},"change_message_id":"7faed27f5a499d13f0ab50fece7fb2c4ed77ef50","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":10,"id":"cbc0cb2d_60bc4b4d","in_reply_to":"a5e14be5_654fbcea","updated":"2025-03-27 18:11:45.000000000","message":"I agree. This https://review.opendev.org/c/openstack/keystone/+/945733/1 should be it. Please, review.","commit_id":"7db8d5fe58cb719b15d63d3f74bfb8535eda179c"}],"keystone/common/password_hashing.py":[{"author":{"_account_id":27900,"name":"Artem Goncharov","email":"artem.goncharov@gmail.com","username":"gtema"},"change_message_id":"cb51260548bdc4c9d03444f16b18624f276a47a5","unresolved":true,"context_lines":[{"line_number":214,"context_line":"    # truncating"},{"line_number":215,"context_line":"    encoded \u003d base64.b64encode(peppered).decode(\"utf-8\").rstrip(\"\u003d\")"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"    max_chars \u003d ("},{"line_number":218,"context_line":"        kwargs.get(\"max_chars\")"},{"line_number":219,"context_line":"        or CONF.security_compliance.invalid_password_hash_max_chars"},{"line_number":220,"context_line":"    )"}],"source_content_type":"text/x-python","patch_set":9,"id":"abe9e744_eff94942","line":217,"updated":"2025-03-07 14:44:55.000000000","message":"looks like you only pass max_chars from tests. But in tests you can also set the config. It would be much cleaner to rely on the config value here","commit_id":"f72b1aaec7fc6d8ac2186e7215ae991be8368b1f"},{"author":{"_account_id":27900,"name":"Artem Goncharov","email":"artem.goncharov@gmail.com","username":"gtema"},"change_message_id":"84590797b6704a66734e5abaea2b2614f6cc5a2d","unresolved":false,"context_lines":[{"line_number":214,"context_line":"    # truncating"},{"line_number":215,"context_line":"    encoded \u003d base64.b64encode(peppered).decode(\"utf-8\").rstrip(\"\u003d\")"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"    max_chars \u003d ("},{"line_number":218,"context_line":"        kwargs.get(\"max_chars\")"},{"line_number":219,"context_line":"        or CONF.security_compliance.invalid_password_hash_max_chars"},{"line_number":220,"context_line":"    )"}],"source_content_type":"text/x-python","patch_set":9,"id":"5cd6fe28_6c21e05b","line":217,"in_reply_to":"a8838082_a4803d08","updated":"2025-03-21 14:53:40.000000000","message":"Done","commit_id":"f72b1aaec7fc6d8ac2186e7215ae991be8368b1f"},{"author":{"_account_id":37374,"name":"Stanislav Zaprudskiy","email":"s.zaprudskiy@sap.com","username":"stanislav-z"},"change_message_id":"1eb71fcc705b8c38d8d4c6eff53da7d0a7bdaf13","unresolved":true,"context_lines":[{"line_number":214,"context_line":"    # truncating"},{"line_number":215,"context_line":"    encoded \u003d base64.b64encode(peppered).decode(\"utf-8\").rstrip(\"\u003d\")"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"    max_chars \u003d ("},{"line_number":218,"context_line":"        kwargs.get(\"max_chars\")"},{"line_number":219,"context_line":"        or CONF.security_compliance.invalid_password_hash_max_chars"},{"line_number":220,"context_line":"    )"}],"source_content_type":"text/x-python","patch_set":9,"id":"a8838082_a4803d08","line":217,"in_reply_to":"abe9e744_eff94942","updated":"2025-03-11 13:28:11.000000000","message":"Indeed, all `kwargs` were only used in tests exactly to avoid dealing with config. I pushed an update which drops `max_chars` and other `kwargs`, and uses CONF in tests - still looks good, IMHO 😊 - and hopefully cleaner.","commit_id":"f72b1aaec7fc6d8ac2186e7215ae991be8368b1f"}],"keystone/conf/security_compliance.py":[{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"325cbd3b728f2c4621c101b6674b55703bcbb707","unresolved":true,"context_lines":[{"line_number":209,"context_line":""},{"line_number":210,"context_line":"invalid_password_hash_max_chars \u003d cfg.IntOpt("},{"line_number":211,"context_line":"    \u0027invalid_password_hash_max_chars\u0027,"},{"line_number":212,"context_line":"    min\u003d1,"},{"line_number":213,"context_line":"    sample_default\u003d5,"},{"line_number":214,"context_line":"    help\u003dutils.fmt("},{"line_number":215,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"aa32cd2f_747ad8a7","line":212,"updated":"2025-03-07 14:26:59.000000000","message":"Could we add a max here? To ensure that the full hashes cannot be accidentally leaked by changing the config. Maybe a value around 10 would work?","commit_id":"f72b1aaec7fc6d8ac2186e7215ae991be8368b1f"},{"author":{"_account_id":37374,"name":"Stanislav Zaprudskiy","email":"s.zaprudskiy@sap.com","username":"stanislav-z"},"change_message_id":"1eb71fcc705b8c38d8d4c6eff53da7d0a7bdaf13","unresolved":true,"context_lines":[{"line_number":209,"context_line":""},{"line_number":210,"context_line":"invalid_password_hash_max_chars \u003d cfg.IntOpt("},{"line_number":211,"context_line":"    \u0027invalid_password_hash_max_chars\u0027,"},{"line_number":212,"context_line":"    min\u003d1,"},{"line_number":213,"context_line":"    sample_default\u003d5,"},{"line_number":214,"context_line":"    help\u003dutils.fmt("},{"line_number":215,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"021b22da_5075e3c1","line":212,"in_reply_to":"aa32cd2f_747ad8a7","updated":"2025-03-11 13:28:11.000000000","message":"In the spec discussion it\u0027s been agreed to not truncate it (https://specs.openstack.org/openstack/keystone-specs/specs/keystone/2025.1/pci-dss-invalid-password-reporting.html#other-deployer-impact), and leave it up to CSP to configure.","commit_id":"f72b1aaec7fc6d8ac2186e7215ae991be8368b1f"}],"keystone/tests/unit/common/test_notifications.py":[{"author":{"_account_id":27900,"name":"Artem Goncharov","email":"artem.goncharov@gmail.com","username":"gtema"},"change_message_id":"cb51260548bdc4c9d03444f16b18624f276a47a5","unresolved":true,"context_lines":[{"line_number":1261,"context_line":"        )"},{"line_number":1262,"context_line":"        hashed_invalid_password \u003d \u0027hashed_\u0027 + invalid_password"},{"line_number":1263,"context_line":"        with mock.patch.object("},{"line_number":1264,"context_line":"            notifications, \u0027generate_partial_password_hash\u0027"},{"line_number":1265,"context_line":"        ) as mocked:"},{"line_number":1266,"context_line":"            mocked.return_value \u003d hashed_invalid_password"},{"line_number":1267,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"0253ea5f_e007eb66","line":1264,"updated":"2025-03-07 14:44:55.000000000","message":"it looks confusing that you mock the import and not the function itself. It took me a while to understand what is `notifications.generate_partial_password_hash`. I would prefer if you mock `common.password_hashing.generate_partial_password_hash`","commit_id":"f72b1aaec7fc6d8ac2186e7215ae991be8368b1f"},{"author":{"_account_id":37374,"name":"Stanislav Zaprudskiy","email":"s.zaprudskiy@sap.com","username":"stanislav-z"},"change_message_id":"1eb71fcc705b8c38d8d4c6eff53da7d0a7bdaf13","unresolved":true,"context_lines":[{"line_number":1261,"context_line":"        )"},{"line_number":1262,"context_line":"        hashed_invalid_password \u003d \u0027hashed_\u0027 + invalid_password"},{"line_number":1263,"context_line":"        with mock.patch.object("},{"line_number":1264,"context_line":"            notifications, \u0027generate_partial_password_hash\u0027"},{"line_number":1265,"context_line":"        ) as mocked:"},{"line_number":1266,"context_line":"            mocked.return_value \u003d hashed_invalid_password"},{"line_number":1267,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"53b60991_69c550c6","line":1264,"in_reply_to":"0253ea5f_e007eb66","updated":"2025-03-11 13:28:11.000000000","message":"The patching https://docs.python.org/3/library/unittest.mock.html#where-to-patch is not intuitive to me overall 😞, but I hope an update implements your suggestion, which is indeed makes it easier to follow.","commit_id":"f72b1aaec7fc6d8ac2186e7215ae991be8368b1f"},{"author":{"_account_id":27900,"name":"Artem Goncharov","email":"artem.goncharov@gmail.com","username":"gtema"},"change_message_id":"84590797b6704a66734e5abaea2b2614f6cc5a2d","unresolved":false,"context_lines":[{"line_number":1261,"context_line":"        )"},{"line_number":1262,"context_line":"        hashed_invalid_password \u003d \u0027hashed_\u0027 + invalid_password"},{"line_number":1263,"context_line":"        with mock.patch.object("},{"line_number":1264,"context_line":"            notifications, \u0027generate_partial_password_hash\u0027"},{"line_number":1265,"context_line":"        ) as mocked:"},{"line_number":1266,"context_line":"            mocked.return_value \u003d hashed_invalid_password"},{"line_number":1267,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"6270d8d4_8d1f4d36","line":1264,"in_reply_to":"53b60991_69c550c6","updated":"2025-03-21 14:53:40.000000000","message":"Done","commit_id":"f72b1aaec7fc6d8ac2186e7215ae991be8368b1f"}],"releasenotes/notes/pci-dss-invalid-password-reporting-975955d2d79c21b3.yaml":[{"author":{"_account_id":37374,"name":"Stanislav Zaprudskiy","email":"s.zaprudskiy@sap.com","username":"stanislav-z"},"change_message_id":"ef1c346fa46061a71b812c3ebdc3251ebefee50d","unresolved":true,"context_lines":[{"line_number":7,"context_line":"    of submitted invalid passwords, which could be used to facilitate analysis"},{"line_number":8,"context_line":"    of failed login attempts (as per PCI DSS requirements). The corresponding"},{"line_number":9,"context_line":"    Keystone specification - `Include invalid password details in audit"},{"line_number":10,"context_line":"    messages \u003chttps://specs.openstack.org/openstack/keystone-specs/specs/keystone/backlog/pci-dss-invalid-password-reporting.html\u003e`_."}],"source_content_type":"text/x-yaml","patch_set":9,"id":"1c1bf4ca_de056615","line":10,"range":{"start_line":10,"start_character":14,"end_line":10,"end_character":129},"updated":"2025-02-18 14:06:25.000000000","message":"The link will have to be changed if and when https://review.opendev.org/c/openstack/keystone-specs/+/942084 is merged.","commit_id":"f72b1aaec7fc6d8ac2186e7215ae991be8368b1f"},{"author":{"_account_id":7414,"name":"David Wilde","email":"dwilde@redhat.com","username":"d34dh0r53"},"change_message_id":"b2891b4fa741ecb5bbba4eac98798e055ecaf82b","unresolved":true,"context_lines":[{"line_number":7,"context_line":"    of submitted invalid passwords, which could be used to facilitate analysis"},{"line_number":8,"context_line":"    of failed login attempts (as per PCI DSS requirements). The corresponding"},{"line_number":9,"context_line":"    Keystone specification - `Include invalid password details in audit"},{"line_number":10,"context_line":"    messages \u003chttps://specs.openstack.org/openstack/keystone-specs/specs/keystone/backlog/pci-dss-invalid-password-reporting.html\u003e`_."}],"source_content_type":"text/x-yaml","patch_set":9,"id":"36a050dc_f906734c","line":10,"range":{"start_line":10,"start_character":14,"end_line":10,"end_character":129},"in_reply_to":"1c1bf4ca_de056615","updated":"2025-03-07 14:33:27.000000000","message":"We can update this link, the spec has merged.","commit_id":"f72b1aaec7fc6d8ac2186e7215ae991be8368b1f"},{"author":{"_account_id":37374,"name":"Stanislav Zaprudskiy","email":"s.zaprudskiy@sap.com","username":"stanislav-z"},"change_message_id":"1eb71fcc705b8c38d8d4c6eff53da7d0a7bdaf13","unresolved":false,"context_lines":[{"line_number":7,"context_line":"    of submitted invalid passwords, which could be used to facilitate analysis"},{"line_number":8,"context_line":"    of failed login attempts (as per PCI DSS requirements). The corresponding"},{"line_number":9,"context_line":"    Keystone specification - `Include invalid password details in audit"},{"line_number":10,"context_line":"    messages \u003chttps://specs.openstack.org/openstack/keystone-specs/specs/keystone/backlog/pci-dss-invalid-password-reporting.html\u003e`_."}],"source_content_type":"text/x-yaml","patch_set":9,"id":"1a779482_24ce9ae6","line":10,"range":{"start_line":10,"start_character":14,"end_line":10,"end_character":129},"in_reply_to":"36a050dc_f906734c","updated":"2025-03-11 13:28:11.000000000","message":"Updated to https://specs.openstack.org/openstack/keystone-specs/specs/keystone/2025.1/pci-dss-invalid-password-reporting.html","commit_id":"f72b1aaec7fc6d8ac2186e7215ae991be8368b1f"}]}