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

PAM fails to set the session variables [properly]. #50

Closed
S-NA opened this issue Aug 28, 2017 · 8 comments
Closed

PAM fails to set the session variables [properly]. #50

S-NA opened this issue Aug 28, 2017 · 8 comments

Comments

@S-NA
Copy link

S-NA commented Aug 28, 2017

Session variables written to .pam_environment are sorted alphabetically is the culprit here.

Given:

home.sessionVariableSetter = "pam";
home.sessionVariables = { XDG_CONFIG_HOME="@{HOME}/.config";
                          HOMERC="$\{XDG_CONFIG_HOME\}/htop/htoprc";
                        };
$ cat ~/.pam_environment

HOMERC OVERRIDE=${XDG_CONFIG_HOME}/htop/htoprc
XDG_CONFIG_HOME OVERRIDE=@{HOME}/.config

Which causes PAM to fail, due to XDG_CONFIG_HOME, not being defined before HOMERC .

I believe this is due to home.sessionVariables being a set ({}), instead of a list ([]) which means order is not preserved. (I have not dived deep enough into the Nix language to know if that is the case, just a conjecture on my part.)

@rycee
Copy link
Member

rycee commented Aug 28, 2017

Yes, you are quite right and I believe the NixOS environment.sessionVariables option also has this problem.

For a fancy solution I guess that this could be fixed using a topological sort if we add support for detecting environment variables in the value. So in your example HOMERC would be inserted into the pam file with a dependency on XDG_CONFIG_HOME. I guess this would go in together with the code in #28 (which basically needs such functionality anyway).

The straight forward and boring solution would be to add support for specifying the variables in a list, instead…

@dermetfan dermetfan mentioned this issue Sep 3, 2017
@S-NA
Copy link
Author

S-NA commented Sep 5, 2017

For what it is worth, I tried implementing a solution specific to the PAM module over the weekend, but I hit a lot of edge cases, as well as it depends bash 4 specific things (associative array), is that OK?

If it is, when the sort fails due to a user's variable self-referencing should I throw an assert or silently ignore?

@rycee
Copy link
Member

rycee commented Sep 5, 2017

Cool! I think it is worth making a PR for it. About the assertion it depends on if it can be done during a generation build or it happens during activation. If it during generation build then assert is definitely preferable. In the activation script it is a bit trickier, probably best to drop it silently (ideally would be to print an message, though).

@rycee rycee closed this as completed Sep 5, 2017
@rycee rycee reopened this Sep 5, 2017
@uvNikita
Copy link
Collaborator

One way to solve this issue is to use nix recursive set feature:

home.sessionVariables = rec {
  XDG_CONFIG_HOME = "$HOME/.config";
  HOMERC = "${XDG_CONFIG_HOME}/htop/htoprc";
};

This configuration will produce the correct output:

export HOMERC="$HOME/.config/htop/htoprc"
export XDG_CONFIG_HOME="$HOME/.config"

Not sure if it covers all cases though.

@rycee
Copy link
Member

rycee commented Oct 13, 2017

Yeah, I'm liking this use of rec and I think it would work in 90+% of cases. The main issue I can think of is if the variable you're interpolating contains side-effects. A ridiculous example:

home.sessionVariables = rec {
  TEST1 = "$(cat /sys/class/power_supply/BAT0/status)";
  TEST2 = "${TEST1}";
};

will result in two executions of cat, which may result in $TEST1 != $TEST2. I'm imaging this is rarely used in practice and such usage could perhaps be considered outside the scope of sessionVariables and, if you are a Bash user, instead go into programs.bash.profileExtra.

@S-NA
Copy link
Author

S-NA commented Dec 17, 2017

My apologies to getting to this late.

rec does everything I need, thank you. Is there a reason that home.sessionVariables does not use rec internally by default? If the case is explicit versus implicit perhaps a note about rec in the man page for home-configuration.nix under the home.sessionVariables option?

@rycee
Copy link
Member

rycee commented Dec 17, 2017

Cool. Glad it worked out for you :-)

About the using rec internally, could you explain a bit more what you mean? That the sessionVariables attribute set would somehow act recursively by default? I don't know of any such feature. If it could be done it would be cool :-)

@rycee
Copy link
Member

rycee commented Feb 27, 2018

I'm not really planning to do anything else on this issue so I'll close it. @S-NA If you think I'm missing something then feel free to reopen and comment.

@rycee rycee closed this as completed Feb 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants