Skip to content

(SIMP-10739) Fix auth issues #91

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.

Sign up for GitHub

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

Open
greatflyingsteve wants to merge 2 commits into
base: master
Choose a base branch
from
Open

(SIMP-10739) Fix auth issues #91

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
74b1113
(SIMP-10739) Fix various section auth issues
greatflyingsteve Aug 9, 2022
b5b4b43
(SIMP-10739) Update documentation and acc. tests
greatflyingsteve Aug 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 52 additions & 32 deletions manifests/server/section.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,15 @@
# The directory to make available to clients
#
# @param auth_users
# A list of usernames that are allowed to connect to this section
#
# * ``simplib::passgen()`` will be used to generated random passwords for
# This should be either:
# * A list of usernames that are allowed to connect to this section.
# ``simplib::passgen()`` will be used to generated random passwords for
# these users, if they do not already exist in the system
# * Ignored if ``user_pass`` is set.
# * A hash of usernames to their passwords. If the password is `undef` or
# an empty string, ``simplib::passgen()`` will be used to generate or
# retrieve it.
# In either case, the contents of the auth_users parameter will be ignored
# if ``user_pass`` is set.
#
# @param user_pass
# An optional array of ``username:password`` combinations to be added to the
Expand Down Expand Up @@ -50,14 +54,14 @@
# List this share when clients ask for a list of available modules
#
# @param uid
# The user ID that transfers should take place as
#
# * This user must have access to all of the relevant files
# The user ID that transfers should take place as. Keep in mind that if this
# user is non-root, it may cause issues if the user lacks permission to write
# out any files that are transferred.
#
# @param gid
# The group ID that transfers should take place as
#
# * Must have access to all of the relevant files
# The group ID that transfers should take place as. Keep in mind that if this
# group is non-root, it may cause issues if the group lacks permission to
# write out any files that are transferred.
#
# @param outgoing_chmod
# A symbolic ``chmod`` that will be applied to files that are transferred
Expand Down Expand Up @@ -87,24 +91,24 @@
# * Should be set to the String ``*`` as it is overridden by ``$hosts_allow``
#
define rsync::server::section (
Stdlib::Absolutepath $path,
Optional[Array[String]] $auth_users = undef,
Optional[Array[String]] $user_pass = undef,
Optional[String] $comment = undef,
Boolean $use_chroot = false,
Integer[0] $max_connections = 0,
Integer[0] $max_verbosity = 1,
Stdlib::Absolutepath $lock_file = '/var/run/rsyncd.lock',
Boolean $read_only = true,
Boolean $write_only = false,
Boolean $list = false,
String $uid = 'root',
String $gid = 'root',
String $outgoing_chmod = 'o-w',
Boolean $ignore_nonreadable = true,
Boolean $transfer_logging = true,
String $log_format = "'%o %h [%a] %m (%u) %f %l'",
Array[String] $dont_compress = [
Stdlib::Absolutepath $path,
Optional[Rsync::Auth_users] $auth_users = undef,
Optional[Array[Pattern[/\A.*:.*\z/]]] $user_pass = undef,
Optional[String] $comment = undef,
Boolean $use_chroot = false,
Integer[0] $max_connections = 0,
Integer[0] $max_verbosity = 1,
Stdlib::Absolutepath $lock_file = '/var/run/rsyncd.lock',
Boolean $read_only = true,
Boolean $write_only = false,
Boolean $list = false,
String $uid = 'root',
String $gid = 'root',
String $outgoing_chmod = 'o-w',
Boolean $ignore_nonreadable = true,
Boolean $transfer_logging = true,
String $log_format = "'%o %h [%a] %m (%u) %f %l'",
Array[String] $dont_compress = [
'*.gz',
'*.tgz',
'*.zip',
Expand All @@ -131,13 +135,29 @@
content => template('rsync/rsyncd.conf.section.erb')
}

if !empty($auth_users) or !empty($user_pass) {
if $auth_users or $user_pass {
if $user_pass {
$secretsfile_lines = $user_pass
.map |$line| { "${line}\n" }
} else {
$secretsfile_lines = Hash.assert_type($auth_users) |$ex, $act| {
$auth_users.reduce({}) |$hash, $user| { $hash + { $user => undef } }
}.map |$username, $maybe_password| {
$password = $maybe_password ? {
String[1] => $maybe_password, # non-empty string: it's a password, use it
default => simplib::passgen($username), # undef or '': look up the password
}

[$username, ':', $password, "\n"].join
}
}

file { "/etc/rsync/${name}.rsyncd.secrets":
ensure => 'file',
owner => $uid,
group => $gid,
owner => 'root',
group => 'root',
mode => '0600',
content => template('rsync/secrets.erb'),
content => $secretsfile_lines.join,
show_diff => false,
require => File['/etc/rsync']
}
Expand Down
3 changes: 1 addition & 2 deletions spec/acceptance/suites/default/10_server_client_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@
}

rsync::server::section { 'test':
auth_users => ['test_user'],
user_pass => ['test_user:test_pass'],
auth_users => { 'test_user' => 'test_pass' },
comment => 'A test system',
hosts_allow => ['#{server1_ip}', '#{server2_ip}'],
path => '/srv/rsync/test',
Expand Down
18 changes: 8 additions & 10 deletions spec/acceptance/suites/default/20_server_client_stunnel_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,10 @@
}

rsync::server::section { 'test':
auth_users => ['test_user'],
user_pass => ['test_user:test_pass'],
comment => 'A test system',
path => '/srv/rsync/test',
require => File['/srv/rsync/test/test_file_srvcli2_server1']
auth_users => { 'test_user' => 'test_pass' },
comment => 'A test system',
path => '/srv/rsync/test',
require => File['/srv/rsync/test/test_file_srvcli2_server1']
}

stunnel::connection { 'rsync':
Expand Down Expand Up @@ -88,11 +87,10 @@
}

rsync::server::section { 'test':
auth_users => ['test_user'],
user_pass => ['test_user:test_pass'],
comment => 'A test system',
path => '/srv/rsync/test',
require => File['/srv/rsync/test/test_file_srvcli2_server2']
auth_users => { 'test_user' => 'test_pass' },
comment => 'A test system',
path => '/srv/rsync/test',
require => File['/srv/rsync/test/test_file_srvcli2_server2']
}

stunnel::connection { 'rsync':
Expand Down
142 changes: 106 additions & 36 deletions spec/defines/server/section_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,61 +4,131 @@
on_supported_os.each do |os, os_facts|
context "on #{os}" do
let(:title) { 'test' }

let(:facts) {
_facts = os_facts
_facts[:os] ||= {}
_facts[:os]['selinux'] ||= {}
_facts[:os]['selinux']['enabled'] = true

_facts
}

let(:pre_condition) {
'include "::rsync::server"'
}
let(:facts) { os_facts }

context 'with default parameters' do
let(:params) {{
:path => '/test/dir'
}}

it { is_expected.to compile.with_all_deps }
it { is_expected.to create_concat__fragment("rsync_#{title}.section") }
it { is_expected.to_not create_file("/etc/rsync/#{title}.rsyncd.secrets") }
it 'renders the section as an anonymous share, with no secrets file' do
is_expected.to compile.with_all_deps
is_expected.to create_concat__fragment("rsync_#{title}.section")
is_expected.to_not create_concat__fragment("rsync_#{title}.section")
.with_content(%r{auth users = })
.with_content(%r{secrets file = })
is_expected.to_not create_file("/etc/rsync/#{title}.rsyncd.secrets")
end
end

context 'with user_pass and comment parameters set' do
context 'with user_pass and comment parameters set and without auth_users' do
let(:params) {{
:path => '/test/dir',
:user_pass => [ 'user1:user1password', 'user2:user2password', 'skipme'],
:user_pass => [ 'user1:user1password', 'user2:user2password'],
:comment => 'section TEST'
}}

it { is_expected.to compile.with_all_deps }
it do
is_expected.to create_file("/etc/rsync/#{title}.rsyncd.secrets").with(
:ensure => 'file',
:owner => 'root',
:group => 'root',
:mode => '0600',
:show_diff => false,
:content => <<-EOM
user1:user1password
user2:user2password
EOM
)
it 'renders the section as an authenticated share, with a secrets file' do
is_expected.to compile.with_all_deps
is_expected.to create_concat__fragment("rsync_#{title}.section")
.with_content(%r{path = /test/dir$})
.with_content(%r{section TEST$})
.with_content(%r{^auth users = all:deny$})
.with_content(%r{^secrets file = /etc/rsync/test\.rsyncd\.secrets$})
is_expected.to create_file("/etc/rsync/#{title}.rsyncd.secrets")
.with(
:ensure => 'file',
:owner => 'root',
:group => 'root',
:mode => '0600',
:show_diff => false,
:content => <<~EOM
user1:user1password
user2:user2password
EOM
)
end

context 'with a badly formatted user:pass line in the user_pass array,' do
let(:params) { super().merge(user_pass: [ 'user1:user1password', 'baduserpass', 'user2:user2password']) }

it 'fails with an invalid parameter error' do
is_expected.to compile.and_raise_error(%r{Evaluation Error:.*parameter 'user_pass'.*expects.*, got 'baduserpass'})
end
end
end

context 'with auth_users parameter set' do
context 'with auth_users parameter set as an Array,' do
let(:params) {{
:path => '/test/dir',
:auth_users => [ 'authuser1', 'authuser2' ],
}}

it 'renders a secrets file and a section with the given auth users' do
is_expected.to compile.with_all_deps
is_expected.to create_concat__fragment("rsync_#{title}.section")
.with_content(%r{^path = /test/dir$})
.with_content(%r{^auth users = authuser1,authuser2$})
is_expected.to create_file("/etc/rsync/#{title}.rsyncd.secrets")
.with_content(/^authuser1:.+$/)
.with_content(/^authuser2:.+$/)
end

context 'with uid and gid set to a non-root user/group' do
let(:params) { super().merge(uid: 'testuser', gid: 'testgroup') }

it 'creates a secrets file owned by root with mode 0600, and sets the given user/group for the section' do
is_expected.to create_concat__fragment("rsync_#{title}.section")
.with_content(%r{^uid = testuser$})
.with_content(%r{^gid = testgroup$})
is_expected.to create_file("/etc/rsync/#{title}.rsyncd.secrets")
.with_owner('root')
.with_group('root')
.with_mode('0600')
end
end
context 'with uid and gid set to a non-root user/group' do
let(:params) { super().merge(auth_users: []) }

it 'renders a section with a defined secrets file, but not as an anonymous share' do
is_expected.to create_concat__fragment("rsync_#{title}.section")
.with_content(%r{^auth users = all:deny$})
is_expected.to create_file("/etc/rsync/#{title}.rsyncd.secrets")
.with_content('')
end
end
end
context 'with auth_users parameter set as a Hash,' do
let(:params) {{
:path => '/test/dir',
:auth_users => [ 'authuser1', 'authuser2'],
:auth_users => {
'authuser1' => 'pass1',
'dontskipme' => '',
'authuser2' => 'pass2',
},
}}
it { is_expected.to compile.with_all_deps }
it { is_expected.to create_file("/etc/rsync/#{title}.rsyncd.secrets").with_content(/^authuser1:/) }
it { is_expected.to create_file("/etc/rsync/#{title}.rsyncd.secrets").with_content(/^authuser2:/) }

it 'renders a secrets file and a section with the given auth users' do
is_expected.to compile.with_all_deps
is_expected.to create_concat__fragment("rsync_#{title}.section")
.with_content(%r{^path = /test/dir$})
.with_content(%r{^auth users = authuser1,dontskipme,authuser2$})
is_expected.to create_file("/etc/rsync/#{title}.rsyncd.secrets")
.with_content(/^authuser1:pass1$/)
.with_content(/^dontskipme:.+$/)
.with_content(/^authuser2:pass2$/)
end

context 'with uid and gid set to a non-root user/group' do
let(:params) { super().merge(auth_users: []) }

it 'renders a section with a defined secrets file, but not as an anonymous share' do
is_expected.to create_concat__fragment("rsync_#{title}.section")
.with_content(%r{^auth users = all:deny$})
is_expected.to create_file("/etc/rsync/#{title}.rsyncd.secrets")
.with_content('')
end
end
end
end
end
Expand Down
5 changes: 3 additions & 2 deletions templates/rsyncd.conf.section.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@

[<%= @name %>]
comment = <%= @comment %>
path = <%= @path %>
Expand All @@ -17,7 +18,7 @@ log format = <%= @log_format %>
dont compress = <%= Array(@dont_compress).join(' ') %>
hosts allow = <%= Array(@hosts_allow).join(',') %>
hosts deny = <%= Array(@hosts_deny).join(',') %>
<% if @auth_users -%>
auth users = <%= Array(@auth_users).join(',') %>
<% if @auth_users or @user_pass -%>
auth users = <%= @auth_users.to_a.empty? ? 'all:deny' : @auth_users.map { |entry| [entry].flatten[0] }.join(',') %>
secrets file = /etc/rsync/<%= @name %>.rsyncd.secrets
<% end -%>
17 changes: 0 additions & 17 deletions templates/secrets.erb
View file
Open in desktop

This file was deleted.

6 changes: 6 additions & 0 deletions types/auth_users.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# @summary Validator for rsync::server::auth_users
#
type Rsync::Auth_users = Variant[
Array[String[1]],
Hash[String[1], Optional[String]]
]