Yubico / yubico-pam

Yubico Pluggable Authentication Module (PAM)
https://developers.yubico.com/yubico-pam
BSD 2-Clause "Simplified" License
689 stars 114 forks source link

Mysql integration - work with the freeradius sql module #221

Closed baimard closed 3 years ago

baimard commented 3 years ago

Useful Mysql / Mariadb function to work with a database. That replaces the file yubikey_mappings. The mapping is made in the database. That's very useful when you work with freeradius which work with an SQL module.

I hope this will suit you.

Sorry for all the movement ...

baimard commented 3 years ago

I have fix some issues, but I can't get it to compile for Apple système (Mac OS). Don't know why ...

The code is in production in my company

baimard commented 3 years ago

I resolved all conversations and opened new comments in all places where I think there's a change needed.

thanks, I appreciate it. I understand that the code have to been clean !

klali commented 3 years ago

Great. I'm ok to merge this. One final thing. I want you to squash everything to one commit so we skip alot of the comment/refactor noise on master.

I'd recommend doing this with a "git rebase -i master" and marking all commits except the first as squash then finally amending that commit with a good commit message.

edit: and after squashing it you'll have to force-push the branch.

baimard commented 3 years ago

Great. I'm ok to merge this. One final thing. I want you to squash everything to one commit so we skip alot of the comment/refactor noise on master.

I'd recommend doing this with a "git rebase -i master" and marking all commits except the first as squash then finally amending that commit with a good commit message.

edit: and after squashing it you'll have to force-push the branch.

Thank you, I have squash all the commits except the first and I have change the commit message. Let me see if that well for you.

I am very happy with this collaboration !

Thank you again for all reviewing and all your constructive comments.

klali commented 3 years ago

Thanks!

I've merged this, it's notable that the CI should be moved to run on github actions and include tests for this, but lets leave that as an exercise for the future.