Skip to content
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

nixos/pam: refactor U2F, docs about u2f_keys path #54756

Merged
merged 1 commit into from Jan 29, 2019

Conversation

Projects
None yet
4 participants
@kalbasit
Copy link
Member

commented Jan 28, 2019

Motivation for this change
  • change enableU2F option to u2f.* set
  • add few u2f options (not all) to customize pam-u2f module
  • document default u2f_keys locations
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Replaces #40455 and #11886.

@kalbasit kalbasit requested a review from Infinisil as a code owner Jan 28, 2019

@kalbasit kalbasit force-pushed the kalbasit:nixpkgs_improve-u2f-support branch from 426c73c to 109c1f7 Jan 28, 2019

kalbasit added a commit to kalbasit/shabka that referenced this pull request Jan 28, 2019

@kalbasit

This comment has been minimized.

Copy link
Member Author

commented Jan 28, 2019

@GrahamcOfBorg test pam-u2f

@kalbasit kalbasit force-pushed the kalbasit:nixpkgs_improve-u2f-support branch from 109c1f7 to b76d168 Jan 28, 2019

@kalbasit

This comment has been minimized.

Copy link
Member Author

commented Jan 28, 2019

@GrahamcOfBorg test pam-u2f

kalbasit added a commit to kalbasit/shabka that referenced this pull request Jan 28, 2019

nixos: apply U2F improvement patch and enable u2f.cue (#121)
This change patches nixpkgs stable with NixOS/nixpkgs#54756 and enables `u2f.cue`.
@kalbasit

This comment has been minimized.

Copy link
Member Author

commented Jan 28, 2019

@spinus this PR is heavily based on your submission #11886. Please let me know if there's anything else you would like to add.
@Infinisil I've been using this PR for a few days now, and I have not had any issues, Do you have some time for a review? I will wait at least another week before merging to get a chance for more testing.

Show resolved Hide resolved nixos/modules/security/pam.nix Outdated
Show resolved Hide resolved nixos/modules/security/pam.nix Outdated
Show resolved Hide resolved nixos/modules/security/pam.nix Outdated
Show resolved Hide resolved nixos/modules/security/pam.nix Outdated

@kalbasit kalbasit force-pushed the kalbasit:nixpkgs_improve-u2f-support branch from b76d168 to 20d702a Jan 28, 2019

@kalbasit

This comment has been minimized.

Copy link
Member Author

commented Jan 28, 2019

@Infinisil I've updated the PR and addressed all of your comments, PTAL.

@kalbasit kalbasit force-pushed the kalbasit:nixpkgs_improve-u2f-support branch from 20d702a to bbb2475 Jan 28, 2019

Show resolved Hide resolved nixos/modules/security/pam.nix Outdated
Show resolved Hide resolved nixos/modules/security/pam.nix Outdated
Show resolved Hide resolved nixos/modules/security/pam.nix Outdated

@kalbasit kalbasit force-pushed the kalbasit:nixpkgs_improve-u2f-support branch from bbb2475 to 396afe2 Jan 29, 2019

@kalbasit

This comment has been minimized.

Copy link
Member Author

commented Jan 29, 2019

@Infinisil PTAL.

@Infinisil

This comment has been minimized.

Copy link
Member

commented Jan 29, 2019

Commit message should be changed to have the nixos/...: prefix. Feel free to merge this after that :)

nixos/pam: refactor U2F, docs about u2f_keys path
* change enableU2F option to u2f.* set
* add few u2f options (not all) to customize pam-u2f module
* document default u2f_keys locations

Co-authored-by: Tomasz Czyż <tomasz.czyz@gmail.com>
Co-authored-by: Arda Xi <arda@ardaxi.com>

@kalbasit kalbasit force-pushed the kalbasit:nixpkgs_improve-u2f-support branch from 396afe2 to 22625f8 Jan 29, 2019

@kalbasit kalbasit changed the title pam-u2f: refactor, docs about u2f_keys path nixos/pam: refactor U2F, docs about u2f_keys path Jan 29, 2019

@kalbasit kalbasit merged commit f072cfe into NixOS:master Jan 29, 2019

10 checks passed

grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details

@kalbasit kalbasit deleted the kalbasit:nixpkgs_improve-u2f-support branch Jan 29, 2019

@spinus

This comment has been minimized.

Copy link
Contributor

commented Jan 30, 2019

@kalbasit thank you for pushing that forward!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.