[GitHub] activemq-artemis issue #1771: ARTEMIS-1600 Support masked passwords in boots...
Github user gaohoward commented on the issue: https://github.com/apache/activemq-artemis/pull/1771 OK it's done. ---
[GitHub] activemq-artemis issue #1771: ARTEMIS-1600 Support masked passwords in boots...
Github user gaohoward commented on the issue: https://github.com/apache/activemq-artemis/pull/1771 @jbertram Please hold for a moment. I just found a change that may be wrong. ---
[GitHub] activemq-artemis issue #1771: ARTEMIS-1600 Support masked passwords in boots...
Github user gaohoward commented on the issue: https://github.com/apache/activemq-artemis/pull/1771 @jbertram @clebertsuconic done! ---
[GitHub] activemq-artemis issue #1771: ARTEMIS-1600 Support masked passwords in boots...
Github user gaohoward commented on the issue: https://github.com/apache/activemq-artemis/pull/1771 well, almost done. Just found a unused var. I'll delete it right away. Sorry about that. :) ---
[GitHub] activemq-artemis issue #1771: ARTEMIS-1600 Support masked passwords in boots...
Github user gaohoward commented on the issue: https://github.com/apache/activemq-artemis/pull/1771 @jbertram Hi Justin, I think it's done. Can you take a look again? Thanks ---
[GitHub] activemq-artemis issue #1771: ARTEMIS-1600 Support masked passwords in boots...
Github user gaohoward commented on the issue: https://github.com/apache/activemq-artemis/pull/1771 that's fine. I'll remove it. ---
[GitHub] activemq-artemis issue #1771: ARTEMIS-1600 Support masked passwords in boots...
Github user jbertram commented on the issue: https://github.com/apache/activemq-artemis/pull/1771 In my opinion, you should remove the mask-password config property from this PR as it will require more code/documentation changes later when it's deprecated and eventually removed. ---
[GitHub] activemq-artemis issue #1771: ARTEMIS-1600 Support masked passwords in boots...
Github user gaohoward commented on the issue: https://github.com/apache/activemq-artemis/pull/1771 OK, so I'd add ENC() syntax and keeps the mask-password as an option. (I mean I won't remove this from this PR, but won't add any more in the future if there is new password mask requirements). ---
[GitHub] activemq-artemis issue #1771: ARTEMIS-1600 Support masked passwords in boots...
Github user jbertram commented on the issue: https://github.com/apache/activemq-artemis/pull/1771 We should keep mask-password config support where it exists already, but don't add any new features that use it. Instead we can rely on the ENC() syntax. ---
[GitHub] activemq-artemis issue #1771: ARTEMIS-1600 Support masked passwords in boots...
Github user gaohoward commented on the issue: https://github.com/apache/activemq-artemis/pull/1771 @jbertram ok I think we can support ENC() as well as "mask-password". ---
[GitHub] activemq-artemis issue #1771: ARTEMIS-1600 Support masked passwords in boots...
Github user gaohoward commented on the issue: https://github.com/apache/activemq-artemis/pull/1771 @jbertram good point. I think I can do that. Do you think we still need to support "mask-password" for backward compatibility? ---
[GitHub] activemq-artemis issue #1771: ARTEMIS-1600 Support masked passwords in boots...
Github user jbertram commented on the issue: https://github.com/apache/activemq-artemis/pull/1771 Aside from the failing tests this looks OK. That said, I would love to see us move all our password masking to use the "ENC()" syntax instead of using boolean "mask-password" attributes everywhere if possible. This is done for the properties login module and it's very clean. If the text follows the pattern then it should be treated as masked otherwise it shouldn't. ---