Hi Robbie, Thanks a lot for taking a look at my issues in the first place. Please see my comments inline
In QPID-2726 you seem to have changed how the authentication is > performed in PlainSaslServer and yet havent modified the existing > PlainPasswordFilePrincipalDatabase at all - does it still actually > work after the patch is applied? > Yes. I checked on this. It works fine with my changes. Actually the changes should not break anything. Moreover if you have a look at the logic in QpidPasswordCallback.isAuthenticated method you could see how it handles the scenarios when stored password is available (i.e. PlainPasswordFilePrincipalDatabase) and when the boolean value is set for authentication (i.e. when the password database does not expose the pain text password). > Im not sure that creating a QpidPasswordCallBack which isnt actually > used in the intended fashion of a PasswordCallBack (to retrieve a > password from the underlying implementation, rather than provide one > to it) is the correct way to go here. Well I agree with you on this but when the password database can never return a plain text password, do you see any other alternative?. > If you have a custom > PrincipalDatabase needing to implement the SASL/PLAIN profile then > perhaps it should also have a custom SaslServer(Factory) and > Initialiser to go with it that can be registered in the same fashion > as the existing Plain/Base64MD5 ones are and do the appropriate > comparisons for that database. This could eg be a subclass of > PlainSaslServer which differed only in how it compares the passwords > after doing the callbacks. > I thought about this already but had to go for the simple change that I have done simply because even if I had a new SASL server it would still be a SALS/PLAIN server which would almost have the same behavior as the current PlainSaslServer that we have does. > Also, I see Andrew has already posted some comments to QPID-2720 and > mentions commented out code, that seems to apply here as well. You > should also probably look at the following page regarding our > conventions for field naming etc: > https://cwiki.apache.org/confluence/display/qpid/Java+Coding+Standards Will do. Thanks, Danushka -- Danushka Menikkumbura Apache Axis2 PMC Member Apache Qpid - World Domination through Advanced Message Queueing ; http://qpid.apache.org phone : +94 77 364 1754 personal blog : http://danushka-menikkumbura.blogspot.com/ <http://danushka-menikkumbura.blogspot.com/>technical blog : http://danushkastechythoughts.blogspot.com/ <http://danushkastechythoughts.blogspot.com/>twitter : http://twitter.com/danushkamenik <http://twitter.com/danushkamenik>linkedin : http://lk.linkedin.com/in/danushka
