----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4994/#review10113 -----------------------------------------------------------
Ship it! 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. /src/org/waveprotocol/box/server/rpc/AuthenticationServlet.java <https://reviews.apache.org/r/4994/#comment21418> Empty line. - Yuri Zelikov 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 > >
