Fix: common-utils configureZshAsDefaultShell option not working on alpine linux image

[krikchaip]

Hi, I was trying to set up a devcontainer for my side-project using the base alpine image with common-utils feature. I knew that the image already included the feature, but I also wanted to set the default shell to zsh. According to the documentation, I had to pass "configureZshAsDefaultShell": "true", but it didn't seem to be working for me.

While reading error logs, I found that the problem came from chsh command, which was always asking for a password.

https://github.com/devcontainers/features/blob/main/src/common-utils/main.sh#LL439C9-L441C7

if [ "${CONFIGURE_ZSH_AS_DEFAULT_SHELL}" == "true" ]; then
    chsh --shell /bin/zsh ${USERNAME}

    # PASSWORD:
    # error -> chsh: PAM: Authentication failure
fi

After doing some research, I found the solution. Basically, You need to modify the content in some file as shown on my PR.

/etc/pam.d/chsh

auth sufficient pam_shells.so

[krikchaip]

@microsoft-github-policy-service agree

[krikchaip]

The license/cla status has been queued for over a day. How can I make it green? 🤔

[samruddhikhandale]

Thanks for the PR, left some thoughts!

The license/cla status has been queued for over a day. How can I make it green? 🤔

Mmm, I expect that it should pass now as the CLA is signed. I don't see an option to retry the check, should have been automatic.

Looking at microsoft/ContributorLicenseAgreement#123 , probably closing & reopening the PR or pushing a new commit helps.

[krikchaip]

I found something interesting inside /etc/pam.d/chsh of the devcontainer of this repo.

Seems like we should add auth sufficient pam_rootok.so instead of pam_shells.so 🤔 .

[krikchaip]

@samruddhikhandale hmmm. strange. I've also tried the command with /etc/pam.d/chsh that has some content inside similar to you. It seemed to work pretty well AFAIK. 🤔

720p.mov

[samruddhikhandale]

@krikchaip In your case, you are running the conditions from the terminal, however, I tested it with a Feature test scenario (which resembles an actual Feature installation). Also, the install script is run with user:root, and in your case, it's node. I believe these are the reasons why we both are seeing different results.

[krikchaip]

in your case, you are running the conditions from the terminal, however, I tested it with a Feature test scenario (which resembles an actual Feature installation). Also, the install script is run with user:root, and in your case, it's node. I believe these are the reasons why we both are seeing different results.

@samruddhikhandale Did you mean the configure_zsh_as_default_shell scenario? If so, do you have any suggestion on how can I provide my own /etc/pam.d/chsh file? I basically want to replicate the same scenario as yours

[samruddhikhandale]

@krikchaip Add echo statements to help debug the file changes to 👇

features/src/common-utils/main.sh

Lines 439 to 446 in 0ae570c

	if [ "${CONFIGURE_ZSH_AS_DEFAULT_SHELL}" == "true" ]; then
	# Fixing chsh always asking for a password on alpine linux
	# ref: https://askubuntu.com/questions/812420/chsh-always-asking-a-password-and-get-pam-authentication-failure.
	if [ -f "/etc/pam.d/chsh" ] && grep -vEq "^auth[[:blank:]]+sufficient[[:blank:]]+pam_rootok\.so$"; then
	awk '/^auth(.*)pam_rootok\.so$/ { $2 = "sufficient" } { print }' /etc/pam.d/chsh > /tmp/chsh.tmp && mv /tmp/chsh.tmp /etc/pam.d/chsh
	else
	echo "auth sufficient pam_rootok.so" >> /etc/pam.d/chsh
	fi

and run BUILDKIT_PROGRESS=plain devcontainer features test -f common-utils --filter configure_zsh_as_default_shell --skip-autogenerated

[krikchaip]

@samruddhikhandale Opsie.. my bad XD. Accidentally forgot that grep requires 2 parameters...

Please check again. It should be working rn.

[samruddhikhandale]

Can add another condition so that we could avoid running this command when /etc/pam.d/chsh already contains auth sufficient pam_rootok.so ?

Following up on #557 (comment)

@krikchaip Can we update the logic to NOT make any changes to the file if auth sufficient pam_rootok.so exists? I don't think we should touch/make changes to a file if that's not needed. Saw this with configure_zsh_as_default_shell scenario, the diff is easily visible with respect to spaces

[krikchaip]

@samruddhikhandale I believe the logic here seems to prevent it from touching the /etc/pam.d/chsh file, but let me check again some time soon.

if [ -f "/etc/pam.d/chsh" ] &&
  grep -vEq "^auth[[:blank:]]+sufficient[[:blank:]]+pam_rootok\.so$" /etc/pam.d/chsh;

[krikchaip]

@samruddhikhandale please recheck

[samruddhikhandale]

Looks great, thank you so much for taking the time to contribute this PR and reiterating on feedbacks. We appreciate it! ❇️