New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add pam option to enable Google Authenticator #34728
Conversation
nixos/modules/security/pam.nix
Outdated
@@ -46,6 +46,16 @@ let | |||
''; | |||
}; | |||
|
|||
google2FA = mkOption { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Would be googleAuthenticator
be more self-explanatory then google2FA
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Considering that there are other config settings that we might want to support later, it might make sense to "namespace" the config option like so:
google2FA = {
enable = mkOption {
default = false;
type = types.bool;
description = ''
If set, users with enabled Google Authenticator (created
<filename>~/.google_authenticator</filename>) will be required
to provide Google Authenticator token to log in.
'';
};
};
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The additional namespace is a very good idea.
authenticator
seemed like a longish word. I have very little idea about conventions here. I am happy to change it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me it was more about being consistent with what upstream calls it.
I have amended the change according to your instructions. |
Thanks! |
Motivation for this change
I couldn't find this option anywhere, and I used it on my Fedora, that I've replaced with NixOS (yay!), so I want it here as well.
Things done
(Not much yet, I am a total noob, please be gentle).
build-use-sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)