Passwords with C++ Console

Question

I am really, really new to coding in general and especially c++. To get more familiar with c++ I am trying to do a series of projects with c++ console apps. One of my projects is with a database that uses a password to authorize entry of the database. I read a post talking about using fstream to create a text file to confirm a password entered with cin.

Eventually I would like to protect the text file so that somebody could not open it. I would appreciate any advice on how to make a .exe file for a console program where the .txt file is included and the files in the .exe can not be accessed.

Here is the program for authorization using fstream:

#include <iostream>
#include <fstream>
#include <string>
#include "sha256.h"
using namespace std;

bool keyExists() {
bool keyExists = false;
ifstream inFile("key.txt");

if (inFile) {
    keyExists = true;
}

return keyExists;
}
bool isMatch(string key, string confirmKey) {

bool match = false;
if (key == confirmKey) {
    match = true;
}

return match;
}

int main() {

if (keyExists()) {
    string key;
    string storedKey;
    cout << "Please enter key: ";
    getline(cin, key);

    SHA256 sha256;
    ifstream inFile("key.txt");
    getline(inFile, storedKey);

    if (isMatch(sha256(key), storedKey)) {
        cout << "Acces Granted!\n";
    }
    else {
        cout << "Access Denied!\n";
    }
    }

    else {
    string key;
    string confirmKey;

    cout << "Please create a key: ";
    getline(cin, key);
    cout << "Confirm key: ";
    getline(cin, confirmKey);

    if (isMatch(key, confirmKey)) {

        SHA256 sha256;
        ofstream outFile("key.txt");
        outFile << sha256(key);
    }

    else {
        cout << "Keys do not match!\n";
    }
}

return 0;
}

Answer 1

Overall

A good first go.

I have some minor points below. But the main thing I think you need to read up on is passing parameters by reference.

 bool isMatch(string key, string confirmKey)

Here, the parameters key and confirmKey are passed by value. This means a copy of the original value is created and passed to the function. When you have large or expensive objects, you would prefer not to copy them (strings are considered expensive as you have to allocate memory and then copy all the bytes in the string to the new location).

Since you don't do much with the strings (all non mutating functions) you could simply pass a reference to the original object (a const version, so you know the function will not change them) and then you can compare those:

 bool isMatch(std::string const& key, std::string const& confirmKey)
 {
     return key == confirmKey;
 }

CodeReview

This is your header?

#include "sha256.h"

Doing sha256 is non-trivial, and I would recommend you leave that to experts. As such, I would expect this to be an installed library and thus be included with <> i.e. #include <sha256.h>

---

Don't do this.

using namespace std;

This is going to get you in trouble in the long run. You should read the article Why is "using namespace std;" considered bad practice? In my opinion, the second answer is the best.

Rather than doing this, prefix standard library types objects with std::. It's only 5 characters, and the name std was designed to be short for that reason.

---

Please, Please, make sure your indentation is good. This makes it really hard to read.

bool keyExists() {
bool keyExists = false;
ifstream inFile("key.txt");

---

All this function does is check that the file "key.txt" exists and is openable. You don't do anything with that information in the file!

bool keyExists() {
bool keyExists = false;         // Not to keen on the variable haveing
                                // the same name as the function.
                                // Feels like you will get confused reading
                                // this.
ifstream inFile("key.txt");


if (inFile) {
    keyExists = true;
}

return keyExists;
}

We can shorten this function considerably if that is all you want to do.

bool doesKeyFileExist()
{
    std::ifstream inFile("key.txt");
    bool keyFileExist = inFile;   // there is an auto conversion from
                                  // ifstream to bool (which you use in
                                  // your if (inFile). You don't need
                                  // to be so roundabout. Just check the
                                  // result and return it.
    return keyFileExist;  
}

---

This is an antipattern

bool match = false;
if (key == confirmKey) {
    match = true;
}

If you see code like:

bool test;

if (>checkSomeCondition<) {
    test = true;
}
else {
    test = false;
}

This is just long hand notation of finding the value of >checkSomeCondition< as this must be a boolean value (or convertible to boolean). So you can simply assign this condition to your variable:

bool test = >checkSomeCondition<;

Does exactly the same thing.

So your code can be re-written as:

 bool match = (key == confirmKey);

---

Put preconditions checks first and bail out if the preconditions fail:

 // i.e.
 if (>CheckConditionIsOK<) {

      // Do all the work
      // this could be long
 }
 else {
      // Print error message
 }

I would reorder that like this:

 if (! >CheckConditionIsOK<) {
      // Print error message
      // Your pre-condition check has not worked so bail.
      return; // or throw exception.
 }

 // This part of the code is only executed if your precondition
 // are all valid. But they are not indented now. So it looks like
 // the main part of the work.

 // Do all the work
 // this could be long

---

Declare variables as close to the point of use as possible.

    string key;
    string storedKey;
    cout << "Please enter key: ";
    getline(cin, key);

    SHA256 sha256;
    ifstream inFile("key.txt");
    getline(inFile, storedKey);

    if (isMatch(sha256(key), storedKey)) {

I would re-order this a bit:

    std::cout << "Please enter key: ";

    std::string key;
    std::getline(std::cin, key);


    std::ifstream inFile("key.txt");
    std::string storedKey;
    std::getline(inFile, storedKey);


    SHA256 sha256;
    if (isMatch(sha256(key), storedKey)) {

---