Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed some TODOs #304

Merged
merged 11 commits into from
Jun 21, 2024
2 changes: 1 addition & 1 deletion .github/workflows/cmake-windows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ jobs:
with:
vcpkgArguments: gtest openssl
vcpkgTriplet: x64-windows
vcpkgGitCommitId: 9b9c2758ece1d8ac0de90589730bb5ccf45c0874
vcpkgGitCommitId: 98a562a04cd03728f399e79e1b37bcccb5a69b37

- name: Install Qt
uses: jurplel/install-qt-action@v3
Expand Down
17 changes: 14 additions & 3 deletions src/controller/controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,8 @@ using namespace electronic_id;
namespace
{

// TODO: Should we use more detailed error codes? E.g. report input data error back to the website
// etc.
const QString RESP_TECH_ERROR = QStringLiteral("ERR_WEBEID_NATIVE_FATAL");
const QString RESP_INVALID_INVOCATION = QStringLiteral("ERR_WEBEID_INVALID_INVOCATION");
const QString RESP_USER_CANCEL = QStringLiteral("ERR_WEBEID_USER_CANCELLED");

QVariantMap makeErrorObject(const QString& errorCode, const QString& errorMessage)
Expand Down Expand Up @@ -102,7 +101,8 @@ void Controller::run()
commandHandler = getCommandHandler(*command);

startCommandExecution();

} catch (const std::invalid_argument& error) {
onInvalidInvocation(error.what());
} catch (const std::exception& error) {
onCriticalFailure(error.what());
}
Expand Down Expand Up @@ -361,6 +361,17 @@ void Controller::onCriticalFailure(const QString& error)
exit();
}

void Controller::onInvalidInvocation(const QString& error)
{
qCritical() << "Invalid arguments to command" << std::string(commandType())
<< "reason:" << error;
_result = makeErrorObject(RESP_INVALID_INVOCATION, error);
writeResponseToStdOut(isInStdinMode, _result, commandType());
disposeUI();
WebEidUI::showFatalError();
exit();
}
lauris71 marked this conversation as resolved.
Show resolved Hide resolved

void Controller::exit()
{
if (window) {
Expand Down
2 changes: 2 additions & 0 deletions src/controller/controller.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ class Controller : public QObject

// Failure handler, reports the error and quits the application.
void onCriticalFailure(const QString& error);
// Invalid arguments, we keep it spearate to distinguish it from technical errors
void onInvalidInvocation(const QString& error);

private:
void startCommandExecution();
Expand Down
14 changes: 7 additions & 7 deletions src/ui/webeiddialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -546,10 +546,12 @@ void WebEidDialog::connectOkToCachePinAndEmitSelectedCertificate(
// QString uses QAtomicPointer internally and is thread-safe.
pin = ui->pinInput->text();

// TODO: We need to erase the PIN in the widget buffer, this needs further work.
// Investigate if it is possible to keep the PIN in secure memory, e.g. with a
// custom Qt widget.
ui->pinInput->clear();
// We use setText instead of clear, so undo/redo will be cleared as well
lauris71 marked this conversation as resolved.
Show resolved Hide resolved
ui->pinInput->setText({});
// TODO: To be sure that no copy of PIN text remains in memory, a custom
// widget should be implemented, that:
// - Stores PIN in locked byte vector
// - DOes not leak content through accessibility interface
lauris71 marked this conversation as resolved.
Show resolved Hide resolved

emit accepted(certAndPin);
});
Expand Down Expand Up @@ -728,13 +730,11 @@ WebEidDialog::retriableErrorToTextTitleAndIcon(const RetriableError error)
tr("Operation not supported"), pixmap("no-id-card"_L1)};

case RetriableError::SMART_CARD_COMMAND_ERROR:
return {tr("Error communicating with the card."), tr("Operation failed"),
return {tr("Error communicating with the card. Please try again."), tr("Operation failed"),
lauris71 marked this conversation as resolved.
Show resolved Hide resolved
pixmap("no-id-card"_L1)};
// TODO: what action should the user take? Should this be fatal?
case RetriableError::PKCS11_ERROR:
return {tr("Card driver error. Please try again."), tr("Card driver error"),
pixmap("no-id-card"_L1)};
// TODO: what action should the user take? Should this be fatal?
case RetriableError::SCARD_ERROR:
return {tr("An error occurred in the Smart Card service required to use the ID-card. Make "
"sure that the ID-card and the card reader are connected correctly or relaunch "
Expand Down