[#36] SSHKit.user errors because sudo is missing #51
Conversation
|
|
||
| [{:ok, output, status}] = SSHKit.run(context, "does-not-exist") | ||
| assert status == 127 | ||
| output = stderr(output) | ||
| assert output =~ "'does-not-exist': No such file or directory" | ||
| assert stderr(output) =~ "'does-not-exist': No such file or directory" |
test/sshkit_functional_test.exs
Outdated
| @tag boot: 1 | ||
| test "user", %{hosts: [host]} do | ||
| adduser(host, "despicable_me") | ||
| add_user_to_group(host, host.user, "danger") |
There was a problem hiding this comment.
why does the user need to be in a group?
There was a problem hiding this comment.
The group is set up in the Dockerfile in order to allow passwordless sudoing, required for user switching. Hence also the name "danger".
Maybe you've got a different idea though?
There was a problem hiding this comment.
hmm, nope. maybe renaming the group from danger to something more describing would help?
sudo_enabled or something?
test/support/functional_case.ex
Outdated
| @@ -46,6 +46,7 @@ defmodule SSHKit.FunctionalCase do | |||
|
|
|||
| def init(host) do | |||
| adduser(host, @user) | |||
| add_user_to_group(host, @user, "wheel") | |||
There was a problem hiding this comment.
here again: why can't the user live without a group by default?
There was a problem hiding this comment.
True, this can be removed.
- Install "sudo" package for the Alpine Docker image - Allow users of system group "wheel" to execute any command - Add "danger" group whose users can run sudo without password
- Add test user to the "danger" group to allow running sudo - Use `id -un` over obsoleted `whoami` - Check exit codes in `SSHKit.umask/2` test file creation
- Use sudo for both switching user and group in a single command - Quick version of `SSHKit.Utils.shellquote/1`, improved one follows
pmeinhardt
force-pushed
the
feature/36-sshkit-user-errors-because-sudo-is-missing
branch
from
89e4a57 to
e11da3f
Compare
| defp sudo(command, nil, nil), do: command | ||
| defp sudo(command, username, nil), do: "sudo -n -u #{username} -- sh -c #{shellquote(command)}" | ||
| defp sudo(command, nil, groupname), do: "sudo -n -g #{groupname} -- sh -c #{shellquote(command)}" | ||
| defp sudo(command, username, groupname), do: "sudo -n -u #{username} -g #{groupname} -- sh -c #{shellquote(command)}" |
There was a problem hiding this comment.
only this match of sudo/3 is ever used, right?
There was a problem hiding this comment.
Errr, nope. Unless I missed something, the context may hold any combination of :user and :group being present or not present.
Example:
context =
hosts
|> SSHKit.context()
|> SSHKit.user("tessi")
|> SSHKit.path("/var/log")
SSHKit.Context.build(context, "ls")Here, context has a user, but no group set.
There was a problem hiding this comment.
ah, you're right. i just misread the code yesterday evening. let's act as if i never wrote this comment ;)
lib/sshkit/utils.ex
Outdated
| @@ -3,7 +3,7 @@ defmodule SSHKit.Utils do | |||
|
|
|||
| def shellescape(value), do: value | |||
|
|
|||
| def shellquote(value), do: value | |||
| def shellquote(value), do: "'#{value}'" # TODO: Proper quoting | |||
There was a problem hiding this comment.
what could possibly go wrong 
There was a problem hiding this comment.
I'm glad you included the .
That's exactly the reason why I included the "TODO", to make the Credo checks fail until I've added proper quoting on this branch 😁
| @@ -9,7 +9,7 @@ defmodule SSHKit.SSHFunctionalTest do | |||
| test "opens a connection with username and password", %{hosts: [host]} do | |||
| options = [port: host.port, user: host.user, password: host.password] | |||
| {:ok, conn} = SSH.connect(host.ip, Keyword.merge(@defaults, options)) | |||
| {:ok, data, status} = SSH.run(conn, "whoami") | |||
| {:ok, data, status} = SSH.run(conn, "id -un") | |||
|
Thanks a lot for your feedback @tessi, I've updated the PR description with a checklist for myself to address the remaining issues. |
|
Thanks! The shellquote might even be done in a separate PR. The only thing left IMO for this PR would be a better name for the sudo-group |
pmeinhardt
deleted the
feature/36-sshkit-user-errors-because-sudo-is-missing
branch
Update the functional tests to allow the test/login user to run
sudoin order to switch user/group.TODOs:
Implement=> Implement shellescaping/-quoting #54SSHKit.Utils.shellquote/1, andshellescape/1