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
[sailfish-secrets] Add support for InteractionRequest. Contributes to JB#36797 #22
Conversation
asynchronousCryptoRequestCompleted(request->cryptoRequestId, result, QVariantList() << userInput); | ||
} else { | ||
// shouldn't happen! | ||
qCWarning(lcSailfishSecretsDaemon) << "Error: UserInputRequest not a secrets crypto request!"; | ||
request->connection.send(request->message.createReply() << QVariant::fromValue<Result>(result) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, it was "should not happen" earlier and now it is okay? Needs some explanation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because before, the only codepath the UserInputRequest was used in, was as an asynchronous crypto request (i.e. when getting input key data for a key derivation request). Now it is used also for "standalone" secrets request (i.e. InteractionRequest).
* // Perform the request. | ||
* Sailfish::Secrets::SecretManager sm; | ||
* Sailfish::Secrets::InteractionRequest ir; | ||
* ir.setManager(&sm); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to me that ir.setInteractionParameters() is missing here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, thanks will fix
/*! | ||
* \brief Sets the interaction parameters which allow customisation of the input prompt to \a uiParams | ||
*/ | ||
void InteractionRequest::setInteractionParameters(const InteractionParameters &uiParams) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is it called set InteractionParameters() while it is called setUiParameters() in storesecretrequest?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. We should standardise. I guess we should use setInteractionParameters() everywhere for consistency. I will update those other cases.
… JB#36797 Some client use cases require the capability to request user input via a system-mediated UI where that input is NOT stored into the secrets storage (for example, a daemon which wishes to use a transient lock code for some functionality). This commit adds support for an InteractionRequest where the user is explicitly informed that the data they are entering should not be considered "secure" in the secrets-storage sense. Contributes to JB#36797
f7073ef
to
425b10f
Compare
Some client use cases require the capability to request user input
via a system-mediated UI where that input is NOT stored into the
secrets storage (for example, a daemon which wishes to use a transient
lock code for some functionality).
This commit adds support for an InteractionRequest where the user
is explicitly informed that the data they are entering should not
be considered "secure" in the secrets-storage sense.
Contributes to JB#36797