)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":21420,"name":"Gage Hugo","email":"gagehugo@gmail.com","username":"ghugo"},"change_message_id":"c0f4374b0088a310552b2b25f21f18044415ddd3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"47d213bb_74b44e5f","updated":"2022-02-15 15:51:56.000000000","message":"If we go this route, we need to update the configuration description here[0] too. This also needs a releasenote since it will impact users.\n\n[0] https://github.com/openstack/keystone/blob/master/keystone/conf/identity.py#L96-L103","commit_id":"6ca1ee95e45f9c9ff3885835bcc2293d11f68aa9"},{"author":{"_account_id":7414,"name":"David Wilde","email":"dwilde@redhat.com","username":"d34dh0r53"},"change_message_id":"542a56ed46509f95cda78e7c0dcdf1bcea9f495a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"c61ab114_fd95f935","in_reply_to":"47d213bb_74b44e5f","updated":"2022-02-21 17:16:23.000000000","message":"Done","commit_id":"6ca1ee95e45f9c9ff3885835bcc2293d11f68aa9"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"a652f39c81a2a9d09cc3face63696d107eb95c3f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"cbf32dcb_f7fb80e3","updated":"2022-03-18 15:16:14.000000000","message":"We could probably add some tests in the utils test class [0]. Otherwise, this looks good.\n\n[0] https://github.com/openstack/keystone/blob/10057702ac361213e74472ec1d0d4e4c4a041f09/keystone/tests/unit/common/test_utils.py#L38","commit_id":"7859ed26003858ebfd9a5e866b43f1a6a9e83dca"},{"author":{"_account_id":7414,"name":"David Wilde","email":"dwilde@redhat.com","username":"d34dh0r53"},"change_message_id":"a659f5161ab4318d8faba43e7ba683d5f14d9ee6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"d60667db_b6b90955","updated":"2023-02-15 05:14:09.000000000","message":"Fixed linting","commit_id":"580d2a32a22ad493cd1006591a3c0e632ade8376"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"e0c53ad42fa94e32c7ddabd4f3c9475265891bd6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"27913932_716fc079","updated":"2023-02-20 16:40:25.000000000","message":"Looking better but the function is a bit verbose.","commit_id":"bc7ff6c5633f3aaa67a74a66cca2cc38255ad9ba"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"65937ce7f3330fe2e095bdd8f70a10b5399f5a45","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"a85e8871_cf7c9d1e","updated":"2023-02-23 13:19:25.000000000","message":"Thanks for updating this.  LGTM now.","commit_id":"3288af579de8ee312c36fb78ac9309ce8c554827"}],"keystone/common/password_hashing.py":[{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"a652f39c81a2a9d09cc3face63696d107eb95c3f","unresolved":true,"context_lines":[{"line_number":81,"context_line":"        algorithm_max_length \u003d None"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"    if algorithm_max_length is not None:"},{"line_number":84,"context_line":"        if algorithm_max_length \u003c conf_max_length:"},{"line_number":85,"context_line":"            max_length \u003d algorithm_max_length"},{"line_number":86,"context_line":"        else:"},{"line_number":87,"context_line":"            max_length \u003d conf_max_length"}],"source_content_type":"text/x-python","patch_set":7,"id":"49bd2f6a_81222c3d","line":84,"updated":"2022-03-18 15:16:14.000000000","message":"We could add a log.info or log.warning here that tells the user that the value they set for max_password_length is being ignored in favor of the algorithm\u0027s defaults.","commit_id":"7859ed26003858ebfd9a5e866b43f1a6a9e83dca"},{"author":{"_account_id":7414,"name":"David Wilde","email":"dwilde@redhat.com","username":"d34dh0r53"},"change_message_id":"a659f5161ab4318d8faba43e7ba683d5f14d9ee6","unresolved":false,"context_lines":[{"line_number":81,"context_line":"        algorithm_max_length \u003d None"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"    if algorithm_max_length is not None:"},{"line_number":84,"context_line":"        if algorithm_max_length \u003c conf_max_length:"},{"line_number":85,"context_line":"            max_length \u003d algorithm_max_length"},{"line_number":86,"context_line":"        else:"},{"line_number":87,"context_line":"            max_length \u003d conf_max_length"}],"source_content_type":"text/x-python","patch_set":7,"id":"7ed8416c_451adb9c","line":84,"in_reply_to":"49bd2f6a_81222c3d","updated":"2023-02-15 05:14:09.000000000","message":"Done","commit_id":"7859ed26003858ebfd9a5e866b43f1a6a9e83dca"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"c3d421880897bd258e3ee2a164728a251edaa48b","unresolved":true,"context_lines":[{"line_number":72,"context_line":"        # passlib.hash.bcrypt security issue."},{"line_number":73,"context_line":"        #  https://passlib.readthedocs.io/en/stable/lib/passlib.hash.bcrypt.html#security-issues"},{"line_number":74,"context_line":"        algorithm_max_length \u003d 54"},{"line_number":75,"context_line":"    elif conf_hasher \u003d\u003d \u0027sha512_crypt\u0027:"},{"line_number":76,"context_line":"        # Per the security issues for sha512_crypt (the same as sha256_crypt)"},{"line_number":77,"context_line":"        # the maximum password size is set to 4k by default."},{"line_number":78,"context_line":"        # https://passlib.readthedocs.io/en/stable/lib/passlib.hash.sha256_crypt.html"},{"line_number":79,"context_line":"        algorithm_max_length \u003d 4096"},{"line_number":80,"context_line":"    else:"},{"line_number":81,"context_line":"        algorithm_max_length \u003d None"},{"line_number":82,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"216765fa_5c3a76ae","line":79,"range":{"start_line":75,"start_character":4,"end_line":79,"end_character":35},"updated":"2023-02-16 20:05:19.000000000","message":"Why is this necessary?  \"sha521_crypt\" is not a valid choice for this configuration option: \n\nhttps://opendev.org/openstack/keystone/src/commit/420f4ff46da106b67912cecdff939f5dc0b079d0/keystone/conf/identity.py#L113","commit_id":"580d2a32a22ad493cd1006591a3c0e632ade8376"},{"author":{"_account_id":7414,"name":"David Wilde","email":"dwilde@redhat.com","username":"d34dh0r53"},"change_message_id":"aeb1afbc269139326d2f7f24d08903def7721fb8","unresolved":false,"context_lines":[{"line_number":72,"context_line":"        # passlib.hash.bcrypt security issue."},{"line_number":73,"context_line":"        #  https://passlib.readthedocs.io/en/stable/lib/passlib.hash.bcrypt.html#security-issues"},{"line_number":74,"context_line":"        algorithm_max_length \u003d 54"},{"line_number":75,"context_line":"    elif conf_hasher \u003d\u003d \u0027sha512_crypt\u0027:"},{"line_number":76,"context_line":"        # Per the security issues for sha512_crypt (the same as sha256_crypt)"},{"line_number":77,"context_line":"        # the maximum password size is set to 4k by default."},{"line_number":78,"context_line":"        # https://passlib.readthedocs.io/en/stable/lib/passlib.hash.sha256_crypt.html"},{"line_number":79,"context_line":"        algorithm_max_length \u003d 4096"},{"line_number":80,"context_line":"    else:"},{"line_number":81,"context_line":"        algorithm_max_length \u003d None"},{"line_number":82,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"5ab6946d_d61aea14","line":79,"range":{"start_line":75,"start_character":4,"end_line":79,"end_character":35},"in_reply_to":"02448e2e_4724dab6","updated":"2023-02-17 14:21:16.000000000","message":"Done","commit_id":"580d2a32a22ad493cd1006591a3c0e632ade8376"},{"author":{"_account_id":7414,"name":"David Wilde","email":"dwilde@redhat.com","username":"d34dh0r53"},"change_message_id":"e54241c1c54360ae6181c3aec23c5e2469407d06","unresolved":true,"context_lines":[{"line_number":72,"context_line":"        # passlib.hash.bcrypt security issue."},{"line_number":73,"context_line":"        #  https://passlib.readthedocs.io/en/stable/lib/passlib.hash.bcrypt.html#security-issues"},{"line_number":74,"context_line":"        algorithm_max_length \u003d 54"},{"line_number":75,"context_line":"    elif conf_hasher \u003d\u003d \u0027sha512_crypt\u0027:"},{"line_number":76,"context_line":"        # Per the security issues for sha512_crypt (the same as sha256_crypt)"},{"line_number":77,"context_line":"        # the maximum password size is set to 4k by default."},{"line_number":78,"context_line":"        # https://passlib.readthedocs.io/en/stable/lib/passlib.hash.sha256_crypt.html"},{"line_number":79,"context_line":"        algorithm_max_length \u003d 4096"},{"line_number":80,"context_line":"    else:"},{"line_number":81,"context_line":"        algorithm_max_length \u003d None"},{"line_number":82,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"02448e2e_4724dab6","line":79,"range":{"start_line":75,"start_character":4,"end_line":79,"end_character":35},"in_reply_to":"216765fa_5c3a76ae","updated":"2023-02-17 13:41:21.000000000","message":"Good catch, it looks like sha512_crypt was removed as of stein and replaced with pbkdf2_sha512.  Looking to see if pbkdf2_sha512 has similar length limitations.","commit_id":"580d2a32a22ad493cd1006591a3c0e632ade8376"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"e0c53ad42fa94e32c7ddabd4f3c9475265891bd6","unresolved":true,"context_lines":[{"line_number":80,"context_line":"            msg \u003d \"Truncating password to algorithm specific maximum length %d characters.\""},{"line_number":81,"context_line":"            LOG.warning(msg, algorithm_max_length)"},{"line_number":82,"context_line":"            max_length \u003d algorithm_max_length"},{"line_number":83,"context_line":"        else:"},{"line_number":84,"context_line":"            max_length \u003d conf_max_length"},{"line_number":85,"context_line":"    else:"},{"line_number":86,"context_line":"        max_length \u003d conf_max_length"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"    try:"},{"line_number":89,"context_line":"        if len(password) \u003e max_length:"}],"source_content_type":"text/x-python","patch_set":11,"id":"93460468_0ac56b23","line":86,"range":{"start_line":83,"start_character":8,"end_line":86,"end_character":36},"updated":"2023-02-20 16:40:25.000000000","message":"we can avoid the duplication here by just setting\n\n    max_length \u003d conf_max_length\n\nbefore the `if` statement.  Or if we want to simplify this even further we can get rid of the `conf_max_length` temp variable and just set\n\n    max_length \u003d CONF.identity.max_password_length\n    \nat the top of the function and then do the checks for algorithm.","commit_id":"bc7ff6c5633f3aaa67a74a66cca2cc38255ad9ba"},{"author":{"_account_id":7414,"name":"David Wilde","email":"dwilde@redhat.com","username":"d34dh0r53"},"change_message_id":"b8466d367118411f8f6c250ef4ddd2b885743b31","unresolved":false,"context_lines":[{"line_number":80,"context_line":"            msg \u003d \"Truncating password to algorithm specific maximum length %d characters.\""},{"line_number":81,"context_line":"            LOG.warning(msg, algorithm_max_length)"},{"line_number":82,"context_line":"            max_length \u003d algorithm_max_length"},{"line_number":83,"context_line":"        else:"},{"line_number":84,"context_line":"            max_length \u003d conf_max_length"},{"line_number":85,"context_line":"    else:"},{"line_number":86,"context_line":"        max_length \u003d conf_max_length"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"    try:"},{"line_number":89,"context_line":"        if len(password) \u003e max_length:"}],"source_content_type":"text/x-python","patch_set":11,"id":"edc4e8cf_f694709a","line":86,"range":{"start_line":83,"start_character":8,"end_line":86,"end_character":36},"in_reply_to":"93460468_0ac56b23","updated":"2023-02-20 16:47:07.000000000","message":"Done","commit_id":"bc7ff6c5633f3aaa67a74a66cca2cc38255ad9ba"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"f4477d26052c1f5a9e511775fe95380b4ef92d0b","unresolved":true,"context_lines":[{"line_number":78,"context_line":"            msg \u003d \"Truncating password to algorithm specific maximum length %d characters.\""},{"line_number":79,"context_line":"            LOG.warning(msg, algorithm_max_length)"},{"line_number":80,"context_line":"            max_length \u003d algorithm_max_length"},{"line_number":81,"context_line":"        else:"},{"line_number":82,"context_line":"            max_length \u003d CONF.identity.max_password_length"},{"line_number":83,"context_line":"    else:"},{"line_number":84,"context_line":"        max_length \u003d CONF.identity.max_password_length"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    try:"},{"line_number":87,"context_line":"        if len(password) \u003e max_length:"}],"source_content_type":"text/x-python","patch_set":14,"id":"f6e74450_406fe23d","line":84,"range":{"start_line":81,"start_character":8,"end_line":84,"end_character":54},"updated":"2023-02-21 21:37:19.000000000","message":"I still think having the same exact assignment copy+pasted in two lines is ugly.  IMO this all can be re-written using a single if statement:\n\n    # When using bcrypt, we limit the password length to 54 to ensure all\n    # bytes are fully mixed.  See:\n    # https://passlib.readthedocs.io/en/stable/lib/passlib.hash.bcrypt.html#security-issues\n    BCRYPT_MAX_LENGTH \u003d 54\n    if (CONF.identity.password_hash_algorithm \u003d\u003d \u0027bcrypt\u0027 and \n        CONF.identity.max_password_length \u003e BCRYPT_MAX_LENGTH):\n        LOG.warning(\"Truncating password to bcrypt maximum length of 54 characters\")\n        max_length \u003d BCRYPT_MAX_LENGTH\n    else:\n        max_legnth \u003d CONF.identity.max_password_length\n        \nIt\u0027s still the same logic, but it doesn\u0027t rely on None-testing a temp variable, or funky nested ifs that need the same assignment in the inner and outer tests.","commit_id":"6ceb130b112ab87a6a2f2a00cd2ca4ae3a30f59b"},{"author":{"_account_id":7414,"name":"David Wilde","email":"dwilde@redhat.com","username":"d34dh0r53"},"change_message_id":"7449f59e99d042c7ecbc6dfd2cb18b7e7a500502","unresolved":false,"context_lines":[{"line_number":78,"context_line":"            msg \u003d \"Truncating password to algorithm specific maximum length %d characters.\""},{"line_number":79,"context_line":"            LOG.warning(msg, algorithm_max_length)"},{"line_number":80,"context_line":"            max_length \u003d algorithm_max_length"},{"line_number":81,"context_line":"        else:"},{"line_number":82,"context_line":"            max_length \u003d CONF.identity.max_password_length"},{"line_number":83,"context_line":"    else:"},{"line_number":84,"context_line":"        max_length \u003d CONF.identity.max_password_length"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    try:"},{"line_number":87,"context_line":"        if len(password) \u003e max_length:"}],"source_content_type":"text/x-python","patch_set":14,"id":"5a457853_f049cbe1","line":84,"range":{"start_line":81,"start_character":8,"end_line":84,"end_character":54},"in_reply_to":"7b709c7b_00d8cd77","updated":"2023-02-22 20:44:02.000000000","message":"Done","commit_id":"6ceb130b112ab87a6a2f2a00cd2ca4ae3a30f59b"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"e0c13a5d715b6b8a8f62502007ee86f3f134a04a","unresolved":true,"context_lines":[{"line_number":78,"context_line":"            msg \u003d \"Truncating password to algorithm specific maximum length %d characters.\""},{"line_number":79,"context_line":"            LOG.warning(msg, algorithm_max_length)"},{"line_number":80,"context_line":"            max_length \u003d algorithm_max_length"},{"line_number":81,"context_line":"        else:"},{"line_number":82,"context_line":"            max_length \u003d CONF.identity.max_password_length"},{"line_number":83,"context_line":"    else:"},{"line_number":84,"context_line":"        max_length \u003d CONF.identity.max_password_length"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    try:"},{"line_number":87,"context_line":"        if len(password) \u003e max_length:"}],"source_content_type":"text/x-python","patch_set":14,"id":"ebfd4c32_8fb38084","line":84,"range":{"start_line":81,"start_character":8,"end_line":84,"end_character":54},"in_reply_to":"7b709c7b_00d8cd77","updated":"2023-02-22 18:30:09.000000000","message":"I prefer to merge simpler code now and then deal with changes if/when they come up.","commit_id":"6ceb130b112ab87a6a2f2a00cd2ca4ae3a30f59b"},{"author":{"_account_id":7414,"name":"David Wilde","email":"dwilde@redhat.com","username":"d34dh0r53"},"change_message_id":"075b0382c6b45c22ad12d59f7123ba3883ad1806","unresolved":true,"context_lines":[{"line_number":78,"context_line":"            msg \u003d \"Truncating password to algorithm specific maximum length %d characters.\""},{"line_number":79,"context_line":"            LOG.warning(msg, algorithm_max_length)"},{"line_number":80,"context_line":"            max_length \u003d algorithm_max_length"},{"line_number":81,"context_line":"        else:"},{"line_number":82,"context_line":"            max_length \u003d CONF.identity.max_password_length"},{"line_number":83,"context_line":"    else:"},{"line_number":84,"context_line":"        max_length \u003d CONF.identity.max_password_length"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    try:"},{"line_number":87,"context_line":"        if len(password) \u003e max_length:"}],"source_content_type":"text/x-python","patch_set":14,"id":"7b709c7b_00d8cd77","line":84,"range":{"start_line":81,"start_character":8,"end_line":84,"end_character":54},"in_reply_to":"f6e74450_406fe23d","updated":"2023-02-22 12:59:15.000000000","message":"I see what you\u0027re saying but that code will be harder to maintain if we need to add another algorithm that has a maximum length. Right now we only have bcrypt but that wasn\u0027t always the case.\n\nThat being said I\u0027m willing to change it since it is cleaner for the single case.","commit_id":"6ceb130b112ab87a6a2f2a00cd2ca4ae3a30f59b"}],"keystone/conf/identity.py":[{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"a652f39c81a2a9d09cc3face63696d107eb95c3f","unresolved":true,"context_lines":[{"line_number":100,"context_line":"    help\u003dutils.fmt(\"\"\""},{"line_number":101,"context_line":"Maximum allowed length for user passwords. Decrease this value to improve"},{"line_number":102,"context_line":"performance. Changing this value does not effect existing passwords. This value"},{"line_number":103,"context_line":"can also be overridden by certain hashing algorithms maximum allowed length."},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"The bcrypt max_password_length is 54."},{"line_number":106,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"05b3dfa5_8c695dd9","line":103,"updated":"2022-03-18 15:16:14.000000000","message":"The algorithm maximum password length with take precedence over what a user provides via configuration. Maybe we can add that here?","commit_id":"7859ed26003858ebfd9a5e866b43f1a6a9e83dca"},{"author":{"_account_id":7414,"name":"David Wilde","email":"dwilde@redhat.com","username":"d34dh0r53"},"change_message_id":"a659f5161ab4318d8faba43e7ba683d5f14d9ee6","unresolved":false,"context_lines":[{"line_number":100,"context_line":"    help\u003dutils.fmt(\"\"\""},{"line_number":101,"context_line":"Maximum allowed length for user passwords. Decrease this value to improve"},{"line_number":102,"context_line":"performance. Changing this value does not effect existing passwords. This value"},{"line_number":103,"context_line":"can also be overridden by certain hashing algorithms maximum allowed length."},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"The bcrypt max_password_length is 54."},{"line_number":106,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"f49aff55_eefd2ad1","line":103,"in_reply_to":"05b3dfa5_8c695dd9","updated":"2023-02-15 05:14:09.000000000","message":"Done","commit_id":"7859ed26003858ebfd9a5e866b43f1a6a9e83dca"}]}
