Summary:	ASTERISK-08426: [patch] option to restrict manager users to a single simultaneous login
Reporter:	Steven Sokol (ssokol)	Labels:
Date Opened:	2006-12-24 14:13:39.000-0600	Date Closed:	2007-07-11 19:58:59
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Core/ManagerInterface
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) final_manager.patch ( 1) manager_allow.patch ( 2) manager.patch
Description:	The attached patch counts the number of sessions owned by a given user and prevents the user from creating duplicate logins.  This is primarily done to prevent duplicate logins to the new GUI. Someone may want to create a configuration option in manager.conf to enable/disable this restriction.
Comments:	By: Brandon Kruse (bkruse) 2006-12-24 15:13:47.000-0600 Thanks for contributing, but im not exactly fond of the idea of this being enabled by default. I run alot of tests, and monitor tools through the managers interface, where this could pose a problem....... I think its a good idea to have it toggle'able for security reasons......Maybe. By: Russell Bryant (russell) 2006-12-24 23:26:15.000-0600 It absolutely should be a configuration option, not enabled by default. By: Serge Vecher (serge-v) 2006-12-27 14:11:26.000-0600 also, please make the new patch against the trunk, please. By: Brandon Kruse (bkruse) 2006-12-27 16:20:59.000-0600 we might want to also correct a minor grammar mistake astman_send_error(s, m, "Duplicate login deined."); denied  instead of deined also, is there going to be an option in manager.conf? By: Brandon Kruse (bkruse) 2006-12-27 22:16:32.000-0600 apply'ing this to trunk did not work (obviously) since the lines were very far apart......but I have cleaned it up, and reuploading the "applied to trunk" patch. I want to "attempt" to make this a configuration also also fixed the small grammar mistake By: Brandon Kruse (bkruse) 2006-12-28 00:04:59.000-0600 ok im done, I have patched a couple of things up, and added a configuration option, and put it where it needs to go in trunk.....as the patch would NOT normally apply.... re-uploaded oh ya, added a configuration option for it to allowduplicate = yes or no By: Serge Vecher (serge-v) 2006-12-28 08:44:05.000-0600 should we name the option something like "allowmultiplelogin" or similar to reflect that it allows more than two sessions, as would be presumed by the use of "duplicate"? By: Brandon Kruse (bkruse) 2006-12-28 13:11:37.000-0600 serge-v yaya, good point, it was just right off the top of my head reuploading allowmultiplelogin By: Serge Vecher (serge-v) 2006-12-28 14:06:24.000-0600 alright, almost there ;) +static int allowduplicate; int displayconnects; /*!< XXX unused */ + int allowduplicate; /* allow or disallow duplicate logins */ By: Brandon Kruse (bkruse) 2006-12-28 14:13:00.000-0600 ahh! I think i just uploaded the wrong patch hold up :] By: Brandon Kruse (bkruse) 2006-12-28 14:15:21.000-0600 ha it was thanks for the eye serv! By: Brandon Kruse (bkruse) 2006-12-28 16:58:36.000-0600 if you could delete all 3 of those patches, so we just have the new one, that would be great...... By: Brandon Kruse (bkruse) 2006-12-28 17:03:19.000-0600 added a new patch (manager_allow.patch) against trunk tested it out, and it works By: Tilghman Lesher (tilghman) 2006-12-29 01:54:37.000-0600 1) It does not appear like you're initializing the value properly. 2) Rather than a simple on/off switch, this may derive more useful functionality if it limited the number of logins to N users (or no limit if N=-1). 3) This should probably also be a per-user attribute, rather than a global attribute. By: Brandon Kruse (bkruse) 2006-12-29 01:58:07.000-0600 I totally agree corydon, i was thinking about it earlier. I wrote the patch to just learn more about asterisk, but if you take a step back and look at the whole picture, I think we should, if anything, go for a total number of managers logged in ( for system resources, as suggested by someone else ), or a per user basis as you have suggested. even then, I do not see the big reason Thanks again corydon By: Brandon Kruse (bkruse) 2006-12-29 02:00:05.000-0600 i updated with at least the patch to get it to build, i dont know how i did not see that mistake. By: Brandon Kruse (bkruse) 2007-02-26 15:28:45.000-0600 Hey, I have fixed this feature and tried it out AWHILE ago. I am not sure if this should even be integrated into a feature unless you guys want it to. By: Serge Vecher (serge-v) 2007-02-26 15:51:26.000-0600 thanks, bkruse. We are marking bugs that have passed a preliminary code review with "ready for testing" status to make it easier for prospective testers to find patches on the bugtracker. Several successful test reports are needed to help make a determination on whether a patch is to merged into trunk. By: Brandon Kruse (bkruse) 2007-02-26 16:25:46.000-0600 Awesome, Thanks Serge-v If you guys think this is a good feature, I will retest it, and re-write a patch to the current trunk, or whatever is needed, let me know. -Brandon By: Russell Bryant (russell) 2007-05-17 12:12:11 This patch has been merged into trunk in revision 64786 with some modifications.  Thanks!