Skip to content
This repository has been archived by the owner on Aug 30, 2021. It is now read-only.

Master to 0.4.0 #616

Merged
merged 128 commits into from
Jul 8, 2015
Merged

Master to 0.4.0 #616

merged 128 commits into from
Jul 8, 2015

Conversation

lirantal
Copy link
Member

@lirantal lirantal commented Jul 3, 2015

This is 0.4.0 branch after I merged it with changes from master and fixes.
Once we review it and want to continue forward with move to 0.4.0:

  1. We will merge this PR with 0.4.0
  2. We will merge any outstanding and important 0.4.0 PRs with 0.4.0
  3. We will merge 0.4.0 into master and make it an 0.4.0 release

lirantal and others added 30 commits October 14, 2014 12:14
It used to fail silently (client only displays error when a message is available).
removed extra comma
The user password salt should be encoded with Base64 before being saved
to the database.

The current code adds an unecessary step of converting the result of
crypto.randomBytes() (which already returns a SlowBuffer) to a Base64
string and back again to a Buffer, and misses the final step of
converting the Buffer's bytes back to a Base64 string.

Because of this, the salt stored in the database is garbled. This is
inconvenient when manipulating the data in a terminal or text editor.

When generating the password hash, the crypto.pbkdf2Sync() method
creates a new Buffer directly from the data supplied. Due to the
incorrect encoding of the salt, entropy is lost at this step,
weakening the security of stored passwords against brute force attacks.
Currently createTransport is unnecessarily called upon every time a password request is made.
…ub.com/lirantal/meanjs into lirantal-enhancements-express-cookie-parameters

Conflicts:
	config/env/all.js
…ookie-parameters

Enhancements express cookie parameters
fix getToggleElement on dropdown by updating to angular-bootstrap 0.12.0 #250
Removed unneeded comas from gruntfile.
Show error message when sending password request mail fails
Remove unecessary comment in karma.conf.js
@@ -95,4 +95,4 @@
"karma-firefox-launcher": "~0.1.3",
"karma-phantomjs-launcher": "~0.1.2"
}
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Newline needed

@ilanbiala
Copy link
Member

@lirantal @roieki @amoshaviv @NeverOddOrEven @Shrizzy Left some comments to review.

@lirantal
Copy link
Member Author

lirantal commented Jul 4, 2015

thanks @ilanbiala I addressed all issues you mentioned

@lirantal
Copy link
Member Author

lirantal commented Jul 6, 2015

@ilanbiala I think we're good to merge this to 0.4, right?

@ilanbiala
Copy link
Member

Yes, I think so.

@mleanos
Copy link
Member

mleanos commented Jul 6, 2015

Had a holiday weekend, so I didn't spend much time at my computer over the last few days.

So far it looks good to me. I'm still going through it right now.

@rhutchison
Copy link
Contributor

Update dependencies: #622

@mleanos
Copy link
Member

mleanos commented Jul 7, 2015

All seems fine with me as well. I think we can start merging this into 0.4

@mleanos
Copy link
Member

mleanos commented Jul 8, 2015

I forked this repository. I then checked the master_to_0.4.0 branch out, and tested locally. Everything went fine, and I merged it to the 0.4.0. The merge had no issues at all, and everything seems to be working perfectly.

I'm not sure what I should do next to help this merge, and then to begin helping out with merging all the PR's.

Forgive me, but I'm a bit new to the forking and contributing aspect of GitHub. Should I push my new 0.4.0 branch to my repo, and create a pull request? I did make one modification to the gitingore. I added VS files.

@lirantal
Copy link
Member Author

lirantal commented Jul 8, 2015

@mleanos thanks for helping out.
The procedure you did confirms that we shouldn't have any issues with the merge, but basically the plan is now:

  1. Merge this PR master_to_0.4.0 to 0.4.0
  2. Then merge any valid and outstanding 0.4.0 PRs to 0.4.0
  3. Then we make a 0.4.0 release and merge 0.4.0 to master

At the moment I'm just waiting to figure out if we're merging this PR #622 to master_to_0.4.0 before I'm closing it, and then we can continue.

@mleanos once step (2) is finished we'll need quite some help with testing the 0.4.0 branch and that's where you can help the most. It'll be a few days/week until we're there.

Thanks for chiming in!

lirantal added a commit that referenced this pull request Jul 8, 2015
@lirantal lirantal merged commit c2bcfa3 into 0.4.0 Jul 8, 2015
openssl genrsa -out ./config/sslcerts/key.pem -aes256 1024
openssl req -new -key ./config/sslcerts/key.pem -out ./config/sslcerts/csr.pem
openssl x509 -req -days 9999 -in ./config/sslcerts/csr.pem -signkey ./config/sslcerts/key.pem -out ./config/sslcerts/cert.pem
rm ./config/sslcerts/csr.pem
# resolve issue with bad password...
# Error: error:0906A068:PEM routines:PEM_do_header:bad password read
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lirantal for some reason I get this error running with ssl after generating a cert with a password:
Error: error:0906A068:PEM routines:PEM_do_header:bad password read

my suspicion is that node is not asking me for the password when running grunt prod. I am running with node v0.12.5

to resolve, I needed to remove the password from the pem like so:

openssl rsa -in ./config/sslcerts/key.pem -out ./config/sslcerts/newkey.pem && mv ./config/sslcerts/newkey.pem ./config/sslcerts/key.pem
chmod 0600 ./config/sslcerts/key.pem ./config/sslcerts/cert.pem

not sure if anyone else has seen this, but I mentioned the a reference to another person solving this issue the same way:
http://blog.mgechev.com/2014/02/19/create-https-tls-ssl-application-with-express-nodejs/

should I open a ticket for this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jloveland yes, please open an issue on that and assign it to me, I'll get to it later this week.

@ilanbiala ilanbiala deleted the master_to_0.4.0 branch August 13, 2015 13:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.