Summary:	ASTERISK-02949: [patch] MWI subscribe error when type=friend
Reporter:	Olle Johansson (oej)	Labels:
Date Opened:	2004-12-09 09:03:06.000-0600	Date Closed:	2008-01-15 15:19:21.000-0600
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Core/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) sip_iax_user_mailbox_rev1.patch ( 1) sipmwi.txt ( 2) subscribe.diff.txt
Description:	If you declare a type=friend in sip.conf and use a phone that sends a SUBSCRIBE for mailbox notification, the check_user_full checks for the user first and returns without checking the peer (where the mailbox= is set). ****** ADDITIONAL INFORMATION ****** Disclaimer on file.
Comments:	By: Kevin P. Fleming (kpfleming) 2004-12-09 19:44:15.000-0600 I have a patch in the works to add the mailbox string to the user structure anyway (so that it can be retrieved in the dialplan and be used to access the user's mailbox for message retrieval). Would this also cure the problem? Sounds like it would, without requiring check_user_full to check anything other than the user specified. By: Russell Bryant (russell) 2004-12-10 23:09:47.000-0600 I added the mailbox to the user struct as well to make this work. By: Kevin P. Fleming (kpfleming) 2004-12-11 23:34:18.000-0600 Here is alternative patch that does the same thing for chan_iax2, and also exposes the value of this variable as a channel variable called USER_MAILBOX. We use this in our dialplan for users who want to retrieve their voicemail; it eliminates the need to duplicate the mailbox name in the dialplan and in sip.conf/iax.conf. Disclaimer is on file. By: Olle Johansson (oej) 2004-12-12 02:38:41.000-0600 Users shouldn't have mailboxes, it would be against the idea with users and peers... Users by definition only call TO the PBX. Instead we should not check the user structure at all when looking for a mailbox... By: Olle Johansson (oej) 2004-12-12 03:49:51.000-0600 Added a patch that stops checking the "user" table if we are looking for a connection between a peer and a mailbox. With this patch, we'll always send a "200 OK" if the peer has a mailbox in sip.conf. If not, we will send an error message. Sometime in the future we will need to fix this so that we really check the exten for a peer name or a mailbox ID. If mailbox ID, we should check the digest auth name to see if this peer has access rights to this mailbox. And we should also use the call ID of this subscribe when sending MWI notifications. Meanwhile, this patch makes phones happier and Grandstream users will not see any more "404" blinking in the screen when they are not making calls. Happy phones in Christmas time must be a good thing :-) By: Olle Johansson (oej) 2004-12-12 03:51:27.000-0600 ...to quote JerJer: "Friends are evil". With this patch, they are less evil :-) By: Kevin P. Fleming (kpfleming) 2004-12-12 09:42:18.000-0600 I really do not understand this hatred of "type=friend". When you are supporting large numbers of (phsyical phone) users, what exactly is the value of having all the users listed twice in the configuration files? As best I can tell, using type=friend is exactly identical to using separate entries, because it generates the same in-memory structures anyway. If you think that "users" don't have mailboxes, I would beg to differ :-) Everyone of my users has a mailbox, and I hate having to "infer" which mailbox belongs to them when they hit the "messages" button on their phone. I also do not want to program every phone with it's absolute mailbox name, for at least two reasons: - nearly every one of our customer phones has more than one mailbox assigned, to different line appearances. using mailbox assignment at the server end _only_ allows for vm retrieval from different boxes based on which line appearance the user chose before hitting the "messages" button - it's even more duplication, and now it's harder to change (unlike keeping it all at the server end) To me it makes much more sense to just tell the phone when it should MWI for _any_ of the mailboxes associated with it (without the phone having any knowledge of that list), and direct the caller to the proper mailbox when they try to retrieve messages. By: Olle Johansson (oej) 2004-12-12 10:38:33.000-0600 Kpfleming: I think you misunderstand me. I understand that all your users has mailboxes. But not the type=user in Asterisk. A friend is registred both as a user and as a peer, which very often leads to problems. One of them is fixed with this patch, my goal was to be able to continue to use type=friend :-) But a SIP type=user calls to the PBX, doesn't get any calls FROM the PBX. THat's why a type=user should NOT have a vm mailbox, we will not have any calls TO a type=user that we can redirect to a mailbox. We only have calls to type=peer kinds of configuration entries. So we need to make sure that the type=peer half of your type=friend works as expected, not create a patch that will not help. By: Kevin P. Fleming (kpfleming) 2004-12-12 11:30:08.000-0600 And you misunderstood my point :-) type=peer entries for people with phones without corresponding type=user entries are pointless. I don't have any users with phones who only receive calls but do not make them. I don't really care how this issue proceeds in the official tree, I'll be happy to continue to maintain my own patch (along with the dozen or so other local patches I have <G>). My point is that your comments about SIP correctness and "type=user should not have a mailbox" do not reflect the way Asterisk is used in most situations as a PBX, at least in my opinion. I would suspect that at least 90% of the sip.conf entries in all Asterisk installations have both a type=peer and a type=user for the same account/device/whatever, and it makes sense to have them both be aware of mailboxes associated with that account/device. I understand that you would prefer to see all MWI handled at the client end though, which would pretty much eliminate the need for mailbox= entries in sip.conf at all (except for possibly authorization/authentication purposes). As someone managing a diverse set of equipment being used in hosted PBX environment, I would disagree. By: Olle Johansson (oej) 2004-12-12 15:04:18.000-0600 kpfleming: As long as we have peers/users, I think we should follow that design. Breaking that design doesn't help, only confuses matters. Try my bug fix and see if it works in your environment, please. And I do not promote removing the asterisk-driven notification we have today, since it works with many phones. But I do promote supporting subscribes correctly, since the way we are doing it now breaks support for some SIP phones. If it's broken, we need to fix it. I am sure you agree with that. By: Kevin P. Fleming (kpfleming) 2004-12-13 09:04:47.000-0600 I do absolutely agree that the subscribe-model for MWI should be fixed, and I'm glad you are doing that :-) I've never tried to use it, so I've never seen the problem you are fixing... I'm still curious as to what problems "type=friend" is purported to cause... can you or anyone give me some pointers where I can see discussion of this issue? As best I can tell, it is no different than using a type=peer and type=user with the identical relevant data in them, since that's what chan_sip does when it sees type=friend. If no one else is interested in having type=user entries know what mailbox is associated with them (automatically) and exposing that into the dialplan, I'll not bother pushing my patch any farther. I personally find it extremely useful. By: Mark Spencer (markster) 2004-12-13 16:19:53.000-0600 Wait, why don't we just use find_peer instead of find_user? By: Olle Johansson (oej) 2004-12-16 15:00:40.000-0600 Good question... I guess find_user is there for the subscribe state notification. Let me look at it once more. By: Olle Johansson (oej) 2004-12-16 15:07:53.000-0600 Well markster, the check_user_full starts with find_user then goes to find_peer. WIth this patch, I simply disable the find_user part and let it head directly to find_peer without changing the logic of check_user_full. By: Olle Johansson (oej) 2004-12-19 03:57:17.000-0600 Markster, ready for cvs. Disclaimed and done. A future addition to the subscribe system would be to use the subscription request 's call-id in subsequent NOTIFY's. Coders welcome! This would especially help those that are using Cisco phones. By: Olle Johansson (oej) 2004-12-27 16:02:30.000-0600 markster: If you want to discuss this, find me on IRC. Otherwise, I still want to promote this error fix for inclusion in CVS. By: Mark Spencer (markster) 2004-12-29 07:45:28.000-0600 Fixed in CVS head.  Thanks. By: Russell Bryant (russell) 2005-01-01 18:27:56.000-0600 fixed in 1.0 By: Digium Subversion (svnbot) 2008-01-15 15:18:39.000-0600 Repository: asterisk Revision: 4585 U   trunk/channels/chan_sip.c ------------------------------------------------------------------------ r4585 | markster | 2008-01-15 15:18:39 -0600 (Tue, 15 Jan 2008) | 2 lines Fix MWI for SIP when using "friend" type (bug ASTERISK-2949) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=4585 By: Digium Subversion (svnbot) 2008-01-15 15:19:21.000-0600 Repository: asterisk Revision: 4631 U   branches/v1-0/channels/chan_sip.c ------------------------------------------------------------------------ r4631 | russell | 2008-01-15 15:19:20 -0600 (Tue, 15 Jan 2008) | 2 lines fix MWI with type=friend (bug ASTERISK-2949) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=4631