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

cool#10630 doc electronic sign: moved esign settings out of UserPrivateInfo #10734

Merged
merged 2 commits into from
Dec 16, 2024

Conversation

vmiklos
Copy link
Contributor

@vmiklos vmiklos commented Dec 16, 2024

  • cool#10630 doc electronic sign: moved esign settings out of UserPrivateInfo
  • cool#10630 doc electronic sign: support per-server private info during 'make run'

…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
…g 'make run'

Now that esign settings are no longer under UserPrivateInfo,
cypress-deskop's draw/esign_spec.js can't set the esign settings for the
test.

The problem is that ServerPrivateInfo's sub-keys should be set, but the
JSON file based mechanism for 'make run' can only set UserPrivateInfo
sub-keys.

Fix this by loading all keys from a <document>.wopi.json file (if
exists) and then keep setting the rest of the keys from code, as before.

This allows setting UserPrivateInfo, ServerPrivateInfo and other
toplevel keys from .wopi.json files next to documents.

Signed-off-by: Miklos Vajna <vmiklos@collabora.com>
Change-Id: Iacbe7aa438d5b30170c96e23df069ec99e496945
@vmiklos vmiklos changed the title private/vmiklos/master cool#10630 doc electronic sign: moved esign settings out of UserPrivateInfo Dec 16, 2024
@vmiklos vmiklos requested a review from caolanm December 16, 2024 08:06
@vmiklos
Copy link
Contributor Author

vmiklos commented Dec 16, 2024

@caolanm could you please review this? Thanks.

Julius suggests to not have the esign settings in UserPrivateInfo, since those are "nextcloud instance-wide" settings, configured by admins, which is fair enough. I don't want to route each & every per-server setting through the various layers, so I suggest to go with a ServerPrivateInfo key to host such per-server credentials. [MS-WOPI] doesn't seem to have something similar, but at the same time ServerPrivateInfo is then not a name they would use, so it's probably safe to take.

Copy link
Contributor

@caolanm caolanm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair enough then

@caolanm caolanm merged commit 8d043c3 into master Dec 16, 2024
14 checks passed
@caolanm caolanm deleted the private/vmiklos/master branch December 16, 2024 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants