You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Hiroaki Kawai <ka...@stratosphere.co.jp> on 2012/11/30 12:06:49 UTC
Review Request: Default admin user account will not be created in clean setup
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8293/
-----------------------------------------------------------
Review request for cloudstack.
Description
-------
Current com.cloud.server.ConfigurationServerImpl#saveUser tries to insert an admin user entity into database, but the sql leaves password field unspecified. This will always fail because the database schema declares password field NOT NULL. This patch will fix database schema, and add missing logging when this sql execution failed.
Diffs
-----
server/src/com/cloud/server/ConfigurationServerImpl.java d2f0bb0
setup/db/create-schema.sql fff084e
Diff: https://reviews.apache.org/r/8293/diff/
Testing
-------
Thanks,
Hiroaki Kawai
RE: Review Request: Default admin user account will not be created
in clean setup
Posted by Animesh Chaturvedi <an...@citrix.com>.
Hiroaki
I was going through the review board and this is still pending. Can you review and respond to reviewer comments?
Animesh
> -----Original Message-----
> From: Rohit Yadav [mailto:noreply@reviews.apache.org] On Behalf Of Rohit
> Yadav
> Sent: Tuesday, January 08, 2013 8:17 PM
> To: cloudstack; Hiroaki Kawai; Rohit Yadav
> Subject: Re: Review Request: Default admin user account will not be created in
> clean setup
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8293/#review15169
> -----------------------------------------------------------
>
>
> Any update on this one?
>
> - Rohit Yadav
>
>
> On Nov. 30, 2012, 11:09 a.m., Hiroaki Kawai wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/8293/
> > -----------------------------------------------------------
> >
> > (Updated Nov. 30, 2012, 11:09 a.m.)
> >
> >
> > Review request for cloudstack.
> >
> >
> > Description
> > -------
> >
> > Current com.cloud.server.ConfigurationServerImpl#saveUser tries to insert
> an admin user entity into database, but the sql leaves password field
> unspecified. This will always fail because the database schema declares
> password field NOT NULL. This patch will fix database schema, and add missing
> logging when this sql execution failed.
> >
> > This happens only on current master:HEAD.
> >
> >
> > Diffs
> > -----
> >
> > server/src/com/cloud/server/ConfigurationServerImpl.java d2f0bb0
> > setup/db/create-schema.sql fff084e
> >
> > Diff: https://reviews.apache.org/r/8293/diff/
> >
> >
> > Testing
> > -------
> >
> >
> > Thanks,
> >
> > Hiroaki Kawai
> >
> >
Re: Review Request: Default admin user account will not be created in clean
setup
Posted by Rohit Yadav <bh...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8293/#review15169
-----------------------------------------------------------
Any update on this one?
- Rohit Yadav
On Nov. 30, 2012, 11:09 a.m., Hiroaki Kawai wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8293/
> -----------------------------------------------------------
>
> (Updated Nov. 30, 2012, 11:09 a.m.)
>
>
> Review request for cloudstack.
>
>
> Description
> -------
>
> Current com.cloud.server.ConfigurationServerImpl#saveUser tries to insert an admin user entity into database, but the sql leaves password field unspecified. This will always fail because the database schema declares password field NOT NULL. This patch will fix database schema, and add missing logging when this sql execution failed.
>
> This happens only on current master:HEAD.
>
>
> Diffs
> -----
>
> server/src/com/cloud/server/ConfigurationServerImpl.java d2f0bb0
> setup/db/create-schema.sql fff084e
>
> Diff: https://reviews.apache.org/r/8293/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hiroaki Kawai
>
>
Re: Review Request: Default admin user account will not be created in clean
setup
Posted by Min Chen <mi...@citrix.com>.
> On Dec. 20, 2012, 6:45 a.m., Hugo Trippaers wrote:
> > Heya,
> >
> > Good catch this one.
> >
> > Please do not use the INSERT IGNORE syntax. This is Mysql specific and would prevent using any other database in the future. Isn't setting a random password a better solution?
> >
> > Adding the logging statements is good.
> >
> > Removing the NOT NULL requirements on the password field would lower (a very small) security threshold, which is something we shouldn't do.
> >
> > Cheers,
> >
> > Hugo
Agree with Hugo here. We should not remove NOT NULL constraints on password field in User table. Inserting a random password will make it work since later on CloudStartupServlet.enableAdmin will update it to encoded version of "password". Ideally, in production system, we should allow user to provide a non-default password in setting up management server.
- Min
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8293/#review14751
-----------------------------------------------------------
On Nov. 30, 2012, 11:09 a.m., Hiroaki Kawai wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8293/
> -----------------------------------------------------------
>
> (Updated Nov. 30, 2012, 11:09 a.m.)
>
>
> Review request for cloudstack.
>
>
> Description
> -------
>
> Current com.cloud.server.ConfigurationServerImpl#saveUser tries to insert an admin user entity into database, but the sql leaves password field unspecified. This will always fail because the database schema declares password field NOT NULL. This patch will fix database schema, and add missing logging when this sql execution failed.
>
> This happens only on current master:HEAD.
>
>
> Diffs
> -----
>
> server/src/com/cloud/server/ConfigurationServerImpl.java d2f0bb0
> setup/db/create-schema.sql fff084e
>
> Diff: https://reviews.apache.org/r/8293/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hiroaki Kawai
>
>
Re: Review Request: Default admin user account will not be created in clean
setup
Posted by Hugo Trippaers <ht...@schubergphilis.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8293/#review14751
-----------------------------------------------------------
Heya,
Good catch this one.
Please do not use the INSERT IGNORE syntax. This is Mysql specific and would prevent using any other database in the future. Isn't setting a random password a better solution?
Adding the logging statements is good.
Removing the NOT NULL requirements on the password field would lower (a very small) security threshold, which is something we shouldn't do.
Cheers,
Hugo
- Hugo Trippaers
On Nov. 30, 2012, 11:09 a.m., Hiroaki Kawai wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8293/
> -----------------------------------------------------------
>
> (Updated Nov. 30, 2012, 11:09 a.m.)
>
>
> Review request for cloudstack.
>
>
> Description
> -------
>
> Current com.cloud.server.ConfigurationServerImpl#saveUser tries to insert an admin user entity into database, but the sql leaves password field unspecified. This will always fail because the database schema declares password field NOT NULL. This patch will fix database schema, and add missing logging when this sql execution failed.
>
> This happens only on current master:HEAD.
>
>
> Diffs
> -----
>
> server/src/com/cloud/server/ConfigurationServerImpl.java d2f0bb0
> setup/db/create-schema.sql fff084e
>
> Diff: https://reviews.apache.org/r/8293/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hiroaki Kawai
>
>
Re: Review Request: Default admin user account will not be created in clean
setup
Posted by Rohit Yadav <ro...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8293/#review14732
-----------------------------------------------------------
Min may review this? Pl. add Min Chen (minchen07) as the reviewer.
- Rohit Yadav
On Nov. 30, 2012, 11:09 a.m., Hiroaki Kawai wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8293/
> -----------------------------------------------------------
>
> (Updated Nov. 30, 2012, 11:09 a.m.)
>
>
> Review request for cloudstack.
>
>
> Description
> -------
>
> Current com.cloud.server.ConfigurationServerImpl#saveUser tries to insert an admin user entity into database, but the sql leaves password field unspecified. This will always fail because the database schema declares password field NOT NULL. This patch will fix database schema, and add missing logging when this sql execution failed.
>
> This happens only on current master:HEAD.
>
>
> Diffs
> -----
>
> server/src/com/cloud/server/ConfigurationServerImpl.java d2f0bb0
> setup/db/create-schema.sql fff084e
>
> Diff: https://reviews.apache.org/r/8293/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hiroaki Kawai
>
>
Re: Review Request: Default admin user account will not be created in clean
setup
Posted by Hiroaki Kawai <ka...@stratosphere.co.jp>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8293/
-----------------------------------------------------------
(Updated Nov. 30, 2012, 11:09 a.m.)
Review request for cloudstack.
Description (updated)
-------
Current com.cloud.server.ConfigurationServerImpl#saveUser tries to insert an admin user entity into database, but the sql leaves password field unspecified. This will always fail because the database schema declares password field NOT NULL. This patch will fix database schema, and add missing logging when this sql execution failed.
This happens only on current master:HEAD.
Diffs
-----
server/src/com/cloud/server/ConfigurationServerImpl.java d2f0bb0
setup/db/create-schema.sql fff084e
Diff: https://reviews.apache.org/r/8293/diff/
Testing
-------
Thanks,
Hiroaki Kawai