Skip to content

[DebOps] Add new "ldap_password" lookup plugin - #2205

Open
Alphix wants to merge 1 commit into
debops:masterfrom
Alphix:ldap_new_lookup_plugin
Open

Alphix wants to merge 1 commit into
debops:masterfrom
Alphix:ldap_new_lookup_plugin

Conversation

@Alphix

@Alphix Alphix commented Oct 4, 2022

Copy link
Copy Markdown
Contributor

On my devel machine, I have a couple of hundred files under ansible/secret/ldap/credentials/, with no easy way to determine which are still relevant and which aren't.

This patch adds a new lookup plugin named "ldap_password", which works in much the same way as the traditional "password" plugin, but adapted to match the LDAP password storage scheme used by DebOps.

Instead of a single argument (path and options as one string), the new lookup takes three arguments: base directory, DN, and password options.

The plugin will automatically log the UUID <-> DN mapping in a file in the base directory as part of the password generation process.

@Alphix

Alphix commented Oct 4, 2022

Copy link
Copy Markdown
Contributor Author

Note that there's no documentation yet and the actual plugin is very lightly tested. I wanted some feedback from @drybjed first to know if the general idea is acceptable before I sink more time into this....

@Alphix
Alphix force-pushed the ldap_new_lookup_plugin branch 2 times, most recently from 9bfb8b8 to 676e0b2 Compare November 9, 2022 06:48

@drybjed drybjed left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea looks sound. I would need to spin an LDAP environment to test it, but I trust that you use it in some capacity already and it works as expected.

At least some documentation would improve things. Can you add a comment that explains where the value of UUID_NAMESPACE_ANSIBLE comes from? IIRC, it's from the Ansible to_uuid filter which uses a static string as a random seed to ensure idempotency.

I can merge it now or wait for some documentation. Let me know which one you prefer.

@Alphix

Alphix commented Apr 3, 2023

Copy link
Copy Markdown
Contributor Author

The idea looks sound. I would need to spin an LDAP environment to test it, but I trust that you use it in some capacity already and it works as expected.

Yeah, I've use it both in devel and production for a couple of months.

At least some documentation would improve things. Can you add a comment that explains where the value of UUID_NAMESPACE_ANSIBLE comes from? IIRC, it's from the Ansible to_uuid filter which uses a static string as a random seed to ensure idempotency.

Correct

I can merge it now or wait for some documentation. Let me know which one you prefer.

I think it can wait a bit, that way I can add the docs and also go through the current codebase and see if there's any new tasks which need to be updated to use the plugin...

@Alphix
Alphix force-pushed the ldap_new_lookup_plugin branch from 676e0b2 to 216a27f Compare May 7, 2023 13:54
@Alphix
Alphix requested a review from tasosalvas as a code owner May 7, 2023 13:54
@Alphix
Alphix force-pushed the ldap_new_lookup_plugin branch from 216a27f to 343874d Compare May 7, 2023 14:01
@Alphix

Alphix commented May 7, 2023

Copy link
Copy Markdown
Contributor Author

I can merge it now or wait for some documentation. Let me know which one you prefer.

I think it can wait a bit, that way I can add the docs and also go through the current codebase and see if there's any new tasks which need to be updated to use the plugin...

Ok, I've added the docs, gone through the current codebase, and run the end result through a test run...all seems ok to me...I'd say this is ready to merge now @drybjed

@jankowa

jankowa commented May 31, 2023

Copy link
Copy Markdown
Contributor

Hi, I just came across this issue when I wanted to make another proposal: instead of using ansible to_uuid why not use b64encode, because this would be reverse readable.

But the approach of @Alphix is even better I think. How is the directory structured? And how is the password file named? Still with uuid? In this case you could combine the approaches and use b64encode. How is backward compatibility planned?

@Alphix

Alphix commented May 31, 2023

Copy link
Copy Markdown
Contributor Author

Hi, I just came across this issue when I wanted to make another proposal: instead of using ansible to_uuid why not use b64encode, because this would be reverse readable.

But the approach of @Alphix is even better I think. How is the directory structured? And how is the password file named? Still with uuid? In this case you could combine the approaches and use b64encode. How is backward compatibility planned?

With my approach there's no difference in directory structure....uuid encoded files are still generated under secret/ldap/credentials/, just like before. It's just that another file will be present at secret/ldap/credentials/debops_ldap_uuid.log and that file will contain lines like:

cac40123-e4d1-5757-9d82-b00ca9e9e994 uid=sudo,cn=qtest1.example.com,ou=Hosts,dc=example,dc=com
f550564f-5f8b-5bba-af58-da807cde61b6 uid=sshd,cn=qtest1.example.com,ou=Hosts,dc=example,dc=com
...

So backward compatibility is not a problem....and old entries will still be appended to debops_ldap_uuid.log the next time the relevant role is executed...

@jankowa

jankowa commented Jun 1, 2023

Copy link
Copy Markdown
Contributor

So backward compatibility is not a problem....and old entries will still be appended to debops_ldap_uuid.log the next time the relevant role is executed...

ok. Very nice.

On my devel machine, I have a couple of hundred files under
ansible/secret/ldap/credentials/, with no easy way to determine
which are still relevant and which aren't.

This patch adds a new lookup plugin named "ldap_password", which
works in much the same way as the traditional "password" plugin, but
adapted to match the LDAP password storage scheme used by DebOps.

Instead of a single argument (path and options as one string), the new
lookup takes three arguments: base directory, DN, and password options.

The plugin will automatically log the UUID <-> DN mapping in a file in
the base directory as part of the password generation process.
@Alphix
Alphix force-pushed the ldap_new_lookup_plugin branch from 343874d to e5b8b88 Compare October 15, 2023 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants