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