<p>Just wanted yall's help with my code. I wrote this wireframe for user authentication in golang using mongodb.</p>
<p>Looking for criticism on what could make it more secure. And also general code simplification/organization would be great as well.</p>
<p><a href="https://github.com/brianhodges/golang-mongodb-users" rel="nofollow">https://github.com/brianhodges/golang-mongodb-users</a></p>
<hr/>**评论：**<br/><br/>luckyleprechaun98: <pre><ul>
<li><p>don't store plain text passwords in your DB and don't use SHA for your hashing function. Use scrypt or bcrypt</p></li>
<li><p>Use a constant-time comparison when you are checking for password hash matches</p></li>
<li><p>Check all your err returns and don't panic on an error, deal with it </p></li>
<li><p>Use standard go fmt to format your code. Most editors can be configured to do this automatically on save</p></li>
</ul></pre>awe8haew089: <pre><p>Thanks for the feedback. I will be implementing these.</p></pre>codingconcepts: <pre><ol>
<li> You're connecting to MongoDB on every request, which is very slow. Instead, create one connection at start-up (the slow bit) and .Copy() that where you're currently connecting in the handlers.</li>
<li> Be clear about what HTTP verbs your handlers are to support. Currently, they can be called with any.</li>
<li> Configure a DB user for MongoDB and make your server use those credentials to login (use DialWithInfo instead of Dial to surface this)</li>
<li> Change the port MongoDB is listening on. MongoDB's default port is well-known and people scan the internet for it. Without a username and password, your database is completely open. See here: <a href="http://www.information-age.com/major-security-alert-40000-mongodb-databases-left-unsecured-internet-123459001/" rel="nofollow">http://www.information-age.com/major-security-alert-40000-mongodb-databases-left-unsecured-internet-123459001/</a></li>
<li> Use hmac.New(sha512.New512_256...) for hashing instead of the SHA algo by itself.</li>
<li> Consider reading the following (I found them very helpful when writing public-facing Go code):</li>
</ol>
<p><a href="https://github.com/unrolled/secure" rel="nofollow">https://github.com/unrolled/secure</a></p>
<p><a href="https://blog.gopheracademy.com/advent-2016/exposing-go-on-the-internet/" rel="nofollow">https://blog.gopheracademy.com/advent-2016/exposing-go-on-the-internet/</a></p>
<ol>
<li> Make sure you only accept HTTPS connections and redirect HTTP traffic to HTTPS because your username and password (and obviously all user data) would otherwise be sent in plaintext. (Let's Encrypt make this free and easy, use something like autocert to generate secure certificates that auto-renew)</li>
</ol></pre>whizack: <pre><ul>
<li>don't seed random every time you generate a salt</li>
<li>use the x/crypto/bcrypt package</li>
<li>if you want to keep util.CheckError, it needs to have a signature like CheckError(err error, onError func(error)) you shouldn't just panic</li>
<li>don't use math/rand for crypto</li>
<li>don't parse your template on every render call</li>
</ul></pre>awe8haew089: <pre><p>Thanks for the info. Everything is pretty straight forward except I don't understand the comments on the seed random or parsing template. Are these for optimization/performance?</p></pre>elagergren: <pre><p>ignore #1, it's irrelevant if you switch to crypto/rand which you 10000000% need to do if you're writing secure code.</p>
<p>wrt parsing, you can cache the parsed template once so you're not redoing that work every time.</p></pre>UniverseCity: <pre><p>Use <code>go fmt</code></p></pre>gentleman_tech: <pre><p>other programs might want to specify a port using an environment variable too ;)</p>
<p>if you're returning a value, maybe, and then checking for it, either use the nil value to represent "no return", or use an "ok" value to indicate whether it was successful.
so your account.AuthenticatedUser func should probably either return nil for "no such user" or return result, ok</p>
<p>It's been mentioned before, but <em>really</em> don't store passwords in the databse in plaintext. There's literally no point encrypting passwords if you also store them in plaintext in the database.</p>
<p>The separation of concerns between util and account seems really weird. GetUserFromSession feels like it should definitely be in account not util. Maybe provide a GetCookieValue func in util and then call that from account to get the user name?</p>
<p>consider using named blocks to reduce the amount of boilerplate and repetition in your templates - defining a standard header block and a standard footer block will save you hassle later.</p>
<p>otherwise, a good start :)</p></pre>Gurrewe: <pre><p>I'd recommend goreportcard for automatic and simple score of your code.</p>
<p><a href="https://goreportcard.com/report/github.com/brianhodges/golang-mongodb-users" rel="nofollow">https://goreportcard.com/report/github.com/brianhodges/golang-mongodb-users</a></p></pre>
