diff --git a/keychain.h b/keychain.h index 7b62695..24e41f3 100644 --- a/keychain.h +++ b/keychain.h @@ -27,36 +27,105 @@ #include +/*! \brief A thin wrapper to provide cross-platform access to the operating + * system's credentials storage. + * + * keychain provides the functions getPassword, setPassword, and deletePassword. + * + * All of these functions require three input parameters to identify the + * credential that should be retrieved or manipulated: `package`, `service`, and + * `user`. These identifiers will be mangled differently on each OS to + * correspond to the OS API. + * While none of the supported OSes has specific requirements to the format + * identifiers, the reverse domain name format is recommended for the `package` + * parameter in order to correspond with conventions. + * + * In addition, each function expects an instance of `keychain::Error` as an + * output parameter to indicate success or failure. Note that the error + * parameter must not already be set to an error at the time the function is + * called. + * + * Also note that all three functions are blocking (potentially indefinitely) + * for example if the OS prompts the user to unlock their credentials storage. + */ namespace keychain { -enum class KeychainError { +struct Error; + +/*! \brief Retrieve a password + * + * \param package, service, user Used to identify the password to get + * \param err Output parameter set to indicate what error occurred, if any + * + * \return The password, if the function was successful + */ +std::string getPassword(const std::string &package, const std::string &service, + const std::string &user, Error &err); + +/*! \brief Insert or update a password + * + * Existing passwords will be overwritten. + * + * \param package, service, user Used to identify the password to get + * \param password The new password + * \param err Output parameter set to indicate what error occurred, if any + */ +void setPassword(const std::string &package, const std::string &service, + const std::string &user, const std::string &password, + Error &err); + +/*! \brief Insert or update a password + * + * Trying to delete a password that does not exist will result in a NotFound + * error. + * + * \param package, service, user Used to identify the password to get + * \param err Output parameter set to indicate what error occurred, if any + */ +void deletePassword(const std::string &package, const std::string &service, + const std::string &user, Error &err); + +enum class ErrorType { // update CATCH_REGISTER_ENUM in tests.cpp when changing this NoError = 0, GenericError, NotFound, // OS-specific errors PasswordTooLong = 10, // Windows only - AccessDenied, // MacOS only (TODO) + AccessDenied, // macOS only }; +/*! \brief A struct to collect error information + * + * An instance of this struct is used as an output parameter to indicate success + * or failure. Note that Errors should not already contain an error when handed + * to a function. + */ struct Error { - KeychainError error; + /*! \brief The type or reason of the error + * + * Note that some types of errors can only occur on certain platforms. In + * cases where a platform-specific error occurs on one platform, both + * NoError or some other (more generic) error might occur on others. + */ + ErrorType type; + + /*! \brief A human-readable explanatory error message + * + * In most cases this message is obtained from the operating system. + */ std::string message; + + /*! \brief The "native" error code set by the operating system + * + * Even for the same type of error this value will differ across platforms. + */ int code; - operator bool() const { return KeychainError::NoError != error; } + //! \brief Checks if the error type is not NoError + operator bool() const { return ErrorType::NoError != type; } }; -std::string getPassword(const std::string &package, const std::string &service, - const std::string &user, Error &err); - -void setPassword(const std::string &package, const std::string &service, - const std::string &user, const std::string &password, - Error &err); - -void deletePassword(const std::string &package, const std::string &service, - const std::string &user, Error &err); - } // namespace keychain #endif diff --git a/keychain_linux.cpp b/keychain_linux.cpp index ed4c705..b6b35ae 100644 --- a/keychain_linux.cpp +++ b/keychain_linux.cpp @@ -28,7 +28,7 @@ #include -namespace keychain { +namespace { const char *ServiceFieldName = "service"; const char *AccountFieldName = "username"; @@ -64,12 +64,18 @@ void updateError(keychain::Error &err, GError *error) { return; } - err.error = keychain::KeychainError::GenericError; + err.type = keychain::ErrorType::GenericError; err.message = error->message; err.code = error->code; g_error_free(error); } +void setErrorNotFound(keychain::Error &err) { + err.type = keychain::ErrorType::NotFound; + err.message = "Password not found."; + err.code = -1; // generic non-zero +} + } // namespace namespace keychain { @@ -111,6 +117,7 @@ std::string getPassword(const std::string &package, const std::string &service, user.c_str(), NULL); + updateError(err, error); std::string password; if (!err && raw_passwords != NULL) { @@ -118,9 +125,7 @@ std::string getPassword(const std::string &package, const std::string &service, secret_password_free(raw_passwords); } else if (raw_passwords == NULL) { // libsecret reports no error if the password was not found - err.error = KeychainError::NotFound; - err.message = "Password not found."; - err.code = -1; // generic non-zero + setErrorNotFound(err); } return password; @@ -131,16 +136,21 @@ void deletePassword(const std::string &package, const std::string &service, const auto schema = makeSchema(package); GError *error = NULL; - secret_password_clear_sync(&schema, - NULL, // not cancellable - &error, - ServiceFieldName, - service.c_str(), - AccountFieldName, - user.c_str(), - NULL); + bool deleted = secret_password_clear_sync(&schema, + NULL, // not cancellable + &error, + ServiceFieldName, + service.c_str(), + AccountFieldName, + user.c_str(), + NULL); updateError(err, error); + + if (!err && !deleted) { + // password did not exist is considered an error + setErrorNotFound(err); + } } } // namespace keychain diff --git a/keychain_mac.cpp b/keychain_mac.cpp index 6c9eed5..6ccdeea 100644 --- a/keychain_mac.cpp +++ b/keychain_mac.cpp @@ -33,8 +33,7 @@ namespace { const SecKeychainRef defaultUserKeychain = NULL; // NULL means 'default' -/*! - * Converts a CFString to a std::string +/*! \brief Converts a CFString to a std::string * * This either uses CFStringGetCStringPtr or (if that fails) CFStringGetCString. */ @@ -56,9 +55,7 @@ std::string CFStringToStdString(const CFStringRef cfstring) { return result ? std::string(cstr.data()) : std::string(); } -/*! - * Extracts a human readable string from a status code - */ +//! \brief Extracts a human readable string from a status code std::string errorStatusToString(OSStatus status) { const auto errorMessage = SecCopyErrorMessageString(status, NULL); std::string errorString; @@ -76,8 +73,7 @@ std::string makeServiceName(const std::string &package, return package + "." + service; } -/*! - * Update error information +/*! \brief Update error information * * If status indicates an error condition, set message, code and error type. * Otherwise, set err to success. @@ -89,14 +85,26 @@ void updateError(keychain::Error &err, OSStatus status) { } err.message = errorStatusToString(status); - err.code = status; // TODO check conversion - err.error = status == errSecItemNotFound - ? keychain::KeychainError::NotFound - : keychain::KeychainError::GenericError; + err.code = status; + + switch (status) { + case errSecItemNotFound: + err.type = keychain::ErrorType::NotFound; + break; + + // potential errors in case the user needs to unlock the keychain first + case errSecUserCanceled: // user pressed the Cancel button + case errSecAuthFailed: // too many failed password attempts + case errSecInteractionRequired: // user interaction required but not allowed + err.type = keychain::ErrorType::AccessDenied; + break; + + default: + err.type = keychain::ErrorType::GenericError; + } } -/*! - * Modify an existing password +/*! \brief Modify an existing password * * Helper function that tries to find an existing password in the keychain and * modifies it. diff --git a/keychain_win.cpp b/keychain_win.cpp index 6703f90..1c59691 100644 --- a/keychain_win.cpp +++ b/keychain_win.cpp @@ -47,8 +47,7 @@ struct LpwstrDeleter { //! Wrapper around a WCHAR pointer a.k.a. LPWStr to take care of memory handling using ScopedLpwstr = std::unique_ptr; -/*! - * Converts a UTF-8 std::string to wide char +/*! \brief Converts a UTF-8 std::string to wide char * * Uses MultiByteToWideChar to convert the input string and wraps the result in * a ScopedLpwstr. Returns nullptr on failure. @@ -80,8 +79,7 @@ ScopedLpwstr utf8ToWideChar(const std::string &utf8) { return lwstr; } -/*! - * Converts a wide char pointer to a std::string +/*! \brief Converts a wide char pointer to a std::string * * Note that this function provides no reliable indication of errors and simply * returns an empty string in case it fails. @@ -118,7 +116,9 @@ std::string wideCharToAnsi(LPWSTR wChar) { return result; } -//! Get an explanatory message for an error code obtained via ::GetLastError() +/*! /brief Get an explanatory message for an error code obtained via + * ::GetLastError() + */ std::string getErrorMessage(DWORD errorCode) { std::string errMsg; LPWSTR errBuffer = nullptr; @@ -147,13 +147,11 @@ void updateError(keychain::Error &err) { err.message = getErrorMessage(code); err.code = code; - err.error = err.code == ERROR_NOT_FOUND - ? keychain::KeychainError::NotFound - : keychain::KeychainError::GenericError; + err.type = err.code == ERROR_NOT_FOUND ? keychain::ErrorType::NotFound + : keychain::ErrorType::GenericError; } -/*! - * Create the target name used to lookup and store credentials +/*! /brief Create the target name used to lookup and store credentials * * The result is wrapped in a ScopedLpwstr. */ @@ -186,7 +184,7 @@ void setPassword(const std::string &package, const std::string &service, if (password.size() > CRED_MAX_CREDENTIAL_BLOB_SIZE || password.size() > DWORD_MAX) { - err.error = KeychainError::PasswordTooLong; + err.type = ErrorType::PasswordTooLong; err.message = "Password too long."; err.code = -1; // generic non-zero return; diff --git a/test/tests.cpp b/test/tests.cpp index 120e6bd..80614df 100644 --- a/test/tests.cpp +++ b/test/tests.cpp @@ -3,15 +3,18 @@ using namespace keychain; -CATCH_REGISTER_ENUM(keychain::KeychainError, keychain::KeychainError::NoError, - keychain::KeychainError::GenericError, - keychain::KeychainError::NotFound, - keychain::KeychainError::PasswordTooLong, - keychain::KeychainError::AccessDenied) +// clang-format off +CATCH_REGISTER_ENUM(keychain::ErrorType, + keychain::ErrorType::NoError, + keychain::ErrorType::GenericError, + keychain::ErrorType::NotFound, + keychain::ErrorType::PasswordTooLong, + keychain::ErrorType::AccessDenied) +// clang-format on void check_no_error(const Error &ec) { const std::string error = - Catch::StringMaker::convert(ec.error); + Catch::StringMaker::convert(ec.type); INFO(error << " [" << ec.code << "] " << ": " << ec.message); CHECK(!ec); @@ -24,7 +27,7 @@ TEST_CASE("Keychain", "[keychain]") { const std::string &password_in) { Error ec{}; getPassword(package, service, user, ec); - REQUIRE(ec.error == KeychainError::NotFound); + REQUIRE(ec.type == ErrorType::NotFound); ec = Error{}; setPassword(package, service, user, password_in, ec); @@ -51,7 +54,7 @@ TEST_CASE("Keychain", "[keychain]") { check_no_error(ec); ec = Error{}; getPassword(package, service, user, ec); - CHECK(ec.error == KeychainError::NotFound); + CHECK(ec.type == ErrorType::NotFound); }; const std::string package = "com.example.keychain-tests"; @@ -73,11 +76,11 @@ TEST_CASE("Keychain", "[keychain]") { const std::string longPassword(4097, '='); Error ec{}; getPassword(package, service, user, ec); - REQUIRE(ec.error == KeychainError::NotFound); + REQUIRE(ec.type == ErrorType::NotFound); ec = Error{}; setPassword(package, service, user, longPassword, ec); - CHECK(ec.error == KeychainError::PasswordTooLong); + CHECK(ec.type == ErrorType::PasswordTooLong); } #else SECTION("long password (unix)") { @@ -87,4 +90,10 @@ TEST_CASE("Keychain", "[keychain]") { #endif SECTION("unicode") { crud("🙈.🙉.🙊", "💛", "👩💻", "🔑"); } + + SECTION("deleting a password that does not exist results in NotFound") { + Error ec{}; + deletePassword("no.package", "no.service", "no.user", ec); + CHECK(ec.type == ErrorType::NotFound); + } }