Merge lp:~ansharyan015/drizzle/auth_file_dynamic into lp:drizzle
- auth_file_dynamic
- 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 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Daniel Nichter (community) | code review | Approve | |
Review via email: mp+108864@code.launchpad.net |
Commit message
Description of the change
To post a comment you must log in.
Revision history for this message
Daniel Nichter (daniel-nichter) wrote : | # |
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; |
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.