[Bug 27622] Pluggable SASL authentication

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=27622

--- Comment #11 from Sjoerd Simons <sjoerd at luon.net> 2010-04-26 11:55:15 PDT ---
The design looks hackish to me in some places unfortunately :(

* having to dig out the sasl-auth-obj from the connector seems quite bad, just
add a function on the 
   connector to add one.
* needing to dig out the jid of the connector, then splitting it yourself and
passing it onto your 
  handler doesn't seem great either. Better to the SaslAuth pass that
information to the SaslHandler
  (given that most if not all need it).

* You removed the initial_response_func for no apparent reason, i'm not sure
why. It makes 
   things harder as we can't have some nice fallback logic (which is the reason
i added that method
   in the first place).. As a result WockySaslPlain doesn't handle the case
where the server sends it 
   a challenge instead of success anymore (yes it will never happen, but shows
the issue nicely)...

   Also it means that you have a success argument that can only ever be useful
for mechanisms only
   doing an initial response and they all need to implement it in the same
silly way ? 

* Given that we're moving to interactive authentication, you should tell the
SaslHandler whether the
   result it sends will go over the wire in plain-text or not and/or if wocky
thinks plain text is acceptable.

* You removed the ->plain boolean from the Handler struct and instead remove
the 
  PLAIN mechanism from what the server claims its mechanims are.. There are
more non-secure 
  SASL mechanisms though, which should be considered plain-text. Having the
filtering be in 
  SaslAuth seems wrong. In the old case SaslHandlers would just tell when they
were plain-text (the   
  naming might have been confusing.)

* There is no way to differentiate between a handler not being able to handle
any of the   
   mechanisms or it marking all the mechanisms as inadequate. I'm assuming it
would be useful 
   for the primary handler to be able to completely abort the authentication

* This setup doesn't work at all for old-style jabber authentication. We should
probably split out the old-style jabber stuff into something akin to SaslAuth
and have it do things for you. We can fake the old-style stuff as being SASL to
the helpers so we can use the same interface.

* When we're splitting out anyway, we should probably split out the handler
picking process into
  it's own object (which could even look like a SaslHandler to the other
objects just for extra fun 
  and profit. This would also mean your gabble itneractive handler could use
such an object (or 
  maybe derive from it for even more fun) to implement just having a password
given to it from 
  the user and then needing to do the actual sassling itself.. Actually i've
just thought of an 
  even more cunning scheme, but it's time to go home first :) (ping me on
xmpp/irc later?)


code-wise i've got several nitpicks that i won't go into now, but the main
issue i saw is that you're doing _async_result_complete from code not called
from the mainloop, which violates how GAsync stuff is supposed to work ;)

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.