> On Aug. 10, 2012, 10:02 a.m., Yuri Zelikov wrote: > > The patch LGTM - just minor comments. I still had no time to test it > > properly, but if it works for you, we can commit it. > > IMO, it would be great to add a script/clear instructions on how to > > generate self signed certificate/CA, create keys store and then how to > > generate certificates signed by your own CA.
Hmm. I will have to try playing with it to find out how to do full self-signed certificates (I never tried non-CA authorized ones). I would suggest still holding off since it will still fail for most people due to the Client-auth websockets commit still not having hit all versions of Chrome (only >140978) Linux Dev Chrome channel is at 22.0.1221.0 (148928) which is modern enough to work correctly Linux Stable Chrome channel -> I don't know. I don't use it. Windows Stable Chrome channel -> only 21.x which doesn't work -> not modern enough Windows Dev Chrome channel -> also 21.0.1180.77 -> not modern enough when I last checked Windows Chromium testing -> does anyone use this one? I haven't checked Firefox, but if Windows Stable Chrome (which most people are using) doesn't support it, yet we have it committed, it would only confuse people trying to use it but finding it doesn't work. I have also had to disable it on my server briefly since I moved back to using Websockets by default. (I had backported Socket.IO for use with client-auth) - Ali ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4994/#review10113 ----------------------------------------------------------- On July 17, 2012, 6:28 p.m., Ali Lown wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4994/ > ----------------------------------------------------------- > > (Updated July 17, 2012, 6:28 p.m.) > > > Review request for wave, Michael MacFadden, Yuri Zelikov, and Vicente J. Ruiz > Jurado. > > > Description > ------- > > Adds ability to login with X.509 client certificates instead of a username > and password. > Relies on the wave userid being the same as the username of the email for the > domain listed in the certificate. > > Patch adds 3 new config values: > ENABLE_CLIENTAUTH - fairly explanatory > CLIENTAUTH_CERT_DOMAIN - required if enabled. Allows the domain the > certificate was issued for to differ (e.g. subdomain) from the wave server > DISABLE_LOGINPAGE - allows password-based authentication to be disabled > forcing the use of client certificates only. > > Patch is a compilation between myself and Thomas Leonard > ([email protected]). > The patch is tidied and rebased version of the original patches from the > mailing list/github from February. > > Known issue: > _Sometimes_ it is has been observed that after a session has expired, the > login screen is presented without the user being automatically logged in. > Entering a username and hitting enter then uses the certificate and the user > is logged in. Reproducing this bug locally has been impossible. (Someone else > can try to narrow down the cause if they want :) ) > > > Diffs > ----- > > /README 1353706 > /server-config.xml 1353706 > /server.config.example 1353706 > /src/org/waveprotocol/box/server/CoreSettings.java 1353706 > /src/org/waveprotocol/box/server/gxp/AuthenticationPage.gxp 1353706 > /src/org/waveprotocol/box/server/rpc/AuthenticationServlet.java 1353706 > /src/org/waveprotocol/box/server/rpc/ServerRpcProvider.java 1353706 > /src/org/waveprotocol/box/server/rpc/UserRegistrationServlet.java 1353706 > /src/org/waveprotocol/box/server/util/RegistrationUtil.java PRE-CREATION > /test/org/waveprotocol/box/server/rpc/AuthenticationServletTest.java > 1353706 > > Diff: https://reviews.apache.org/r/4994/diff/ > > > Testing > ------- > > Compiled and run locally without issue. > Been deployed to my server and client certificates were issued for all users. > Has been operating fine since February. > > > Thanks, > > Ali Lown > >
