Skip to content

FM-2298 and FM-2299 update Login and User to take hash of permissions #79

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

Merged
merged 3 commits into from
Mar 4, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 21 additions & 0 deletions lib/puppet/parser/functions/sqlserver_validate_hash_uniq_values.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# === Defined Parser Function: sqlserver_validate_hash_uniq_values
#
# [args*] A hash, that contains string or string[] for values
#
# @raise [Puppet::ParserError] When duplicates are found
#
module Puppet::Parser::Functions
newfunction(:sqlserver_validate_hash_uniq_values) do |arguments|
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing in-code docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, this is the same pattern we have in all the other functions for sqlserver however. At that point we would either to redo all of them or have inconsistency. This one also works with Yardocs to generate documentation with puppet-strings.


raise(Puppet::ParseError, 'Expect a Hash as an argument') unless arguments[0].is_a?(Hash)

value = arguments[0].each_value.collect { |v| v }.flatten

total_count = value.count
uniq_count = value.uniq.count
msg = arguments[1] ? arguments[1] : "Duplicate values passed to hash #{value}"
if uniq_count != total_count
raise(Puppet::ParseError, msg)
end
end
end
46 changes: 44 additions & 2 deletions manifests/login.pp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@
# @see http://technet.microsoft.com/en-us/library/ms189751(v=sql.110).aspx Create Login
# @see http://technet.microsoft.com/en-us/library/ms189828(v=sql.110).aspx Alter Login
#
# [permissions]
# A hash of permissions that should be managed for the login. Valid keys are 'GRANT', 'GRANT_WITH_OPTION', 'DENY' or 'REVOKE'. Valid values must be an array of Strings i.e. {'GRANT' => ['CONNECT SQL', 'CREATE ANY DATABASE'] }
#
##
define sqlserver::login (
$login = $title,
$instance = 'MSSQLSERVER',
Expand All @@ -57,6 +61,7 @@
$check_expiration = false,
$check_policy = true,
$disabled = false,
$permissions = { },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parameter not added to in-file docs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

) {

sqlserver_validate_instance_name($instance)
Expand All @@ -67,15 +72,52 @@
fail ('Can not have check expiration enabled when check_policy is disabled')
}

$create_delete = $ensure ? {
$_create_delete = $ensure ? {
present => 'create',
absent => 'delete',
}

sqlserver_tsql{ "login-${instance}-${login}":
instance => $instance,
command => template("sqlserver/${create_delete}/login.sql.erb"),
command => template("sqlserver/${_create_delete}/login.sql.erb"),
onlyif => template('sqlserver/query/login_exists.sql.erb'),
require => Sqlserver::Config[$instance]
}

if $ensure == present {
validate_hash($permissions)
$_upermissions = sqlserver_upcase($permissions)
sqlserver_validate_hash_uniq_values($_upermissions, "Duplicate permissions found for sqlserver::login[${title}]")

Sqlserver::Login::Permissions{
login => $login,
instance => $instance,
require => Sqlserver_tsql["login-${instance}-${login}"]
}
if has_key($_upermissions, 'GRANT') and is_array($_upermissions['GRANT']) {
sqlserver::login::permissions{ "Sqlserver::Login[${title}]-GRANT-${login}":
state => 'GRANT',
permissions => $_upermissions['GRANT'],
}
}
if has_key($_upermissions, 'DENY') and is_array($_upermissions['DENY']) {
sqlserver::login::permissions{ "Sqlserver::Login[${title}]-DENY-${login}":
state => 'DENY',
permissions => $_upermissions['DENY'],
}
}
if has_key($_upermissions, 'REVOKE') and is_array($_upermissions['REVOKE']) {
sqlserver::login::permissions{ "Sqlserver::Login[${title}]-REVOKE-${login}":
state => 'REVOKE',
permissions => $_upermissions['REVOKE'],
}
}
if has_key($_upermissions, 'GRANT_WITH_OPTION') and is_array($_upermissions['GRANT_WITH_OPTION']) {
sqlserver::login::permissions{ "Sqlserver::Login[${title}]-GRANT-WITH_GRANT_OPTION-${login}":
state => 'GRANT',
with_grant_option => true,
permissions => $_upermissions['GRANT_WITH_OPTION'],
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
##
# == Define Resource Type: sqlserver::login::permission#
# == Define Resource Type: sqlserver::login::permissions#
#
# === Requirement/Dependencies:
#
Expand All @@ -20,7 +20,7 @@
# The name of the instance where the user and database exists. Defaults to 'MSSQLSERVER'
#
##
define sqlserver::login::permission (
define sqlserver::login::permissions (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this intentionally backwards incompatible?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, unreleased, and b/c it takes arrays now and roles::permissions as well, wanted to ensure same pattern.

$login,
$permissions,
$state = 'GRANT',
Expand Down
41 changes: 41 additions & 0 deletions manifests/user.pp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@
# [password]
# The password for the user, can only be used when the database is a contained database.
#
# [permissions]
# A hash of permissions that should be managed for the user. Valid keys are 'GRANT', 'GRANT_WITH_OPTION', 'DENY' or 'REVOKE'. Valid values must be an array of Strings i.e. {'GRANT' => ['SELECT', 'INSERT'] }
#
##
define sqlserver::user (
$database,
Expand All @@ -43,6 +46,7 @@
$instance = 'MSSQLSERVER',
$login = undef,
$password = undef,
$permissions = { },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parameter not added to in-file docs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

)
{
sqlserver_validate_instance_name($instance)
Expand All @@ -69,4 +73,41 @@
require => Sqlserver::Config[$instance]
}

if $ensure == present {
validate_hash($permissions)
$_upermissions = sqlserver_upcase($permissions)
sqlserver_validate_hash_uniq_values($_upermissions, "Duplicate permissions found for sqlserver::user[${title}]")

Sqlserver::User::Permissions{
user => $user,
database => $database,
instance => $instance,
require => Sqlserver_tsql["user-${instance}-${database}-${user}"]
}
if has_key($_upermissions, 'GRANT') and is_array($_upermissions['GRANT']) {
sqlserver::user::permissions{ "Sqlserver::User[${title}]-GRANT-${user}":
state => 'GRANT',
permissions => $_upermissions['GRANT'],
}
}
if has_key($_upermissions, 'DENY') and is_array($_upermissions['DENY']) {
sqlserver::user::permissions{ "Sqlserver::User[${title}]-DENY-${user}":
state => 'DENY',
permissions => $_upermissions['DENY'],
}
}
if has_key($_upermissions, 'REVOKE') and is_array($_upermissions['REVOKE']) {
sqlserver::user::permissions{ "Sqlserver::User[${title}]-REVOKE-${user}":
state => 'REVOKE',
permissions => $_upermissions['REVOKE'],
}
}
if has_key($_upermissions, 'GRANT_WITH_OPTION') and is_array($_upermissions['GRANT_WITH_OPTION']) {
sqlserver::user::permissions{ "Sqlserver::User[${title}]-GRANT-WITH_GRANT_OPTION-${user}":
state => 'GRANT',
with_grant_option => true,
permissions => $_upermissions['GRANT_WITH_OPTION'],
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
##
# == Define Resource Type: sqlserver::user::permission#
# == Define Resource Type: sqlserver::user::permissions
#
# === Requirement/Dependencies:
#
Expand All @@ -26,7 +26,7 @@
# The name of the instance where the user and database exists. Defaults to 'MSSQLSERVER'
#
##
define sqlserver::user::permission (
define sqlserver::user::permissions (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above, backwards incompat

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unreleased pattern matching to sqlserver::role::permissions

$user,
$database,
$permissions,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
require 'spec_helper'
require File.expand_path(File.join(File.dirname(__FILE__), '..', 'manifest_shared_examples.rb'))

describe 'sqlserver::login::permission' do
describe 'sqlserver::login::permissions' do
let(:facts) { {:osfamily => 'windows'} }
context 'validation errors' do
include_context 'manifests' do
Expand All @@ -10,11 +10,11 @@
end
context 'login =>' do
let(:params) { {
:permissions=> ['SELECT'],
:permissions => ['SELECT'],
} }
let(:raise_error_check) { 'Login must be between 1 and 128 characters' }
describe 'missing' do
let(:raise_error_check) { 'Must pass login to Sqlserver::Login::Permission[myTitle]' }
let(:raise_error_check) { 'Must pass login to Sqlserver::Login::Permissions[myTitle]' }
it_behaves_like 'validation error'
end
describe 'empty' do
Expand All @@ -28,26 +28,26 @@
end
context 'permission' do
let(:params) { {
:login => 'loggingUser',
:login => 'loggingUser',
} }
let(:raise_error_check) { 'Permission must be between 4 and 128 characters' }
describe 'empty' do
let(:additional_params) { {:permissions=> ['']} }
let(:additional_params) { {:permissions => ['']} }
it_behaves_like 'validation error'
end
describe 'under limit' do
let(:additional_params) { {:permissions=> [random_string_of_size(3, false)]} }
let(:additional_params) { {:permissions => [random_string_of_size(3, false)]} }
it_behaves_like 'validation error'
end
describe 'over limit' do
let(:additional_params) { {:permissions=> [random_string_of_size(129, false)]} }
let(:additional_params) { {:permissions => [random_string_of_size(129, false)]} }
it_behaves_like 'validation error'
end
end
context 'state =>' do
let(:params) { {
:permissions=> ['SELECT'],
:login => 'loggingUser'
:permissions => ['SELECT'],
:login => 'loggingUser'
} }
describe 'invalid' do
let(:additional_params) { {:state => 'invalid'} }
Expand All @@ -61,8 +61,8 @@
let(:title) { 'myTitle' }
let(:sqlserver_tsql_title) { 'login-permission-MSSQLSERVER-loggingUser-GRANT' }
let(:params) { {
:login => 'loggingUser',
:permissions=> ['SELECT'],
:login => 'loggingUser',
:permissions => ['SELECT'],
} }
end
%w(revoke grant deny).each do |state|
Expand Down Expand Up @@ -90,7 +90,7 @@
it_behaves_like 'sqlserver_tsql command'
end
describe 'alter' do
let(:additional_params) { {:permissions=> ['ALTER']} }
let(:additional_params) { {:permissions => ['ALTER']} }
let(:should_contain_command) { ['USE [master];', 'GRANT ALTER TO [loggingUser];'] }
let(:sqlserver_tsql_title) { "login-permission-MSSQLSERVER-loggingUser-GRANT" }
it_behaves_like 'sqlserver_tsql command'
Expand All @@ -113,19 +113,19 @@
let(:title) { 'myTitle' }
let(:sqlserver_tsql_title) { 'login-permission-MSSQLSERVER-loggingUser-GRANT' }
let(:params) { {
:login => 'loggingUser',
:permissions => ['SELECT'],
:login => 'loggingUser',
:permissions => ['SELECT'],
} }
describe '' do
let(:should_contain_command) { [
'USE [master];',
'GRANT SELECT TO [loggingUser];',
/DECLARE @perm_state varchar\(250\)/,
/SET @perm_state = ISNULL\(\n\s+\(SELECT perm.state_desc FROM sys\.server_permissions perm\n\s+JOIN sys\./,
/JOIN sys\.server_principals princ ON princ.principal_id = perm\.grantee_principal_id\n\s+WHERE/,
/WHERE princ\.type IN \('U','S','G'\)\n\s+ AND princ\.name = 'loggingUser'\n\s+AND perm\.permission_name = @permission\),\n\s+'REVOKE'\)/,
/SET @error_msg = 'EXPECTED login \[loggingUser\] to have permission \[' \+ @permission \+ '\] with GRANT but got ' \+ @perm_state;/,
/IF @perm_state != 'GRANT'\n\s+THROW 51000, @error_msg, 10/
'USE [master];',
'GRANT SELECT TO [loggingUser];',
/DECLARE @perm_state varchar\(250\)/,
/SET @perm_state = ISNULL\(\n\s+\(SELECT perm.state_desc FROM sys\.server_permissions perm\n\s+JOIN sys\./,
/JOIN sys\.server_principals princ ON princ.principal_id = perm\.grantee_principal_id\n\s+WHERE/,
/WHERE princ\.type IN \('U','S','G'\)\n\s+ AND princ\.name = 'loggingUser'\n\s+AND perm\.permission_name = @permission\),\n\s+'REVOKE'\)/,
/SET @error_msg = 'EXPECTED login \[loggingUser\] to have permission \[' \+ @permission \+ '\] with GRANT but got ' \+ @perm_state;/,
/IF @perm_state != 'GRANT'\n\s+THROW 51000, @error_msg, 10/
] }
it_behaves_like 'sqlserver_tsql command'
end
Expand Down
Loading