BRL-CAD / OGV-meteor

Port of BRL-CAD's OGV to meteor
BSD 3-Clause "New" or "Revised" License
16 stars 26 forks source link

Meteor Admin User Creation Fix #91

Closed quentinpaden closed 5 years ago

quentinpaden commented 5 years ago

Creating a randomly generated 32 digit alphanumeric password for secure and successful meteor admin user creation and logging it to the server.

Gauravjeetsingh commented 5 years ago

Few notes

So what we want is, we want to use the values from the settings.json file. But if the user has not changed the adminPassword in it, only then we will set it to a random password, otherwise, we will let it use what's written in the file.

But, I must say, I am pretty glad, it's almost fixed. Just a few minor things. Let me know if you still have any doubts about it.

quentinpaden commented 5 years ago

Created a fix under this requirement.

The Meteor.Settings function clears out things if you don't replicate some of the settings from settings.json in there so I had to set the adminPassword with Private, Public, and SMTP or else Meteor gives an error thinking you have overwritten the previous Public, Private, and SMTP from settings.json with an unidentified object.

This new commit fixes this problem.

inderpreetsingh commented 5 years ago

Hey @Max2020q, there's a small syntactical error of missing quote in the code in if statement. Even after fixing that, it will still create an account using a random password instead of the updated password in settings.json file.

This is because you are changing the password twice, once in if statement and then in else if statement. But you added the Meteor settings check at only one place (inside if at line 52).

Possible Solution: If you move the code in lines 56 - 69 in file accounts.js after the else if statement, it will work just fine. Then you won't have to check the password twice.

Gauravjeetsingh commented 5 years ago

In addition to the above change, I see a few lines of code without a semicolon at the end in accounts.js. This sure works, but is not as per the coding standards we follow in OGV code.

If you can fix them as well, it would be awesome.

Gauravjeetsingh commented 5 years ago

@Max2020q I did meteor reset and changed the password in settings.json, but It still logs in with a random password instead of the password in settings.json file. The possible reason for this was already covered in Inder's previous review.

Hey @Max2020q, there's a small syntactical error of missing quote in the code in if statement.

This is fixed.

Even after fixing that, it will still create an account using a random password instead of the updated password in settings.json file.

This is because you are changing the password twice, once in if statement and then in else if statement. But you added the Meteor settings check at only one place (inside if at line 52).

Possible Solution: If you move the code in lines 56 - 69 in file accounts.js after the else if statement, it will work just fine. Then you won't have to check the password twice.

This is not fixed. Can you please re-read above review, and let me know if you have any doubts.