Skip to content

Commit

Permalink
cool#10630 doc electronic sign: moved esign settings out of UserPriva…
Browse files Browse the repository at this point in the history 
…teInfo

Electronic sign settings are provided by the integration, and they are
an admin setting there, so they are shared between users. Based on this,
it was confusing to have them in UserPrivateInfo.

UserPrivateInfo was handy as we had infrastructure there to replace
values of keys with placeholders where the value is not to be sent to
the browser, but otherwise it was indeed a workaround.

Solve this by introducing a new ServerPrivateInfo top-level key:
currently this hosts the 3 esign settings, but it could host more in the
future. Not having esign settings in UserPrivateInfo was explicitly
requrested by Nextcloud.

Disable the esign test for now, since currently 'make run' can only set
UserPrivateInfo keys from JSON files next to documents, that's still to
be extended.

Signed-off-by: Miklos Vajna <vmiklos@collabora.com>
Change-Id: I073886ce6811605a1678b77241b14f5055be0dbe
  • Loading branch information
@vmiklos @caolanm
vmiklos authored and caolanm committed Dec 16, 2024
1 parent b2679c5 commit e1e6a08
Show file tree
Hide file tree
Showing 9 changed files with 93 additions and 53 deletions.
19 changes: 9 additions & 10 deletions browser/src/control/Control.UIManager.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1071,11 +1071,7 @@ L.Control.UIManager = L.Control.extend({
}

var userPrivateInfo = myViewData.userprivateinfo;
if (userPrivateInfo === undefined)
{
return;
}
if (window.zoteroEnabled) {
if (userPrivateInfo && window.zoteroEnabled) {
var apiKey = userPrivateInfo.ZoteroAPIKey;
if (apiKey !== undefined && !this.map.zotero) {
this.map.zotero = L.control.zotero(this.map);
Expand All @@ -1085,16 +1081,19 @@ L.Control.UIManager = L.Control.extend({
}
}
if (window.documentSigningEnabled) {
if (this.notebookbar) {
if (userPrivateInfo && this.notebookbar) {
const show = userPrivateInfo.SignatureCert && userPrivateInfo.SignatureKey;
// Show or hide the signature button on the notebookbar depending on if we
// have a signing cert/key specified.
this.showButton('signature', show);
}
const baseUrl = userPrivateInfo.ESignatureBaseUrl;
const clientId = userPrivateInfo.ESignatureClientId;
if (baseUrl !== undefined && !this.map.eSignature) {
this.map.eSignature = L.control.eSignature(baseUrl, clientId);
const serverPrivateInfo = myViewData.serverprivateinfo;
if (serverPrivateInfo) {
const baseUrl = serverPrivateInfo.ESignatureBaseUrl;
const clientId = serverPrivateInfo.ESignatureClientId;
if (baseUrl !== undefined && !this.map.eSignature) {
this.map.eSignature = L.control.eSignature(baseUrl, clientId);
}
}
}
},
Expand Down
5 changes: 5 additions & 0 deletions common/Session.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,11 @@ void Session::parseDocOptions(const StringVector& tokens, int& part, std::string
_userPrivateInfo = Uri::decode(value);
++offset;
}
else if (name == "serverprivateinfo")
{
_serverPrivateInfo = Uri::decode(value);
++offset;
}
else if (name == "readonly")
{
_isReadOnly = value != "0";
Expand Down
7 changes: 7 additions & 0 deletions common/Session.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,8 @@ class Session : public MessageHandlerInterface

void setUserPrivateInfo(const std::string& userPrivateInfo) { _userPrivateInfo = userPrivateInfo; }

void setServerPrivateInfo(const std::string& serverPrivateInfo) { _serverPrivateInfo = serverPrivateInfo; }

void setUserName(const std::string& userName) { _userName = userName; }

const std::string& getUserName() const {return _userName; }
Expand Down Expand Up @@ -260,6 +262,8 @@ class Session : public MessageHandlerInterface

const std::string& getUserPrivateInfo() const { return _userPrivateInfo; }

const std::string& getServerPrivateInfo() const { return _serverPrivateInfo; }

const std::string& getDocURL() const { return _docURL; }

const std::string& getJailedFilePath() const { return _jailedFilePath; }
Expand Down Expand Up @@ -382,6 +386,9 @@ class Session : public MessageHandlerInterface
/// Private info per user, not shared with others.
std::string _userPrivateInfo;

/// Private info per server, shared with others.
std::string _serverPrivateInfo;

/// In case a watermark has to be rendered on each tile.
std::string _watermarkText;

Expand Down
2 changes: 1 addition & 1 deletion cypress_test/integration_tests/desktop/draw/esign_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ var helper = require('../../common/helper');

describe(['tagdesktop'], 'Electronic sign operations.', function() {

it('Create an electronic signature.', { env: { 'pdf-view': true } }, function() {
it.skip('Create an electronic signature.', { env: { 'pdf-view': true } }, function() {
// Given a document that can be signed:
helper.setupAndLoadDocument('draw/esign.pdf', /*isMultiUser=*/false, /*copyCertificates=*/true);

Expand Down
90 changes: 53 additions & 37 deletions kit/Kit.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1641,6 +1641,40 @@ void Document::reapZombieChildren()
}
}

namespace
{
// No need to actually send the values of some keys to the client, it's enough to know if these are
// provided or not. Replace the actual content with a placeholder.
void replaceKeysWithPlaceholder(std::string& json, std::initializer_list<std::string>& keys)
{
try
{
if (!json.empty())
{
Parser parser;
Poco::Dynamic::Var var = parser.parse(json);
Object::Ptr jsonObj = var.extract<Object::Ptr>();
for (const auto& key : keys)
{
std::string value;
JsonUtil::findJSONValue(jsonObj, key, value);
if (!value.empty())
{
jsonObj->set(key, " ");
}
}
std::ostringstream jsonStream;
jsonObj->stringify(jsonStream);
json = jsonStream.str();
}
}
catch(const Poco::BadCastException& exception)
{
LOG_DBG("user private data is not a dictionary: " << exception.what());
}
}
}

void Document::notifyViewInfo()
{
// Get the list of view ids from the core
Expand Down Expand Up @@ -1697,49 +1731,31 @@ void Document::notifyViewInfo()

for (const auto& viewId : viewIds)
{
if (viewId == it.second->getViewId() && !it.second->getUserPrivateInfo().empty())
oss << "{" << viewStrings[viewId];
if (viewId == it.second->getViewId())
{
oss << "{" << viewStrings[viewId];
std::string userPrivateInfo = it.second->getUserPrivateInfo();
try
if (!it.second->getUserPrivateInfo().empty())
{
if (!userPrivateInfo.empty())
{
// No need to actually send the signing certs/keys to the client, it's
// enough to know if these are provided or not. Replace the actual content
// with a placeholder.
Parser parser;
Poco::Dynamic::Var var = parser.parse(userPrivateInfo);
Object::Ptr userPrivateInfoObj = var.extract<Object::Ptr>();
std::initializer_list<std::string> keys = {
"SignatureCert",
"SignatureKey",
"SignatureCa",
"ESignatureSecret",
};
for (const auto& key : keys)
{
std::string value;
JsonUtil::findJSONValue(userPrivateInfoObj, key, value);
if (!value.empty())
{
userPrivateInfoObj->set(key, " ");
}
}
std::ostringstream userPrivateStream;
userPrivateInfoObj->stringify(userPrivateStream);
userPrivateInfo = userPrivateStream.str();
}
std::string userPrivateInfo = it.second->getUserPrivateInfo();
std::initializer_list<std::string> keys = {
"SignatureCert",
"SignatureKey",
"SignatureCa",
};
replaceKeysWithPlaceholder(userPrivateInfo, keys);
oss << ",\"userprivateinfo\":" << userPrivateInfo;
}
catch(const Poco::BadCastException& exception)
if (!it.second->getServerPrivateInfo().empty())
{
LOG_DBG("user private data is not a dictionary: " << exception.what());
std::string serverPrivateInfo = it.second->getServerPrivateInfo();
std::initializer_list<std::string> keys = {
"ESignatureSecret",
};
replaceKeysWithPlaceholder(serverPrivateInfo, keys);
oss << ",\"serverprivateinfo\":" << serverPrivateInfo;
}
oss << ",\"userprivateinfo\":" << userPrivateInfo;
oss << "},";
}
else
oss << "{" << viewStrings[viewId] << "},";
oss << "},";
}

if (viewCount > 0)
Expand Down
17 changes: 12 additions & 5 deletions wsd/ClientSession.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -475,14 +475,14 @@ bool ClientSession::handleSignatureAction(const StringVector& tokens)
{
const std::string commandName = tokens[1];
// Make the HTTP session: this requires an URL
Poco::JSON::Object::Ptr userPrivateInfoObject = new Poco::JSON::Object();
if (!JsonUtil::parseJSON(getUserPrivateInfo(), userPrivateInfoObject))
Poco::JSON::Object::Ptr serverPrivateInfoObject = new Poco::JSON::Object();
if (!JsonUtil::parseJSON(getServerPrivateInfo(), serverPrivateInfoObject))
{
LOG_WRN("SignatureAction: failed to parse user private info as JSON");
LOG_WRN("SignatureAction: failed to parse server private info as JSON");
return false;
}
std::string requestUrl;
JsonUtil::findJSONValue(userPrivateInfoObject, "ESignatureBaseUrl", requestUrl);
JsonUtil::findJSONValue(serverPrivateInfoObject, "ESignatureBaseUrl", requestUrl);
if (commandName == ".uno:PrepareSignature")
{
requestUrl += "/api/signatures/prepare-files-for-signing";
Expand Down Expand Up @@ -512,7 +512,7 @@ bool ClientSession::handleSignatureAction(const StringVector& tokens)
return false;
}
std::string secret;
JsonUtil::findJSONValue(userPrivateInfoObject, "ESignatureSecret", secret);
JsonUtil::findJSONValue(serverPrivateInfoObject, "ESignatureSecret", secret);
requestBodyObject->set("secret", secret);
std::string requestBody;
std::stringstream oss;
Expand Down Expand Up @@ -1414,6 +1414,13 @@ bool ClientSession::loadDocument(const char* /*buffer*/, int /*length*/,
oss << " authorprivateinfo=" << encodedUserPrivateInfo;
}

if (!getServerPrivateInfo().empty())
{
std::string encodedServerPrivateInfo;
Poco::URI::encode(getServerPrivateInfo(), "", encodedServerPrivateInfo);
oss << " serverprivateinfo=" << encodedServerPrivateInfo;
}

oss << " readonly=" << isReadOnly();

if (isAllowChangeComments())
Expand Down
2 changes: 2 additions & 0 deletions wsd/DocumentBroker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1287,6 +1287,7 @@ DocumentBroker::updateSessionWithWopiInfo(const std::shared_ptr<ClientSession>&
const std::string username = wopiFileInfo->getUsername();
const std::string userExtraInfo = wopiFileInfo->getUserExtraInfo();
const std::string userPrivateInfo = wopiFileInfo->getUserPrivateInfo();
const std::string serverPrivateInfo = wopiFileInfo->getServerPrivateInfo();
const std::string watermarkText =
(ConfigUtil::isSupportKeyEnabled() && !COOLWSD::OverrideWatermark.empty())
? COOLWSD::OverrideWatermark
Expand Down Expand Up @@ -1442,6 +1443,7 @@ DocumentBroker::updateSessionWithWopiInfo(const std::shared_ptr<ClientSession>&
session->setUserExtraInfo(userExtraInfo);
session->setIsAdminUser(isAdminUser);
session->setUserPrivateInfo(userPrivateInfo);
session->setServerPrivateInfo(serverPrivateInfo);
session->setWatermarkText(watermarkText);

return templateSource;
Expand Down
1 change: 1 addition & 0 deletions wsd/wopi/WopiStorage.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ WopiStorage::WOPIFileInfo::WOPIFileInfo(const FileInfo& fileInfo, Poco::JSON::Ob

JsonUtil::findJSONValue(object, "UserExtraInfo", _userExtraInfo);
JsonUtil::findJSONValue(object, "UserPrivateInfo", _userPrivateInfo);
JsonUtil::findJSONValue(object, "ServerPrivateInfo", _serverPrivateInfo);
JsonUtil::findJSONValue(object, "WatermarkText", _watermarkText);
JsonUtil::findJSONValue(object, "UserCanWrite", _userCanWrite);
JsonUtil::findJSONValue(object, "PostMessageOrigin", _postMessageOrigin);
Expand Down
3 changes: 3 additions & 0 deletions wsd/wopi/WopiStorage.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ class WopiStorage : public StorageBase
const std::string& getUsername() const { return _username; }
const std::string& getUserExtraInfo() const { return _userExtraInfo; }
const std::string& getUserPrivateInfo() const { return _userPrivateInfo; }
const std::string& getServerPrivateInfo() const { return _serverPrivateInfo; }
const std::string& getWatermarkText() const { return _watermarkText; }
const std::string& getTemplateSaveAs() const { return _templateSaveAs; }
const std::string& getTemplateSource() const { return _templateSource; }
Expand Down Expand Up @@ -102,6 +103,8 @@ class WopiStorage : public StorageBase
std::string _userExtraInfo;
/// Private info per user, for API keys and other non-public information.
std::string _userPrivateInfo;
/// Private info per server, for API keys and other non-public information.
std::string _serverPrivateInfo;
/// In case a watermark has to be rendered on each tile.
std::string _watermarkText;
/// In case we want to use this file as a template, it should be first re-saved under this name (using PutRelativeFile).
Expand Down

0 comments on commit e1e6a08

Please sign in to comment.