From 3fb0beab9b0acfdda33bfa26b1e99b81d9156542 Mon Sep 17 00:00:00 2001 From: Tim Smith Date: Wed, 6 Dec 2017 16:41:40 -0800 Subject: Improve the passwordless user error, logic, and tests Specify exactly what we're checking for in the error. This needs to go into the docs site as well. Signed-off-by: Tim Smith --- lib/chef/resource/windows_task.rb | 13 ++++++------- spec/functional/resource/windows_task_spec.rb | 6 +++--- spec/unit/resource/windows_task_spec.rb | 27 ++++++++++++++++----------- 3 files changed, 25 insertions(+), 21 deletions(-) diff --git a/lib/chef/resource/windows_task.rb b/lib/chef/resource/windows_task.rb index d8e6818c0b..37a818c7fe 100644 --- a/lib/chef/resource/windows_task.rb +++ b/lib/chef/resource/windows_task.rb @@ -119,15 +119,14 @@ class Chef SYSTEM_USERS = ['NT AUTHORITY\SYSTEM', "SYSTEM", 'NT AUTHORITY\LOCALSERVICE', 'NT AUTHORITY\NETWORKSERVICE', 'BUILTIN\USERS', "USERS"].freeze def validate_user_and_password(user, password) - if user && use_password?(user) - if password.nil? - raise ArgumentError, "Cannot specify a user other than the 'SYSTEM' user without specifying a password!" - end + if password_required?(user) && password.nil? + raise ArgumentError, %q{Cannot specify a user other than the system users without specifying a password!. Valid passwordless users: 'NT AUTHORITY\SYSTEM', 'SYSTEM', 'NT AUTHORITY\LOCALSERVICE', 'NT AUTHORITY\NETWORKSERVICE', 'BUILTIN\USERS', 'USERS'} end end - def use_password?(user) - @use_password ||= !SYSTEM_USERS.include?(user.upcase) + def password_required?(user) + return false if user.nil? + @password_required ||= !SYSTEM_USERS.include?(user.upcase) end def validate_interactive_setting(interactive_enabled, password) @@ -198,7 +197,7 @@ class Chef end unless idle_time.to_i > 0 && idle_time.to_i <= 999 - raise "idle_time value #{idle_time} is invalid. Valid values for :on_idle frequency are 1 - 999." + raise "idle_time value #{idle_time} is invalid. Valid values for :on_idle frequency are 1 - 999." end end diff --git a/spec/functional/resource/windows_task_spec.rb b/spec/functional/resource/windows_task_spec.rb index b59f0670b6..b6e6540693 100644 --- a/spec/functional/resource/windows_task_spec.rb +++ b/spec/functional/resource/windows_task_spec.rb @@ -339,7 +339,7 @@ describe Chef::Resource::WindowsTask, :windows_only do it "raises error" do subject.user "Administrator" subject.frequency :onstart - expect { subject.after_created }.to raise_error("Cannot specify a user other than the 'SYSTEM' user without specifying a password!") + expect { subject.after_created }.to raise_error(%q{Cannot specify a user other than the system users without specifying a password!. Valid passwordless users: 'NT AUTHORITY\SYSTEM', 'SYSTEM', 'NT AUTHORITY\LOCALSERVICE', 'NT AUTHORITY\NETWORKSERVICE', 'BUILTIN\USERS', 'USERS'}) end end @@ -363,7 +363,7 @@ describe Chef::Resource::WindowsTask, :windows_only do it "raises error" do subject.months "xyz" subject.frequency :monthly - expect { subject.after_created }.to raise_error("months attribute invalid. Only valid values are: JAN, FEB, MAR, APR, MAY, JUN, JUL, AUG, SEP, OCT, NOV, DEC and *. Multiple values must be separated by a comma.") + expect { subject.after_created }.to raise_error("months property invalid. Only valid values are: JAN, FEB, MAR, APR, MAY, JUN, JUL, AUG, SEP, OCT, NOV, DEC and *. Multiple values must be separated by a comma.") end end @@ -379,7 +379,7 @@ describe Chef::Resource::WindowsTask, :windows_only do it "raises error" do subject.idle_time 300 subject.frequency :monthly - expect { subject.after_created }.to raise_error("idle_time attribute is only valid for tasks that run on_idle") + expect { subject.after_created }.to raise_error("idle_time property is only valid for tasks that run on_idle") end end end diff --git a/spec/unit/resource/windows_task_spec.rb b/spec/unit/resource/windows_task_spec.rb index dc5c362388..0498292888 100644 --- a/spec/unit/resource/windows_task_spec.rb +++ b/spec/unit/resource/windows_task_spec.rb @@ -62,6 +62,18 @@ describe Chef::Resource::WindowsTask do expect(resource.frequency).to eql(:hourly) end + context "when user is set but password is not" do + it "raises an error if the user is a non-system user" do + resource.user "bob" + expect { resource.after_created }.to raise_error(Chef::Exceptions::ArgumentError, %q{Cannot specify a user other than the system users without specifying a password!. Valid passwordless users: 'NT AUTHORITY\SYSTEM', 'SYSTEM', 'NT AUTHORITY\LOCALSERVICE', 'NT AUTHORITY\NETWORKSERVICE', 'BUILTIN\USERS', 'USERS'}) + end + + it "does not raise an error if the user is a system user" do + resource.user 'NT AUTHORITY\SYSTEM' + expect { resource.after_created }.to_not raise_error(Chef::Exceptions::ArgumentError) + end + end + context "when random_delay is passed" do it "raises error if frequency is `:once`" do resource.frequency :once @@ -83,12 +95,14 @@ describe Chef::Resource::WindowsTask do end end - context "when execution_time_limit is passed" do + context "when execution_time_limit isn't specified" do it "sets the default value to PT72H" do resource.after_created expect(resource.execution_time_limit).to eq("PT72H") end + end + context "when execution_time_limit is passed" do it "raises error for invalid execution_time_limit" do resource.execution_time_limit "abc" expect { resource.after_created }.to raise_error(Chef::Exceptions::ArgumentError, "Invalid value passed for `execution_time_limit`. Please pass seconds as a String e.g. '60'.") @@ -120,15 +134,6 @@ describe Chef::Resource::WindowsTask do end end - context "#validate_user_and_password" do - context "when password is not passed" do - it "raises error with non-system users" do - allow(resource).to receive(:use_password?).and_return(true) - expect { resource.send(:validate_user_and_password, "Testuser", nil) }.to raise_error("Cannot specify a user other than the 'SYSTEM' user without specifying a password!") - end - end - end - context "#validate_interactive_setting" do it "raises error when interactive_enabled is passed without password" do expect { resource.send(:validate_interactive_setting, true, nil) }.to raise_error("Please provide the password when attempting to set interactive/non-interactive.") @@ -213,7 +218,7 @@ describe Chef::Resource::WindowsTask do end it "raises error if idle_time > 999" do - expect { resource.send(:validate_idle_time, 1000, :on_idle) }.to raise_error("idle_time value 1000 is invalid. Valid values for :on_idle frequency are 1 - 999.") + expect { resource.send(:validate_idle_time, 1000, :on_idle) }.to raise_error("idle_time value 1000 is invalid. Valid values for :on_idle frequency are 1 - 999.") end end end -- cgit v1.2.1