Drizzle

Merge lp:~ansharyan015/drizzle/auth_file_dynamic into lp:drizzle

  1. auth_file_dynamic
  2. Merge into 7.2
Proposed by Daniel Nichter
Status: Merged
Merged at revision: 2565
Proposed branch: lp:~ansharyan015/drizzle/auth_file_dynamic
Merge into: lp:drizzle
Diff against target: 364 lines (+194/-53)
7 files modified
plugin/auth_file/auth_file.cc (+132/-50)
plugin/auth_file/docs/index.rst (+14/-3)
plugin/auth_file/tests/r/dynamic_plugin.result (+10/-0)
plugin/auth_file/tests/t/dynamic.users1 (+3/-0)
plugin/auth_file/tests/t/dynamic.users2 (+3/-0)
plugin/auth_file/tests/t/dynamic_plugin-master.opt (+1/-0)
plugin/auth_file/tests/t/dynamic_plugin.test (+31/-0)
To merge this branch: bzr merge lp:~ansharyan015/drizzle/auth_file_dynamic
Reviewer Review Type Date Requested Status
Daniel Nichter (community) code review Approve
Review via email: mp+108864@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Daniel Nichter (daniel-nichter) wrote :

Overall the changes look godo. There are minor code style changes for later perhaps, like in

bool AuthFile::setUsersFile(std::string& usersFile)

the arg should probably be called "users_file" instead. Similarly, the use of "*_dummy" vars are more clear when named "new_*" (e.g. new_users_file), as was done in regex_policy.

review: Approve (code review)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugin/auth_file/auth_file.cc'
2--- plugin/auth_file/auth_file.cc 2012-01-15 20:54:59 +0000
3+++ plugin/auth_file/auth_file.cc 2012-06-06 03:05:26 +0000
4@@ -27,6 +27,7 @@
5 #include <boost/program_options.hpp>
6 #include <boost/filesystem.hpp>
7
8+#include <drizzled/item.h>
9 #include <drizzled/configmake.h>
10 #include <drizzled/plugin/authentication.h>
11 #include <drizzled/identifier.h>
12@@ -43,24 +44,22 @@
13 namespace auth_file {
14
15 static const fs::path DEFAULT_USERS_FILE= SYSCONFDIR "/drizzle.users";
16+typedef std::map<string, string> users_t;
17+bool updateUsersFile(Session *, set_var *);
18+bool parseUsersFile(std::string, users_t&);
19
20 class AuthFile : public plugin::Authentication
21 {
22 public:
23- AuthFile(fs::path users_file_arg);
24+ AuthFile(std::string users_file_arg);
25
26 /**
27 * Retrieve the last error encountered in the class.
28 */
29 const string& getError() const;
30
31- /**
32- * Load the users file into a map cache.
33- *
34- * @return True on success, false on error. If false is returned an error
35- * is set and can be retrieved with getError().
36- */
37- bool loadFile();
38+ std::string& getUsersFile();
39+ bool setUsersFile(std::string& usersFile);
40
41 private:
42
43@@ -85,18 +84,29 @@
44 const string &scrambled_password);
45
46 string error;
47- const fs::path users_file;
48+ fs::path users_file;
49+ std::string sysvar_users_file;
50
51 /**
52 * Cache or username:password entries from the file.
53 */
54- typedef std::map<string, string> users_t;
55 users_t users;
56+ /**
57+ * This method is called to update the users cache with the new
58+ * username:password pairs given in new users file upon update.
59+ */
60+ void setUsers(users_t);
61+ /**
62+ * This method is called to delete all the cached username:password pairs
63+ * when users file is updated.
64+ */
65+ void clearUsers();
66 };
67+AuthFile *auth_file= NULL;
68
69-AuthFile::AuthFile(fs::path users_file_arg) :
70+AuthFile::AuthFile(std::string users_file_arg) :
71 plugin::Authentication("auth_file"),
72- users_file(users_file_arg)
73+ users_file(users_file_arg), sysvar_users_file(users_file_arg)
74 {
75 }
76
77@@ -105,41 +115,29 @@
78 return error;
79 }
80
81-bool AuthFile::loadFile()
82-{
83- ifstream file(users_file.string().c_str());
84-
85- if (!file.is_open())
86- {
87- error = "Could not open users file: " + users_file.string();
88- return false;
89- }
90-
91- string line;
92- while (getline(file, line))
93- {
94- /* Ignore blank lines and lines starting with '#'. */
95- if (line.empty() || line[line.find_first_not_of(" \t")] == '#')
96- continue;
97-
98- string username;
99- string password;
100- size_t password_offset = line.find(":");
101- if (password_offset == string::npos)
102- username = line;
103- else
104- {
105- username = string(line, 0, password_offset);
106- password = string(line, password_offset + 1);
107- }
108-
109- if (not users.insert(pair<string, string>(username, password)).second)
110- {
111- error = "Duplicate entry found in users file: " + username;
112- return false;
113- }
114- }
115- return true;
116+std::string& AuthFile::getUsersFile()
117+{
118+ return sysvar_users_file;
119+}
120+
121+bool AuthFile::setUsersFile(std::string& usersFile)
122+{
123+ if (usersFile.empty())
124+ {
125+ errmsg_printf(error::ERROR, _("users file cannot be an empty string"));
126+ return false; // error
127+ }
128+ users_t users_dummy;
129+ if(parseUsersFile(usersFile, users_dummy))
130+ {
131+ this->clearUsers();
132+ this->setUsers(users_dummy);
133+ sysvar_users_file= usersFile;
134+ fs::path newUsersFile(getUsersFile());
135+ users_file= newUsersFile;
136+ return true; //success
137+ }
138+ return false; // error
139 }
140
141 bool AuthFile::verifyMySQLHash(const string &password,
142@@ -195,12 +193,96 @@
143 : password == *user;
144 }
145
146+void AuthFile::setUsers(users_t users_dummy)
147+{
148+ users.insert(users_dummy.begin(), users_dummy.end());
149+}
150+
151+void AuthFile::clearUsers()
152+{
153+ users.clear();
154+}
155+
156+/**
157+ * This function is called when the value of users file is changed in the system.
158+ *
159+ * @return False on success, True on error.
160+ */
161+bool updateUsersFile(Session *, set_var* var)
162+{
163+ if (not var->value->str_value.empty())
164+ {
165+ std::string newUsersFile(var->value->str_value.data());
166+ if (auth_file->setUsersFile(newUsersFile))
167+ return false; //success
168+ else
169+ return true; // error
170+ }
171+ errmsg_printf(error::ERROR, _("auth_file file cannot be NULL"));
172+ return true; // error
173+}
174+
175+/**
176+ * Parse the users file into a map cache.
177+ *
178+ * @return True on success, false on error. If an error is encountered, false is
179+ * returned with error message printed.
180+ */
181+bool parseUsersFile(std::string new_users_file, users_t& users_dummy)
182+{
183+ ifstream file(new_users_file.c_str());
184+
185+ if (!file.is_open())
186+ {
187+ string error_msg= "Could not open users file: " + new_users_file;
188+ errmsg_printf(error::ERROR, _(error_msg.c_str()));
189+ return false;
190+ }
191+
192+ string line;
193+ try
194+ {
195+ while (getline(file, line))
196+ {
197+ /* Ignore blank lines and lines starting with '#'. */
198+ if (line.empty() || line[line.find_first_not_of(" \t")] == '#')
199+ continue;
200+
201+ string username;
202+ string password;
203+ size_t password_offset = line.find(":");
204+ if (password_offset == string::npos)
205+ username = line;
206+ else
207+ {
208+ username = string(line, 0, password_offset);
209+ password = string(line, password_offset + 1);
210+ }
211+
212+ if (not users_dummy.insert(pair<string, string>(username, password)).second)
213+ {
214+ string error_msg= "Duplicate entry found in users file: " + username;
215+ errmsg_printf(error::ERROR, _(error_msg.c_str()));
216+ return false;
217+ }
218+ }
219+ return true;
220+ }
221+ catch (const std::exception &e)
222+ {
223+ /* On any non-EOF break, unparseable line */
224+ string error_msg= "Unable to parse users file " + new_users_file + ":" + e.what();
225+ errmsg_printf(error::ERROR, _(error_msg.c_str()));
226+ return false;
227+ }
228+}
229+
230 static int init(module::Context &context)
231 {
232 const module::option_map &vm= context.getOptions();
233
234- AuthFile *auth_file = new AuthFile(fs::path(vm["users"].as<string>()));
235- if (not auth_file->loadFile())
236+ auth_file= new AuthFile(vm["users"].as<string>());
237+ if (!auth_file->setUsersFile(auth_file->getUsersFile()))
238 {
239 errmsg_printf(error::ERROR, _("Could not load auth file: %s\n"), auth_file->getError().c_str());
240 delete auth_file;
241@@ -208,7 +290,7 @@
242 }
243
244 context.add(auth_file);
245- context.registerVariable(new sys_var_const_string_val("users", vm["users"].as<string>()));
246+ context.registerVariable(new sys_var_std_string("users", auth_file->getUsersFile(), NULL, &updateUsersFile));
247
248 return 0;
249 }
250
251=== modified file 'plugin/auth_file/docs/index.rst'
252--- plugin/auth_file/docs/index.rst 2011-11-06 00:00:03 +0000
253+++ plugin/auth_file/docs/index.rst 2012-06-06 03:05:26 +0000
254@@ -7,8 +7,8 @@
255
256 :program:`auth_file` is a security risk! Do not use this plugin with production servers!
257
258-:program:`auth_file` is an authentication plugin that authenticates connections
259-using a list of ``username:password`` entries in a plain text file.
260+:program:`auth_file` is an :doc:`/administration/authorization` plugin that authenticates connections
261+using a list of ``username:password`` entries in a plain text file. When :program:`drizzled` is started with ``--plugin-add=auth_file``, the file based authorization plugin is enabled with the default users file. Users file can be specified by either specifying ``--auth-file.users=<users file>`` at the time of server startup or by changing the ``auth_file_users`` with ``SET GLOBAL``.
262
263 .. note::
264
265@@ -61,7 +61,7 @@
266 * ``auth_file_users``
267
268 :Scope: Global
269- :Dynamic: No
270+ :Dynamic: Yes
271 :Option: :option:`--auth-file.users`
272
273 File to load for usernames and passwords.
274@@ -94,6 +94,17 @@
275 Welcome to the Drizzle client.. Commands end with ; or \g.
276 ...
277
278+Changing users file at runtime
279+-------------------------------
280+
281+Users file can be reloaded by::
282+
283+ SET GLOBAL auth_file_users=@@auth_file_users
284+
285+Moreover, the users file can be changed by::
286+
287+ SET GLOBAL auth_file_users=/path/to/new/users/file
288+
289 .. _auth_file_authors:
290
291 Authors
292
293=== added file 'plugin/auth_file/tests/r/dynamic_plugin.result'
294--- plugin/auth_file/tests/r/dynamic_plugin.result 1970-01-01 00:00:00 +0000
295+++ plugin/auth_file/tests/r/dynamic_plugin.result 2012-06-06 03:05:26 +0000
296@@ -0,0 +1,10 @@
297+SET GLOBAL auth_file_users="";
298+ERROR HY000: Incorrect arguments to SET
299+SET GLOBAL auth_file_users="TOP_SRCDIR/plugin/auth_file/tests/t/dynamic.users3";
300+ERROR HY000: Incorrect arguments to SET
301+SET GLOBAL auth_file_users="TOP_SRCDIR/plugin/auth_file/tests/t/dynamic.users2";
302+connect(localhost,auth_file_password,test_password1,test,MASTER_PORT,);
303+ERROR 28000: Access denied for user 'auth_file_password' (using password: YES)
304+SELECT 1;
305+1
306+1
307
308=== added file 'plugin/auth_file/tests/t/dynamic.users1'
309--- plugin/auth_file/tests/t/dynamic.users1 1970-01-01 00:00:00 +0000
310+++ plugin/auth_file/tests/t/dynamic.users1 2012-06-06 03:05:26 +0000
311@@ -0,0 +1,3 @@
312+# Always allow root user with no password for drizzletest program
313+root
314+auth_file_password:test_password1
315
316=== added file 'plugin/auth_file/tests/t/dynamic.users2'
317--- plugin/auth_file/tests/t/dynamic.users2 1970-01-01 00:00:00 +0000
318+++ plugin/auth_file/tests/t/dynamic.users2 2012-06-06 03:05:26 +0000
319@@ -0,0 +1,3 @@
320+# Always allow root user with no password for drizzletest program
321+root
322+auth_file_password:test_password2
323
324=== added file 'plugin/auth_file/tests/t/dynamic_plugin-master.opt'
325--- plugin/auth_file/tests/t/dynamic_plugin-master.opt 1970-01-01 00:00:00 +0000
326+++ plugin/auth_file/tests/t/dynamic_plugin-master.opt 2012-06-06 03:05:26 +0000
327@@ -0,0 +1,1 @@
328+--plugin-remove=auth_all --plugin-add=auth_file --auth-file.users=$TOP_SRCDIR/plugin/auth_file/tests/t/dynamic.users1
329
330=== added file 'plugin/auth_file/tests/t/dynamic_plugin.test'
331--- plugin/auth_file/tests/t/dynamic_plugin.test 1970-01-01 00:00:00 +0000
332+++ plugin/auth_file/tests/t/dynamic_plugin.test 2012-06-06 03:05:26 +0000
333@@ -0,0 +1,31 @@
334+# Making connection using the username:password pair provided in dynamic.users1 file.
335+--replace_result $MASTER_MYSOCK MASTER_SOCKET $MASTER_MYPORT MASTER_PORT
336+connect (connection1,localhost,auth_file_password,test_password1,,);
337+connection connection1;
338+
339+#Test that the auth_file_users can't be replaced with an empty value
340+--error ER_WRONG_ARGUMENTS
341+SET GLOBAL auth_file_users="";
342+
343+#Test that the auth_file_users can't be replaced with a non existent file
344+--replace_result $TOP_SRCDIR TOP_SRCDIR
345+--error ER_WRONG_ARGUMENTS
346+eval SET GLOBAL auth_file_users="$TOP_SRCDIR/plugin/auth_file/tests/t/dynamic.users3";
347+
348+#Test that the auth_file_users can be replaced with a different file
349+--replace_result $TOP_SRCDIR TOP_SRCDIR
350+eval SET GLOBAL auth_file_users="$TOP_SRCDIR/plugin/auth_file/tests/t/dynamic.users2";
351+
352+disconnect connection1;
353+# Test that the connection to database can't be now made using the old username:password pair given in dynamic.users1 file.
354+--replace_result $MASTER_MYSOCK MASTER_SOCKET $MASTER_MYPORT MASTER_PORT
355+--replace_regex /@'.*?'/@'LOCALHOST'/
356+--error ER_ACCESS_DENIED_ERROR
357+connect (connection2,localhost,auth_file_password,test_password1,,);
358+
359+# Test that the connection to database can now be made using the username:password pair provided in dynamic.users2 file
360+--replace_result $MASTER_MYSOCK MASTER_SOCKET $MASTER_MYPORT MASTER_PORT
361+connect (connection3,localhost,auth_file_password,test_password2,,);
362+connection connection3;
363+SELECT 1;
364+disconnect connection3;

Subscribers

People subscribed via source and target branches

to all changes: