Closed sokomishalov closed 2 years ago
Thanks for opening an PR. Overall code is ok, but I've questing about auth design: Why username and password are sent to agent? For AppRole authentication, Response Wrapping is used, so credentials are stored and used only on the server side. Seems the same should be done for LDAP as well. For IAM that's not the case because there are no explicit credentials at all.
@VladRassokhin thanks for the quick reply and review! Good point, I've reworked it according to your recommendations. My main concern about such design is that IMO server is overwhelmed with I/O operations by requesting to the vault too much which is more appropriate work for the agent. However, passing plain user/pass to the agent is not a very good solution either. My secondary concern is that maybe we should rework UI a bit and replace radio buttons with dropdown due to the growing amount of auth methods (4 for now). Also, I suppose in this case docs should be updated and demo configuration screen too.
@VladRassokhin nope, userpass auth isn't needed at all, but its implementation and flow are so similar to LDAP so I thought it might be useful. But ok, I've removed it and made all changes according to your review.
@VladRassokhin Good afternoon, when are you planning to merge the change?
@VladRassokhin could you review and merge please?
@VladRassokhin just a gentle ping
Thank you for the contribution. I've simplified it, polished, fixed one bug (around path
for initial POST) and manually merged.
@VladRassokhin Great! Thank you!
Implements #45