From 87ff939ad2ee37028e1a84f9c39daeb301a08463 Mon Sep 17 00:00:00 2001 From: yackob03 Date: Thu, 10 Oct 2013 12:55:03 -0400 Subject: [PATCH 01/10] Handle some of the error cases with github login. --- data/model.py | 62 +++++++++++++++++++++++++++++--------- endpoints/api.py | 7 +---- endpoints/web.py | 10 ++++-- templates/githuberror.html | 29 ++++++++++++++++++ templates/signin.html | 2 +- 5 files changed, 86 insertions(+), 24 deletions(-) create mode 100644 templates/githuberror.html diff --git a/data/model.py b/data/model.py index d6d3c9154..1a6e91f1e 100644 --- a/data/model.py +++ b/data/model.py @@ -15,18 +15,52 @@ class DataModelException(Exception): pass -def create_user(username, password, email): - pw_hash = bcrypt.hashpw(password, bcrypt.gensalt()) +class InvalidEmailAddressException(DataModelException): + pass + +class InvalidUsernameException(DataModelException): + pass + + +class InvalidPasswordException(DataModelException): + pass + + +def create_user(username, password, email): if not validate_email(email): - raise DataModelException('Invalid email address: %s' % email) + raise InvalidEmailAddressException('Invalid email address: %s' % email) if not validate_username(username): - raise DataModelException('Invalid username: %s' % username) - if not validate_password(password): - raise DataModelException('Invalid password, password must be at least ' + - '8 characters and contain no whitespace.') + raise InvalidUsernameException('Invalid username: %s' % username) + + # We allow password none for the federated login case. + if password is not None and not validate_password(password): + raise InvalidPasswordException('Invalid password, password must be at ' + + 'least 8 characters and contain no ' + + 'whitespace.') try: + existing = User.get((User.username == username) | (User.email == email)) + + logger.debug('Existing user with same username or email.') + + # A user already exists with either the same username or email + if existing.username == username: + raise InvalidUsernameException('Username has already been taken: %s' % + username) + raise InvalidEmailAddressException('Email has already been used: %s' % + email) + + except User.DoesNotExist: + # This is actually the happy path + logger.debug('Email and username are unique!') + pass + + try: + pw_hash = None + if password is not None: + pw_hash = bcrypt.hashpw(password, bcrypt.gensalt()) + new_user = User.create(username=username, password_hash=pw_hash, email=email) return new_user @@ -35,18 +69,16 @@ def create_user(username, password, email): def create_federated_user(username, email, service_name, service_id): - try: - new_user = User.create(username=username, email=email, verified=True) + new_user = create_user(username, None, email) + new_user.verified = True + new_user.save() + service = LoginService.get(LoginService.name == service_name) federated_user = FederatedLogin.create(user=new_user, service=service, service_ident=service_id) return new_user - except Exception as ex: - raise DataModelException(ex.message) - - def verify_federated_login(service_name, service_id): selected = FederatedLogin.select(FederatedLogin, User) with_service = selected.join(LoginService) @@ -98,7 +130,9 @@ def verify_user(username, password): except User.DoesNotExist: return None - if bcrypt.hashpw(password, fetched.password_hash) == fetched.password_hash: + if (fetched.password_hash and + bcrypt.hashpw(password, fetched.password_hash) == + fetched.password_hash): return fetched # We weren't able to authorize the user diff --git a/endpoints/api.py b/endpoints/api.py index 0e5a9cdd5..2c6dcf6a8 100644 --- a/endpoints/api.py +++ b/endpoints/api.py @@ -72,13 +72,8 @@ def create_user_api(): send_confirmation_email(new_user.username, new_user.email, code.code) return make_response('Created', 201) except model.DataModelException as ex: - message = ex.message - m = re.search('column ([a-zA-Z]+) is not unique', message) - if m and m.group(1): - message = m.group(1) + ' already exists' - error_resp = jsonify({ - 'message': message, + 'message': ex.message, }) error_resp.status_code = 400 return error_resp diff --git a/endpoints/web.py b/endpoints/web.py index f253264b5..6f170cbdc 100644 --- a/endpoints/web.py +++ b/endpoints/web.py @@ -135,14 +135,18 @@ def github_oauth_callback(): to_login = model.verify_federated_login('github', github_id) if not to_login: # try to create the user - to_login = model.create_federated_user(username, found_email, 'github', - github_id) + + try: + to_login = model.create_federated_user(username, found_email, 'github', + github_id) + except model.DataModelException, ex: + return render_template('githuberror.html', error_message=ex.message) if common_login(to_login): return redirect(url_for('index')) # TODO something bad happened, we need to tell the user somehow - return redirect(url_for('signin')) + return render_template('githuberror.html') @app.route('/confirm', methods=['GET']) diff --git a/templates/githuberror.html b/templates/githuberror.html new file mode 100644 index 000000000..05370326b --- /dev/null +++ b/templates/githuberror.html @@ -0,0 +1,29 @@ + + + + Error Logging in with GitHub - Quay + + + + + + + +
+
+
+

There was an error logging in with GitHub.

+ + {% if error_message %} +
{{ error_message }}
+ {% endif %} + +
+ Please register using the registration form to continue. +
+
+
+ +
+ + \ No newline at end of file diff --git a/templates/signin.html b/templates/signin.html index 85c63940c..4b14b6dcb 100644 --- a/templates/signin.html +++ b/templates/signin.html @@ -17,7 +17,7 @@ - OR + OR Sign In with GitHub From e016d5822f7025b2ccc7c66b8644a2af4f762264 Mon Sep 17 00:00:00 2001 From: yackob03 Date: Thu, 10 Oct 2013 13:44:07 -0400 Subject: [PATCH 02/10] Switch the email on one of the test users to allow for testing of github login. --- initdb.py | 2 +- test.db | Bin 56320 -> 56320 bytes 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/initdb.py b/initdb.py index cf795a0a0..435eca603 100644 --- a/initdb.py +++ b/initdb.py @@ -72,7 +72,7 @@ if __name__ == '__main__': logger.debug('Populating the DB with test data.') new_user_1 = model.create_user('devtable', 'password', - 'jake@devtable.com') + 'jschorr@devtable.com') new_user_1.verified = True new_user_1.save() diff --git a/test.db b/test.db index 6a37359429329994e59cf7ed588acf89ca4071b6..f23bc85257bc5ad86d63b9725347f9292629b80b 100644 GIT binary patch literal 56320 zcmeHQe~cYRec#=AckiBm#<9;Y=d;QAoWwao{C0MCc6LA^aqPerLYFKHu4SM<0G*?u?nr78Xy*GgF~caa`BAYii1I zoKN8&$G^_!7MxT%U+}-%cYd+cPdT@LfA0wHj*{=&GPjZ6kYABskQd00S{E+%V0pdd zK*@pEhyyoIp`sgi;c&wZI9z`{4jVS&ux`vAUGlPnzvZXoK*@n?fde&{I1`W0ojGwf zT?cwy@+${_%TLLHk^|QS2Wn)Z){=i~{|Wg4h>7_sH8F@);b;PsxFj1DDBx)Ejdqnj?4AoYQC1lXL2(K;A@yo7DXu)~D_}liefd zegA#Omc+qF7Y;o*d+dpZ&+l$L7M)?{zJo{P{C&{@ao@v@a|iEv*Ri=Lq*_S#omyBr zVdkG)xa;v#a_;0lwQ%b9+%acYbA;6#ZO)yM=}FUWC41I3^ftbNMQAAV?G zeBZt&bUgndnjCx2{r8^w5IZ3=?H>#u2+uFPd;Y?yM-M!)q)sd>F5cCt%eG+czej%J zke6^MKP3lB4y;WMtRoX{OHq02$*x*Q2ejn>>kj!k`SIE$yrin+z%{{vo#YPpVvdAY zC#-gH`~QkVeoJ1tCb(1*S#qG{K!rH22VAgb z+-m*io89KUc~En5;qkfoC9`;LPMQ95@4%7S{l{jfj_tquf!V3){#(;iw@%OL>8bk< zADg{*_Q=%X2aio1e&m4%rXD(S|DpXy9-Dgi>|<}6nx2=Z%=FZI_8&QL-~J=F28_*g zOU|71s`Uo?uw`yun+yGwAGeS7Um)#{JaYK{haZ_mqX%anE%qmWWP0ks!^O+Cn|U8* zo*>ow__*8TImL?$Cyo7ADN<-Jy#|yWtc=!g9CuYtn5{GKojn<8x7Vhid#}~=PV=oE zH`PeqCEEXX>;7pnf~c8^XO=`z8B--=FGBPJl^T2{l7YN z|LI*@?Tih_xcKcS$oNWk^@%|2dw;$IL|rGbM6->#^~HW7r(79nx^8-S7D@bE3wS8I5KY91Eu`GF?=ptDLHT@a-hWjuf$O)`&4q^jp0BG|9{DGUh;N&pCvaDqypCZ=b4)~ zA@kpGpM5INDCFr?wJ@K}**#F^7Uq{-*wTh#+v^aP-M`G#f?CiQTa3^L$u3{guu|gT z^!ek9X2~FBFf8ev_w;z1w&6t--V8xDWqRr$R$3gp|IlnN-OzM4Bj3<)n>Xk9^)j+i zu{O)DZXJqSFDsTm+ybkY-Lw46nKje&u0!epU1+5qmX++JFqYle*6u}np~JHKod;Lw zK^G(c$d+n-_ind&AfGPh=9cEtxs!8ep6Z>A7bi&X#%0co6;?&eeBk=g`fa=2r}HuB z)toQs=zZN@q_=mYZ(&OBqU}~|kw@wOuXSx#(pPfe;v6XD|KhwU@01)^>m0EDfByd8 z%jNrjYkjOs`mSLPti$FXlcyKw&LP^%_y20W3~24YM}F#%pO!m-UBk1ZBz?_uU_F_1 z+w$+({l8lBZ|#2tpda}O`3CvBYXSXR^8;NHxTZKTUKw?EO}O11K$cF(MWem#WYn41 z-PwGkCCrQF^ul%z#V32kvB$^hljedq?haHlXRmu!{zsi@$C-9_xS#ZPlDCpi7TMQ4 zyETVDpJh4xY1>$uJF~F(RPU@_gg<*XUIqNQbXuwv(D4-zQefvjWb@K1@Ox$3z023% zYqO;Af|!N>r>Bn1K6-5C!nSJt4xqj_=D=%@rsKT|&{*&KL=l1Q-6%fL0-SqyZ6^vf zV%_-2I6!YS%?F#dT*w*KdZDj76g%2&7uUA;8p(_I9vs$(Zq;@ndkY(WX8rbRea{~E z*$?I;FO@P&OJ^3IH1mC@lf|*`yK|X?|0=U0D1-)dbNK0 z9=EwMACx{i^GrtH_nkurqpv_;>Z9+f?M5r<@x%?)dWe?RFKeltlIVMCS8>+$-6>`` z`fk{Uy15PO|5vR4|8wlj|1wU>PsxFj18a~2BgA#RN`)Z!&riJyacf>n{=evue*@*$J$K&0dzn1a< z!218?`+sX>dX!|89C(!+u=c-{|5wShvUtgXHNt^X{@2JrmSmJ1_#HS<%Kz`cwDOUX z18al>HvS)RKH)f@a5s7H@cx;6@XaOcBNIq{A@|XG-=i;Uf!AC*iASW}`fj}C;u(;_ zBdw%Y--9*}QYx{Y2o*&3{o;Hi8=9ZR_9qc{PyEawKfekCtPBJZjxu>{a zX(unUyj*|o%9d(Zm?tlL4PH9^*(+P9!EO?I>6}?_d(xSyndYX+YJHZu&Bj2Qvd5U? z`39r?S<>FU-NkY1^{ZE;^Y~!CvR5J>!*<`%p`DlPP@BQSTARL%4t2-JZU;8BlK$jc zb81(${xCRn?+P61pVq@_+;bV->OGea;!2$>7R~qn93lVekYD3hxqnQ)htu*?a-ihE zRn391krBtW={|3?GUC+STBS0sPTbO?cV) zk^?0NmUAG__>b7+e`^JRJ;VP09r^!ZOa5Pg5Aeg~50_U;4wM|YnmN!X{`n~xkbnIC z?{D!7;6KM(K+j*zbe4TDIZ$$7kONlaDhrU$Y`Yo4Nz%)B_#jqVJ$G?e~bSU_iK*(HSZ6}Utg6}UhAE1-u;$p zT>@opZ4uQ2czZB;=UtFu5@lG)o7!me#oi>kRGlkB^&R*euNd_jw~+PBGdpjr*6Vfm z)Ds3-9wSotG4F5dscU)^oa&=+;Ws3(`so+|$C zioG$oIhwt;aOzb4iy-zbv1Np;oVe)`-Jg7s^LVieZ`3XI+K(d^_WxGhd#A7oSiQXo zSc{7j47cN2g~-AUziU!zYFh@GBqyFO(SIwXRZ1`qv%@SRFJ) zTXCS9aqunj-x#sSo!$lO|9^*k$onGsC3XV%E;&IyKn{_2kXy(m@0Z@Uy)S#8^FHnU zk#~Xo3;9R#m*kV=Joz~G3wWJ>ClsA4KWmZ$TgYCw5k!rEHG;Si@W#$fWOv_{NxNM| zyTFbyGM=ALjF8Q4o#cApsYdT2IUQ!G^l6Fg zPQqcgwrwM~FT1`qC!xO^*XJa3uWZRlXnnmdCjsZ1a}qXfAiLZKYoPajW8-GBr~8G{ z#)kfz>uaRmy18y6xv_P|8%d*Mp_+c1dXs!$;*H}4U(6k z4ZQ!ixz@<#AB%qM?|--Qf8@W&e~@oO&i{#gjr=|N8}cRcdGa~(S@LP}r{s^xACM+_ znw%vIWR9ped$rJ-6pfOowZefMa@(07C-dWu{0KFMaQp^)B=!8bJwJ};$8Gs>Yks^w zKW@p7*X2j(4?MNW9=(nEaYKGwpC8xd$69_I%a7Ik2z`L^!}5O~cdUa_1=S4U7pEQW|&q_3yj4!P|s|ev_`dbVfctH~;v0Z#@?Lq3pR}pO0NWu;_2x zt=R`F8?f+i!mYKMsI>O~ddzWO@jmGNpZojHubszU6QY~DZmHG}?RA?+2i88LgGCs6 zU@t4&-**WRdUzFs&@)Qq%ftH@Sob!!YF2Kp)(@b6w_U1#JEy z&fbKd*g!8@Kj~ept_9-uubxK--nLQ)I!|1r?e6Uvy&GFh)jL~EwXXLX2Uwn8y0zHW zkYWSCa-j-rWcHTnYW*mhIiB-)$n(bsF#5d6%+TS#jHg%Z4WRmg#?Zl!?WxxH?scEN zW8i%cFuF$zzPz>FrFIsgaPKm+5i9Rt*P`x{Z(!ub`f~@pp5z&h6@Rq8SGHS2{`=o! z|m;bEKm;coAwm|{ zUpTP-KSO5kE99Ga2k;BHP<~1dlpJ`|Ik2JPPPmm;us)W5A+0afbrpBq^=wE#nqT)? z*GD#1+)3AK;{cUbDT@M-TBSCgV+Y>4N^PPI0JPr!4;^xs_fhvN&Pz^sNs^oI#HxYj z#^%ia-R|ZrFj=a&qKQ(HMX}E{j{?E`G%;NIxED#8aIJi;qf}?15;BQ>6`4#2oF`}G zA{JiVhP?~}7Qa1BLR)}w#-+)45Yxy{LYh$?NtGl#5F(VJ6f};qjO#eku|z#_5QJe$ zRUG&uc>r!ti-6jsPfZX>PG!o2h-bcxK#ho! zScM#&iGxT4eh`QZU6)Ca#ypc*A|%k+B*;uqBmi_trhG<2CW0hPL=vZ2CQT%@Qk2Gu z3z|h?LVe*2^gRYgOp0I8q*_W5n4ttjGzwXdfQHTDhK=Ta(xsvvTO3%?ko(G~ocf#v z!tg|xEJ-7!m|#MQEJ>7LslnPZ6M?iaqax!p4MSa|0?h?!7-3X_FZ&XM82ViCFwnjh zG6p{F$1;vUTo_Bq!dU6ZS6NC0c6l3I7m;nn<{Vedg@AELGCe7l^=p@NRkp>ooOo}wlViWnX&q9qU z6Pbic!$U!n$b^_nK}chcnTJLhWdV;;W-^vy0Ef~*gP8HcNHp(oyA*6zEM^gpbRalt zh$1fIL`u%`8J(H{GfM_B=Br4fM5Re2vNVaA3Z)X5QD%UG(9a^Fc$!KTCQ|uQ97ZD8&6BW4qL;6; zXx@WYo!d(5#?@v`ibSA8Jk7#5v*QfSrV^SYlAFXw7ADr5W!#UUxr|c2&x~P)3y}m0 zvsW98-cTYU-w&4&VHbWh4~}&SnZyKFSw^+az#@sdQl1GN7${vG3e6ajLTE5yN_Y$l{A3JD#$A20RP7Vy=PGfg^97p zoDs3eG>sv@MKWkA1@u}5?F+>aXPKQ$B2!#)O(P!o(84}cI?W(OTEz+Y69)-&VVdfc z50!kV)Q~5GWHj#@?GmvyR)OYR2AM?eEl~!B#ZQ?JhC&_$w3SI?G!0`!jZSUvQ;mdQ zz*8Coe&QF2fXT{4=$kaqP$x2l(iKW&e(GCp8yG>AX#sl?QksC6$nZE;KIT{&K<5m% zglIDy7>kztPdOy^iaLJ2!P{*sNgJD6}w46{TFH zA%mm}p%P{iNhypl#>5XY9s0PB+I*vB6r_PQAh0Aw8j>tajD~XK(114g8e$-lI6&pd z)h2j~2FmaNQ_sZ6x@w(3Md;LrVvB?ss=$&6)1_~&kw$ZNW0!zk@Qh@tf+&H85P_zU z5FV#VrZL+>Xk4mb>8Etlwl|YR7GUbJpZq55Pbm`aworT;sgR+DK2Mlu^ z?w}HW7=$tgH9k!<2#D73#2B<3Y&3k46wV+lA&kZ=r#6tT_F^Ejghj002iDU2BWz61d4J}8kK9U5lfNzMf~)@9JtH+Nz*w!vUujjdT9=!&s#&eq>&;rCYO8fU+v`bI zYs5y|^{iG&ZEV-GT6c3@yPnlLN@JaRR_Uo!+x4tgW2|r2vsxtzee7U=pbL(B!TpCd zE&ErP@@kLYYT=WSRz1W=j2JKyB>HgnGeHdF`8H-{t^gRB3R!L71Dsp#5&1-4BH$r-?~PAz*2;M82}SF zLR&la#2l^%dyCQZ;3o)ZlO9D5^;5U_vsPCW|l zKb#yJw`V33a1K)gpFZT7W^ghR1urU#Lqs{K5&=(yWp40d5FA?{OI+Ws2h>EUAVPcr z-!k*5b+Le005*vt&_@~8&e;$kjx}QBlt=K1;142{guj#d6hZA;rg7W8= zSL{tF)jYJd>zUoY+WA~`GLsCT4v-zV(}>yu=tY4}fe0alv0fh{6)v>_N)bSeAV&sq zCW3C-sJ|d+93Wp-4|aW57m91VB$CqF@|Tj7eYz z5pY7zDW+r0;8P;%_JNTp4bU|OkO=@r0%Orb>G30k6g{W8NKfm;vnRJEX6*;7|6Cxe3c?phKNGZL7;>h2*qTqphlEVtey}tA_}7e z3t9kHLKrRpx5y-dqEK;)GbJp}KoNK%tTc%p9*Q9d_5Gp5G!I_arDRgBQk;v!0{B^m z0AnZu3hZ>te2fV|WlIwS0BJFdlrtq$ps)$_l}@A3A{&SUk{FY1*svr#hT>gLNlX5p zbI9%9d)<#Z&pD60HX?iIwW$WRG#{&X89fX1X;pTm$Y?>K&NE;`8ulxVV1R-I5DkDv zpaTHwWQaf!F+2t%OEQ4}SrRb;8N^C$paetFdcmO%1`&vUdG3Ext@)nqUE1zktqm{I zMnxiHSXYkN9@!UPz`}tXi){dslo}GGVLn+Xbpl}B*f<0MxQ6?HSaop(*~d>BlN9zjtk#OFZ_ z&xJAA+ackxfbg(O6V`laTbHVEwXQ&sDq|rH1Ly)NGhYEO0_1^wgbxMHbPTr$pgvqI zxC|DS(H6k8ZWjE02n6lx(vV(u+bvD45kvk zR;YpHAy%X zDZF6Xymzuo%x`=dge*xw5}a4Szk`DZ zVi;^!pTh1Vrg?Bjmy$_fT8gDY0`UqgY%gFWggt>*8zD0R9M1qmLLM*91n3BuK%nOhF)7K`;z^ z0wqWj1Vbok?w{zAF^)(CUChvYnjp~#SiJGGZe`yc;iC{ z1a3#vmxj#;r1{Ppx-@J?I04I?u>`4A0UrYyOJQP`gs>-xhK@q2j4SvYIzzG}q+uLO zq?bV=-ttMqf!r9Z4sx*YzCz&0ActXMG8QYQ5aa~tC^VMj+@{kd=9Yp-q>vtCOee~R z3=O4$`f+~-ppxT%quxW#8b>4(XMB;`hK!q&uZ1w SM7y5Vs)`#r^{mn;@BDwK+G833 delta 7255 zcmbuE&99tQ9mntOZEIU86k03_r4*akQQ@BVml{kEsX~DYffz_!oL8XOR-hnme0t-5 z!0b#6F>$ZL1`R?47bcpb0wj<4@WnPo(cZ zbpK=De)PGTpE-KuTcX_a_&r@bb>jHZ>D&8nc<4K;+gA2%Y+2vFQrq*VQ~qu{sM0~H z4*CbmsYhOT@cGBSd0akl`WgSk@w?QiW5>S!_`~<<$G!T&gGXOTkIGXrE{&STto{tS?hDKUfGsoaeP)O=~B5``Rbx|F$(A7&X<37=bnM@R<%WwTItF- z)msrlC*ex<(zX(9QQa4A8Iz6K_mrLSR!h%MTTZ8Eo_+4bm!5qhDeDf4suFkdRtVc` zk5xD+`R=BK?nSufQsbLN5vrw9t&@XS-BmAA6tPC5gw(f5!GE_1aahO0Lfvk?kxHLD zbxS#Q;zT=ndei#GU7L5TZ0y{a+<9R8jVifbU3STul%rET!+ITyHYTeWlQvGaT(zw= zhY&~QPibr27v;$;CxD6)kK}0a+!BkO2?m| zs%@z|nS=MH%OJJwM$W&$S<_t5O=vpQtLHFjy-U&b*n=~aT~{OXcyBL@)Q5V_RahNV ztmkHBQ4^GkVS4h{`vzTgO)~iG`EWP3%HWi4%Gp2@OA0wCla$m+M=vx@U_hBcn#`4w zo4;%upJ-y>MUhDnTE}Qg);&6#l*-BbSbOSCC#6$}Zs5S)jgBhx8nazkZxN;Z$iz_Z zjXPF97@2EwE7Hlz6GTcyWzVgJq?#gBUZRzbvQolvVua_pC9+o1(C;v0>FoR)d@;I8 z#ci?)))eMM%)M1QU%h>LVkwlhxm{^7)nN zw$}~~KC-8t84V?|TP8^%qUO6(P{C;W%X)^win8wBa}n!9q5&@0BFaVgr(w!?!#dk2 zQ%H1JJKv3=E5%9MqtrTkDQ)3fsna3fl5#F6%~*HU7xlo6^E7?`wVMZ4LP*g#rD!2z z;K@`YnBbaJC2&N^jScbOh7n`AiqIl7z%VpX+eO6&K6UKcft10g5E8$a_|t+n9-!#f zm%#jp=$zNWO?a;W+(fM$NxK(Jx6?XEJN?OT4h)eRRCYdSpD9FOHL_B!c<*x_mr&Ty zvgxL^Zn$4;f;tuD0U^a2&EFQhG`<2A>_G&4Q!}k3nRvGn=Mda?DT>T2C}Y(6L{-37 z$)0I{r$CEtD_JZz?XT}2Oa+ty3@J^G-3PAZgRsm)E7U`_RvZW`il@jiMavX7QyVK* zo3>{DwhnBpLfLKs2pT9_&bAq8Wn^%8KILs*X|`MdfT2#D3J!2$Oq!KyA$&WbgwoU7 z-q<(5WyRMRRMEMK?jSm9YdSw+1X(;An2KQgJ2X+X0B&XBGZaBf*XM657y=99!Aq_O zy3)Ipy^2t}>(Zu{xpvXTooksWaS~i{G?bR;-uhyw4uMWg|MSLI1`sy0z!kE#JVHV= z9N1W5V`{Y-cr&2pxGBOk&;xj*gC#^>7EGGItqZx-EV~LN@)YmjTs|MPR&X|mlz?Xz zw$U;hN|8g18vYJIGE@#9ESLmurStAXlvrDrW_{zUS6_g-)oZR?aSZ1r$NVcq2RKm( z78s*jYFs~yJkc;mtG*e!skDT{;GF^*CY~o_>lWIVol&@y-w`}g%{^k{?wvbUhVul! zy6*~v&YaGfe?egWXb-G|(n_Ae3L`z$UPG%)ADTb~3GBn|T2@d5R^Xk+^`xR<$rn9A z8Wr92u{UoSqVicp5R;jJYB|tZ5%~;^0uCDQ;7bVJqNh?+0It+Z)UJID70ow)TMHbi zz%RO{SV}0?vKy^82u921P?6{^U3$p6C|Lba%qgP57@-8pF2)aDW2Oh*Iyl5;x^2z~ zz!`dA*};2lp^%~=vT@IWQ)LmJ6c9#|-imw5$!CX%{E} z0cRVfQZNE#R&5m!`vi{aC^PL+?+DICrKMKUyXn8*x^?xp(_O#&$FQ8mLcm>sn2K3sL- zoEa|!O|v) z2hCs%f(>L3*qc;KHKiqVq0q&$>d80jOhK6sR)#828`obrXw+Y$55*So+c+_Y{wjbf zD$*gh1jY;>ToPUkm~+_zYtkgZu_86DotJoC zbM6K12)?2FF<7yS&&&?O^NpUd*OpU_0!}Czu*^!L=<%8fcy?)&px7WannJ;^AoL1; z2vNOfc20(O{a`b;!&+UWF$lSc{6iR`;2<{3SAgoGCUh#q3_zD@&2Bz;mVGqSm9QLYX3{iuPc(0_>RsAu`;M+7|3bx$eb3{%xoY!9fuX_F;S`KTXJ0BgXdSFFz3QDChmCu$`#>#yVVMGq@1MVVs;O#~ zBt0#vU}btuh@jMsY671h`AdhpAf7?HW`o97qbR^DC5^pbD5n`~L?c(fVS4K9&H>r6 z)RxWPnBioy=^814z&ClsM#ZS9>O(;l&;`}Ugr@)x^g>NiX*)k&;EGp^oM`mIrxPDb@})-f9qfya+*F{V^bm$ z0q+bdpT!ISInb9oXhpz7oC15C3J?aHXVkfpadp zF=-je0OJ|(g~&>hHC8SYaTwIxqJMiSE|*ku9j zI1LG2AOs*~BLBv6H}BvU$Cr^8S2@9r5G&m4fMrN<5*g}|K7l6X)lYiYmFiVDMK+L7$)0mWubZ;bLCClt$ zB2EnCLuA^dp~O}wH&PZ7ZY;4g7uB}#3PM09&~4E~5SNgPJlGQH5XX%G z--s+NqQZ|#cD}Zy5?2t6lNLD#3s5WK!pOgYG@49A(}gDCDWrOIhT2RV|878_iowSz zw}qKeL}*z3%KzRko0b+oyU$Cp%~$wrRQT-OH_Q%NcK2_ZopX5`Ieh#|-Z(pG*=70M P?3{&T{N+zD$DjQ_$xo+r From 16ee147eae94d69527575eb89c50e2279c42dc6d Mon Sep 17 00:00:00 2001 From: yackob03 Date: Thu, 10 Oct 2013 13:44:34 -0400 Subject: [PATCH 03/10] Add a form for changing the password and prompt the user to do so when there is no password on the account. --- data/model.py | 10 +++++----- endpoints/api.py | 29 ++++++++++++++++++++++++++++- static/css/quay.css | 9 +++++++-- static/js/app.js | 3 ++- static/js/controllers.js | 30 +++++++++++++++++++++++++++++- static/partials/user-admin.html | 25 +++++++++++++++++++++++++ util/validation.py | 2 ++ 7 files changed, 98 insertions(+), 10 deletions(-) diff --git a/data/model.py b/data/model.py index 1a6e91f1e..855e85a2d 100644 --- a/data/model.py +++ b/data/model.py @@ -4,8 +4,7 @@ import dateutil.parser import operator from database import * -from util.validation import (validate_email, validate_username, - validate_password) +from util.validation import * logger = logging.getLogger(__name__) @@ -35,9 +34,7 @@ def create_user(username, password, email): # We allow password none for the federated login case. if password is not None and not validate_password(password): - raise InvalidPasswordException('Invalid password, password must be at ' + - 'least 8 characters and contain no ' + - 'whitespace.') + raise InvalidPasswordException(INVALID_PASSWORD_MESSAGE) try: existing = User.get((User.username == username) | (User.email == email)) @@ -210,6 +207,9 @@ def get_matching_repositories(repo_term, username=None): def change_password(user, new_password): + if not validate_password(new_password): + raise InvalidPasswordException(INVALID_PASSWORD_MESSAGE) + pw_hash = bcrypt.hashpw(new_password, bcrypt.gensalt()) user.password_hash = pw_hash user.save() diff --git a/endpoints/api.py b/endpoints/api.py index 2c6dcf6a8..db8355b54 100644 --- a/endpoints/api.py +++ b/endpoints/api.py @@ -51,8 +51,35 @@ def get_logged_in_user(): 'username': user.username, 'email': user.email, 'gravatar': compute_hash(user.email), + 'askForPassword': user.password_hash is None, }) +@app.route('/api/user/', methods=['PUT']) +@api_login_required +def change_user_details(): + user = current_user.db_user + + user_data = request.get_json(); + + try: + if user_data['password']: + logger.debug('Changing password for user: %s', user.username) + model.change_password(user, user_data['password']) + except model.InvalidPasswordException, ex: + error_resp = jsonify({ + 'message': ex.message, + }) + error_resp.status_code = 400 + return error_resp + + return jsonify({ + 'verified': user.verified, + 'anonymous': False, + 'username': user.username, + 'email': user.email, + 'gravatar': compute_hash(user.email), + 'askForPassword': user.password_hash is None, + }) @app.route('/api/user/', methods=['POST']) def create_user_api(): @@ -60,7 +87,7 @@ def create_user_api(): existing_user = model.get_user(user_data['username']) if existing_user: error_resp = jsonify({ - 'message': 'The username already exists' + 'message': 'The username already exists' }) error_resp.status_code = 400 return error_resp diff --git a/static/css/quay.css b/static/css/quay.css index dc7c2fade..0c7a33c1a 100644 --- a/static/css/quay.css +++ b/static/css/quay.css @@ -137,11 +137,11 @@ margin-left: 0px; } -.form-signup input.ng-invalid.ng-dirty { +form input.ng-invalid.ng-dirty { background-color: #FDD7D9; } -.form-signup input.ng-valid.ng-dirty { +form input.ng-valid.ng-dirty { background-color: #DDFFEE; } @@ -689,6 +689,11 @@ p.editable:hover i { margin-bottom: 10px; } +.user-admin .form-change-pw input { + margin-top: 12px; + margin-bottom: 12px; +} + #image-history-container .node circle { cursor: pointer; fill: #fff; diff --git a/static/js/app.js b/static/js/app.js index 310d3dd74..4edfa3549 100644 --- a/static/js/app.js +++ b/static/js/app.js @@ -5,7 +5,8 @@ quayApp = angular.module('quay', ['restangular', 'angularMoment', 'angulartics', verified: false, anonymous: true, username: null, - email: null + email: null, + askForPassword: false, } var userService = {} diff --git a/static/js/controllers.js b/static/js/controllers.js index 4f25a52e2..100805a5c 100644 --- a/static/js/controllers.js +++ b/static/js/controllers.js @@ -456,9 +456,13 @@ function RepoAdminCtrl($scope, Restangular, $routeParams, $rootScope) { }); } -function UserAdminCtrl($scope, Restangular, PlanService, KeyService, $routeParams) { +function UserAdminCtrl($scope, $timeout, Restangular, PlanService, UserService, KeyService, $routeParams) { $scope.plans = PlanService.planList(); + $scope.$watch(function () { return UserService.currentUser(); }, function (currentUser) { + $scope.askForPassword = currentUser.askForPassword; + }, true); + var subscribedToPlan = function(sub) { $scope.subscription = sub; $scope.subscribedPlan = PlanService.getPlan(sub.plan); @@ -545,4 +549,28 @@ function UserAdminCtrl($scope, Restangular, PlanService, KeyService, $routeParam $scope.subscribe(requested); } } + + $scope.updatingUser = false; + $scope.changePasswordSuccess = false; + $('.form-change-pw').popover(); + + $scope.changePassword = function() { + $('.form-change-pw').popover('hide'); + $scope.updatingUser = true; + $scope.changePasswordSuccess = false; + var changePasswordPost = Restangular.one('user/'); + changePasswordPost.customPUT($scope.user).then(function() { + $scope.updatingUser = false; + $scope.changePasswordSuccess = true; + + UserService.load(); + }, function(result) { + $scope.updatingUser = false; + + $scope.changePasswordError = result.data.message; + $timeout(function() { + $('.form-change-pw').popover('show'); + }); + }); + }; } \ No newline at end of file diff --git a/static/partials/user-admin.html b/static/partials/user-admin.html index 17c98a070..39201569d 100644 --- a/static/partials/user-admin.html +++ b/static/partials/user-admin.html @@ -7,6 +7,11 @@
{{ errorMessage }}
+
+
+
Your account does not currently have a password. You will need to create a password before you will be able to push or pull repositories.
+
+
@@ -52,4 +57,24 @@
+
+
+ +
+
+
+
+ Change Password +
+
+
+ + + + Password changed successfully +
+
+
+
+
diff --git a/util/validation.py b/util/validation.py index 9121a23e2..bbe02e017 100644 --- a/util/validation.py +++ b/util/validation.py @@ -1,6 +1,8 @@ import re import urllib +INVALID_PASSWORD_MESSAGE = 'Invalid password, password must be at least ' + \ + '8 characters and contain no whitespace.' def validate_email(email_address): if re.match(r'[^@]+@[^@]+\.[^@]+', email_address): From 33e691cca684e90d30acf0bb57f80649ebb00123 Mon Sep 17 00:00:00 2001 From: yackob03 Date: Thu, 10 Oct 2013 14:01:47 -0400 Subject: [PATCH 04/10] Use a badge to prompt GitHub users to set a password. --- templates/index.html | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/templates/index.html b/templates/index.html index fd892e009..144ef31bf 100644 --- a/templates/index.html +++ b/templates/index.html @@ -85,10 +85,16 @@ mixpanel.init(isProd ? "50ff2b2569faa3a51c8f5724922ffb7e" : "38014a0f27e7bdc3ff8 {{ user.username }} + 1 From 3cadc5bdb8bab02ea32c2eadffa81ca5c08fa630 Mon Sep 17 00:00:00 2001 From: yackob03 Date: Thu, 10 Oct 2013 14:02:11 -0400 Subject: [PATCH 05/10] Fix a literally insignificant typo. --- static/partials/landing.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/static/partials/landing.html b/static/partials/landing.html index 335819b28..bc65e0afc 100644 --- a/static/partials/landing.html +++ b/static/partials/landing.html @@ -40,7 +40,7 @@ - +

No credit card required.

From 5b25d8db5bee9bbae94e08a9bb6f691062a8fb39 Mon Sep 17 00:00:00 2001 From: yackob03 Date: Thu, 10 Oct 2013 14:02:28 -0400 Subject: [PATCH 06/10] Reset the form back to a pristine state on a successful password change. --- static/js/controllers.js | 5 +++++ static/partials/user-admin.html | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/static/js/controllers.js b/static/js/controllers.js index 100805a5c..d9870ae52 100644 --- a/static/js/controllers.js +++ b/static/js/controllers.js @@ -563,6 +563,11 @@ function UserAdminCtrl($scope, $timeout, Restangular, PlanService, UserService, $scope.updatingUser = false; $scope.changePasswordSuccess = true; + // Reset the form + $scope.user.password = ''; + $scope.user.repeatPassword = ''; + $scope.changePasswordForm.$setPristine(); + UserService.load(); }, function(result) { $scope.updatingUser = false; diff --git a/static/partials/user-admin.html b/static/partials/user-admin.html index 39201569d..16df0fe00 100644 --- a/static/partials/user-admin.html +++ b/static/partials/user-admin.html @@ -69,7 +69,7 @@
- + Password changed successfully
From 669b3fcde1d8173ada21ef376039fa4bed5eb634 Mon Sep 17 00:00:00 2001 From: yackob03 Date: Thu, 10 Oct 2013 14:42:14 -0400 Subject: [PATCH 07/10] Add login with GitHub to the landing page. --- static/css/quay.css | 29 +++++++++++++++++++++++++++-- static/js/app.js | 2 ++ static/js/controllers.js | 4 +++- static/partials/landing.html | 9 +++++++-- 4 files changed, 39 insertions(+), 5 deletions(-) diff --git a/static/css/quay.css b/static/css/quay.css index 0c7a33c1a..1523e8a8c 100644 --- a/static/css/quay.css +++ b/static/css/quay.css @@ -115,7 +115,7 @@ margin-top: -20px; margin-bottom: 0px; - padding-top: 66px; + padding-top: 46px; min-height: 440px; } @@ -137,6 +137,31 @@ margin-left: 0px; } +.signin-buttons { + text-align: center; +} + +.landing-signup-button { + margin-bottom: 10px; +} + +.landing-social-alternate { + color: #777; + font-size: 2em; + margin-left: 43px; + line-height: 1em; +} + +.landing-social-alternate .inner-text { + text-align: center; + position: relative; + color: white; + left: -43px; + top: -9px; + font-weight: bold; + font-size: .4em; +} + form input.ng-invalid.ng-dirty { background-color: #FDD7D9; } @@ -201,7 +226,7 @@ form input.ng-valid.ng-dirty { @media (max-height: 768px) { .landing { padding: 20px; - padding-top: 46px; + padding-top: 20px; } } diff --git a/static/js/app.js b/static/js/app.js index 4edfa3549..e9cca9b46 100644 --- a/static/js/app.js +++ b/static/js/app.js @@ -42,8 +42,10 @@ quayApp = angular.module('quay', ['restangular', 'angularMoment', 'angulartics', if ($location.host() === 'quay.io') { keyService['stripePublishableKey'] = 'pk_live_P5wLU0vGdHnZGyKnXlFG4oiu'; + keyService['githubClientId'] = '5a8c08b06c48d89d4d1e'; } else { keyService['stripePublishableKey'] = 'pk_test_uEDHANKm9CHCvVa2DLcipGRh'; + keyService['githubClientId'] = 'cfbc4aca88e5c1b40679'; } return keyService; diff --git a/static/js/controllers.js b/static/js/controllers.js index d9870ae52..eab1d4530 100644 --- a/static/js/controllers.js +++ b/static/js/controllers.js @@ -130,7 +130,7 @@ function RepoListCtrl($scope, Restangular, UserService) { }); } -function LandingCtrl($scope, $timeout, Restangular, UserService) { +function LandingCtrl($scope, $timeout, Restangular, UserService, KeyService) { $('.form-signup').popover(); $scope.$watch( function () { return UserService.currentUser(); }, function (currentUser) { @@ -141,6 +141,8 @@ function LandingCtrl($scope, $timeout, Restangular, UserService) { $scope.user = currentUser; }, true); + $scope.githubClientId = KeyService.githubClientId; + $scope.awaitingConfirmation = false; $scope.registering = false; diff --git a/static/partials/landing.html b/static/partials/landing.html index bc65e0afc..999e580eb 100644 --- a/static/partials/landing.html +++ b/static/partials/landing.html @@ -41,8 +41,13 @@ -
- + From b9a5060882ed6510bcbfaafadaccb4bad226cde9 Mon Sep 17 00:00:00 2001 From: yackob03 Date: Thu, 10 Oct 2013 16:34:59 -0400 Subject: [PATCH 08/10] Set the created variable in a set_once on the user service. This will make it work for github logins as well. --- static/js/app.js | 3 +++ static/js/controllers.js | 6 ------ 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/static/js/app.js b/static/js/app.js index e9cca9b46..0fe4decd1 100644 --- a/static/js/app.js +++ b/static/js/app.js @@ -23,6 +23,9 @@ quayApp = angular.module('quay', ['restangular', 'angularMoment', 'angulartics', '$username': userResponse.username, 'verified': userResponse.verified }); + mixpanel.people.set_once({ + '$created': new Date() + }) } }); }; diff --git a/static/js/controllers.js b/static/js/controllers.js index eab1d4530..0fbe4c911 100644 --- a/static/js/controllers.js +++ b/static/js/controllers.js @@ -164,12 +164,6 @@ function LandingCtrl($scope, $timeout, Restangular, UserService, KeyService) { $scope.registering = false; mixpanel.alias($scope.newUser.username); - mixpanel.people.set_once({ - '$email': $scope.newUser.email, - '$username': $scope.newUser.username, - '$created': new Date(), - 'verified': false - }); }, function(result) { $scope.registering = false; $scope.registerError = result.data.message; From 32b28df2d2f2bdee8fa5c7a8670d6f20008e6e86 Mon Sep 17 00:00:00 2001 From: yackob03 Date: Thu, 10 Oct 2013 17:32:32 -0400 Subject: [PATCH 09/10] Send a registration even for github created users. Alias their new username to their old mixpanel ID passed in the OAuth state parameter. --- endpoints/web.py | 10 +++++++++- static/js/controllers.js | 5 +++++ static/partials/landing.html | 2 +- templates/signin.html | 26 +++++++++++++++++++++++++- 4 files changed, 40 insertions(+), 3 deletions(-) diff --git a/endpoints/web.py b/endpoints/web.py index 6f170cbdc..41a669d58 100644 --- a/endpoints/web.py +++ b/endpoints/web.py @@ -7,7 +7,7 @@ from flask.ext.login import login_user, UserMixin, login_required, logout_user from flask.ext.principal import identity_changed, Identity, AnonymousIdentity from data import model -from app import app, login_manager +from app import app, login_manager, mixpanel logger = logging.getLogger(__name__) @@ -143,6 +143,14 @@ def github_oauth_callback(): return render_template('githuberror.html', error_message=ex.message) if common_login(to_login): + # Success + mixpanel.track(to_login.username, 'register', {'service': 'github'}) + + state = request.args.get('state', None) + if state: + logger.debug('Aliasing with state: %s' % state) + mixpanel.alias(to_login.username, state) + return redirect(url_for('index')) # TODO something bad happened, we need to tell the user somehow diff --git a/static/js/controllers.js b/static/js/controllers.js index 0fbe4c911..57eacb213 100644 --- a/static/js/controllers.js +++ b/static/js/controllers.js @@ -141,6 +141,11 @@ function LandingCtrl($scope, $timeout, Restangular, UserService, KeyService) { $scope.user = currentUser; }, true); + angulartics.waitForVendorApi(mixpanel, 500, function(loadedMixpanel) { + var mixpanelId = loadedMixpanel.get_distinct_id(); + $scope.github_state_clause = '&state=' + mixpanelId; + }); + $scope.githubClientId = KeyService.githubClientId; $scope.awaitingConfirmation = false; diff --git a/static/partials/landing.html b/static/partials/landing.html index 999e580eb..6e1e10029 100644 --- a/static/partials/landing.html +++ b/static/partials/landing.html @@ -47,7 +47,7 @@ OR - Sign In with GitHub + Sign In with GitHub

No credit card required.

diff --git a/templates/signin.html b/templates/signin.html index 4b14b6dcb..4a107a9c3 100644 --- a/templates/signin.html +++ b/templates/signin.html @@ -7,7 +7,17 @@ + + + +
{% if invalid_credentials %} @@ -31,5 +41,19 @@
You must verify your email address before you can sign in.
{% endif %}
+ + \ No newline at end of file From ce81431cd335936473ae2ae6a3b19cd9f98f75bf Mon Sep 17 00:00:00 2001 From: yackob03 Date: Thu, 10 Oct 2013 19:06:04 -0400 Subject: [PATCH 10/10] Enable HTML5 mode for routing. --- endpoints/web.py | 5 +++-- static/js/app.js | 2 ++ static/partials/landing.html | 10 +++++----- static/partials/repo-admin.html | 2 +- static/partials/repo-list.html | 6 +++--- static/partials/view-repo.html | 4 ++-- templates/index.html | 17 ++++++++++------- 7 files changed, 26 insertions(+), 20 deletions(-) diff --git a/endpoints/web.py b/endpoints/web.py index 41a669d58..d50f15b2c 100644 --- a/endpoints/web.py +++ b/endpoints/web.py @@ -34,8 +34,9 @@ def load_user(username): return None -@app.route('/', methods=['GET']) -def index(): +@app.route('/', methods=['GET'], defaults={'path': ''}) +@app.route('/') # Catch all +def index(path): return send_file('templates/index.html') diff --git a/static/js/app.js b/static/js/app.js index 0fe4decd1..16710cc3f 100644 --- a/static/js/app.js +++ b/static/js/app.js @@ -122,6 +122,8 @@ quayApp = angular.module('quay', ['restangular', 'angularMoment', 'angulartics', $analyticsProvider.virtualPageviews(true); + $locationProvider.html5Mode(true); + $routeProvider. when('/repository/:namespace/:name', {templateUrl: '/static/partials/view-repo.html', controller: RepoCtrl}). when('/repository/:namespace/:name/tag/:tag', {templateUrl: '/static/partials/view-repo.html', controller: RepoCtrl}). diff --git a/static/partials/landing.html b/static/partials/landing.html index 6e1e10029..b7e2d6248 100644 --- a/static/partials/landing.html +++ b/static/partials/landing.html @@ -5,7 +5,7 @@

Secure hosting for private docker repositories

Use the docker images your team needs with the safety of private repositories

- +
@@ -25,9 +25,9 @@ You don't have any private repositories yet!
@@ -157,7 +157,7 @@

Support

diff --git a/static/partials/repo-admin.html b/static/partials/repo-admin.html index 637cb000d..10ebaaee9 100644 --- a/static/partials/repo-admin.html +++ b/static/partials/repo-admin.html @@ -8,7 +8,7 @@
- +

{{repo.namespace}} / {{repo.name}}

diff --git a/static/partials/repo-list.html b/static/partials/repo-list.html index f5343dbf0..7350559c0 100644 --- a/static/partials/repo-list.html +++ b/static/partials/repo-list.html @@ -8,7 +8,7 @@ @@ -16,7 +16,7 @@ @@ -26,7 +26,7 @@

Top Public Repositories

diff --git a/static/partials/view-repo.html b/static/partials/view-repo.html index 6894dbbe6..97faff11e 100644 --- a/static/partials/view-repo.html +++ b/static/partials/view-repo.html @@ -18,7 +18,7 @@ {{repo.namespace}} / {{repo.name}} - + @@ -63,7 +63,7 @@ {{currentTag.name}} diff --git a/templates/index.html b/templates/index.html index d0a7b5348..95986c2b1 100644 --- a/templates/index.html +++ b/templates/index.html @@ -2,9 +2,12 @@ Quay - Private Docker Repository + + + @@ -60,15 +63,15 @@ mixpanel.init(isProd ? "50ff2b2569faa3a51c8f5724922ffb7e" : "38014a0f27e7bdc3ff8 - Quay + Quay