Skip to content

(CONT-567) allow deferred function for password #436

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 2 commits into from
Jun 2, 2023
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
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ group :development do
gem "github_changelog_generator", '= 1.15.2', require: false
end
group :system_tests do
gem "puppet_litmus", '< 1.0.0', require: false, platforms: [:ruby, :x64_mingw]
gem "puppet_litmus", '~> 1.0', require: false, platforms: [:ruby, :x64_mingw]
gem "serverspec", '~> 2.41', require: false
end

Expand Down
13 changes: 13 additions & 0 deletions lib/puppet/functions/sqlserver/password.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# frozen_string_literal: true

# This function exists for usage of a role password that is a deferred function
Puppet::Functions.create_function(:'sqlserver::password') do
dispatch :password do
optional_param 'Any', :pass
Copy link
Contributor

Choose a reason for hiding this comment

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

Interested in the types here.

I wonder if we should be more prescriptive? For example, would String be a better choice?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chelnak the password is not mandetory field here so restricting with String will lead to make it mandetory. That's why I have added to any type.
Ref : https://github.com/puppetlabs/puppetlabs-sqlserver/blob/main/manifests/login.pp#L59
Please let me know if we wanted to make it required field.

return_type 'Any'
end

def password(pass)
pass
end
end
14 changes: 13 additions & 1 deletion manifests/login.pp
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,21 @@
'absent' => 'delete',
}

$parameters = {
'password' => Deferred('sqlserver::password', [$password]),
'disabled' => $disabled,
'login_type' => $login_type,
'login' => $login,
'default_language' => $default_language,
'default_database' => $default_database,
'check_policy' => $check_policy,
'check_expiration' => $check_expiration,
'svrroles' => $svrroles,
}

sqlserver_tsql { "login-${instance}-${login}":
instance => $instance,
command => template("sqlserver/${_create_delete}/login.sql.erb"),
command => stdlib::deferrable_epp("sqlserver/${_create_delete}/login.sql.epp", $parameters),
onlyif => template('sqlserver/query/login_exists.sql.erb'),
require => Sqlserver::Config[$instance],
}
Expand Down
10 changes: 9 additions & 1 deletion manifests/user.pp
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,17 @@
'absent' => 'delete',
}

$parameters = {
'password' => Deferred('sqlserver::password', [$password]),
'database' => $database,
'user' => $user,
'login' => $login,
'default_schema' => $default_schema,
}

sqlserver_tsql { "user-${instance}-${database}-${user}":
instance => $instance,
command => template("sqlserver/${create_delete}/user.sql.erb"),
command => stdlib::deferrable_epp("sqlserver/${create_delete}/user.sql.epp", $parameters),
onlyif => template('sqlserver/query/user_exists.sql.erb'),
require => Sqlserver::Config[$instance],
}
Expand Down
2 changes: 1 addition & 1 deletion metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"dependencies": [
{
"name": "puppetlabs/stdlib",
"version_requirement": ">= 4.13.1 < 9.0.0"
"version_requirement": ">= 8.4.0 < 9.0.0"
},
{
"name": "puppetlabs/powershell",
Expand Down
8 changes: 8 additions & 0 deletions spec/functions/password_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# frozen_string_literal: true

require 'spec_helper'

describe 'sqlserver::password' do
it { is_expected.to run.with_params('password').and_return('password') }
it { is_expected.to run.with_params(nil).and_return(nil) }
end
58 changes: 58 additions & 0 deletions templates/create/login.sql.epp
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
DECLARE
@is_disabled as tinyint = <%= if $disabled {1} else {0} %>,
@login_type as varchar(255) = NULL;

SET @login_type = (SELECT [type] FROM sys.server_principals where name = '<%= $login %>')
IF (@login_type IS NULL)
BEGIN
-- Create the login
CREATE LOGIN [<%= $login %>]
<% if $login_type !~ /WINDOWS_LOGIN/ { -%>
WITH
PASSWORD = '<%= $password %>',
CHECK_EXPIRATION = <% if $check_expiration { %>ON<% } else { %>OFF<% } %>,
CHECK_POLICY = <% if $check_policy { %>ON<% } else { %>OFF<% } %>,
<% } else { -%>
FROM WINDOWS WITH
<% } -%>
DEFAULT_LANGUAGE = [<%= $default_language %>],
DEFAULT_DATABASE = [<%= $default_database %>];
-- Fetch the login type
SET @login_type = (SELECT [type] FROM sys.server_principals where name = '<%= $login %>')
END

IF (@login_type = 'G')
BEGIN
-- Windows Group type logins can only be granted/denied connection
IF @is_disabled = 0 GRANT CONNECT SQL TO [<%= $login %>]
ELSE DENY CONNECT SQL TO [<%= $login %>]
END
ELSE
BEGIN
IF @is_disabled = 0 ALTER LOGIN [<%= $login %>] ENABLE
ELSE ALTER LOGIN [<%= $login %>] DISABLE
END

ALTER LOGIN [<%= $login %>] WITH
<% if $login_type != 'WINDOWS_LOGIN' { -%>
CHECK_EXPIRATION = <% if $check_expiration { %>ON<% } else { %>OFF<% } %>,
CHECK_POLICY = <% if $check_policy { %>ON<% } else { %>OFF<% } %>,
<% } -%>
DEFAULT_LANGUAGE = [<%= $default_language %>],
DEFAULT_DATABASE = [<%= $default_database %>];

<% $svrroles.each |String $role, Any $enable_bit| { -%>
IF (SELECT COUNT(me.role_principal_id) from sys.server_role_members me
JOIN sys.server_principals rol ON me.role_principal_id = rol.principal_id
JOIN sys.server_principals pri ON me.member_principal_id = pri.principal_id
WHERE rol.type_desc = 'SERVER_ROLE'
AND rol.name = '<%= $role %>'
AND pri.name = '<%= $login %>') != <%= $enable_bit %>
BEGIN
<% if ($enable_bit == '1') or ($enable_bit == 1) { -%>
ALTER SERVER ROLE [<%= $role %>] ADD MEMBER [<%= $login %>];
<% } else { -%>
ALTER SERVER ROLE [<%= $role %>] DROP MEMBER [<%= $login %>];
<% } -%>
END
<% } -%>
58 changes: 0 additions & 58 deletions templates/create/login.sql.erb

This file was deleted.

18 changes: 18 additions & 0 deletions templates/create/user.sql.epp
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
USE [<%= $database %>];
<% if $password { %>
IF EXISTS(select containment from sys.databases WHERE name = '<%= $database %>' AND containment = 0)
THROW 51000, 'Database must be contained in order to use passwords', 10
<% } %>
CREATE USER [<%= $user %>]
<% if $login { -%>
FROM LOGIN [<%= $login %>]
<% } else { -%>
<% if $password { -%>
WITH PASSWORD = '<%= $password %>'
<% } -%>
<% } -%>
<% if $default_schema { -%>
<% if $password { -%>,<% } else { -%>
WITH <% } -%>
DEFAULT_SCHEMA = <%= $default_schema %>
<% } -%>
18 changes: 0 additions & 18 deletions templates/create/user.sql.erb

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
USE master;
IF exists(select * from sys.server_principals where name = '<%= @login %>')
IF exists(select * from sys.server_principals where name = '<%= $login %>')
BEGIN
-- need to add logic to kill all possible connections if any exists,
-- possible force flag to prevent from happening during transaction if user would prefer to wait
DROP LOGIN [<%= @login %>]
DROP LOGIN [<%= $login %>]
END
4 changes: 4 additions & 0 deletions templates/delete/user.sql.epp
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
USE [<%= $database %>];
DROP USER [<%= $user %>];
IF EXISTS(SELECT name FROM sys.database_principals WHERE name = '<%= $user %>')
THROW 51000, 'Failed to drop user <%= $user %>', 10
4 changes: 0 additions & 4 deletions templates/delete/user.sql.erb

This file was deleted.