|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|
|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....
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
yaya, good point, it was just right off the top of my head
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
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)
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
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.
By: Russell Bryant (russell) 2007-05-17 12:12:11
This patch has been merged into trunk in revision 64786 with some modifications. Thanks!