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-0600Date Closed:2007-07-11 19:58:59
Versions:Frequency of
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

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


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


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!