* [pbs-devel] [PATCH backup/proxmox-backup 0/4] fix #5463: add optional consent banner before login
@ 2024-05-22 13:19 Gabriel Goller
2024-05-22 13:19 ` [pbs-devel] [PATCH proxmox-backup 1/4] api: add consent api handler and config Gabriel Goller
` (5 more replies)
0 siblings, 6 replies; 15+ messages in thread
From: Gabriel Goller @ 2024-05-22 13:19 UTC (permalink / raw)
To: pbs-devel
This has been requested many times already for all products. To keep
it simple as it's still quite a niche feature, works with a single file:
/etc/proxmox-backup/consent.txt. If the file exists and is not empty,
a consent banner will be shown in front of the login view.
This is just a reference implementation for pbs to get some feedback
and check if my general approach is alright. The same implementation
will then be ported to pve and eventually pmg.
Another disclaimer: IANAL (I am not a lawyer :) ), so the wording is
probably off.
v1, thanks @Dominik:
- embed consent text into index.html file instead of extra api request
- removed decline button
- added alwaysOnTop property to popup
widget-toolkit:
Gabriel Goller (3):
api: add consent api handler and config
ui: show consent banner before login
docs: add section about consent banner
docs/gui.rst | 7 +++++++
src/bin/proxmox-backup-proxy.rs | 6 ++++++
src/config/consent.rs | 11 +++++++++++
src/config/mod.rs | 1 +
www/LoginView.js | 9 +++++++++
www/index.hbs | 1 +
6 files changed, 35 insertions(+)
create mode 100644 src/config/consent.rs
backup:
Gabriel Goller (1):
window: add consent modal
src/Makefile | 1 +
src/window/ConsentModal.js | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 35 insertions(+)
create mode 100644 src/window/ConsentModal.js
Summary over all repositories:
8 files changed, 70 insertions(+), 0 deletions(-)
--
Generated by git-murpp 0.5.0
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 1/4] api: add consent api handler and config
2024-05-22 13:19 [pbs-devel] [PATCH backup/proxmox-backup 0/4] fix #5463: add optional consent banner before login Gabriel Goller
@ 2024-05-22 13:19 ` Gabriel Goller
2024-05-22 13:19 ` [pbs-devel] [PATCH proxmox-backup 2/4] ui: show consent banner before login Gabriel Goller
` (4 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Gabriel Goller @ 2024-05-22 13:19 UTC (permalink / raw)
To: pbs-devel
Add config function to retrieve consent text from file and add it to
template function.
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
src/bin/proxmox-backup-proxy.rs | 6 ++++++
src/config/consent.rs | 11 +++++++++++
src/config/mod.rs | 1 +
3 files changed, 18 insertions(+)
create mode 100644 src/config/consent.rs
diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index 15444685..16b7fb6c 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -152,6 +152,11 @@ async fn get_index_future(env: RestEnvironment, parts: Parts) -> Response<Body>
let theme = get_theme(&parts.headers);
+ let consent = if let Ok(text) = proxmox_backup::config::consent::config() {
+ text
+ } else {
+ "".to_string()
+ };
let data = json!({
"NodeName": nodename,
"UserName": user,
@@ -160,6 +165,7 @@ async fn get_index_future(env: RestEnvironment, parts: Parts) -> Response<Body>
"theme": theme,
"auto": theme == "auto",
"debug": debug,
+ "consentText": consent,
});
let (ct, index) = match api.render_template(template_file, &data) {
diff --git a/src/config/consent.rs b/src/config/consent.rs
new file mode 100644
index 00000000..55ef201c
--- /dev/null
+++ b/src/config/consent.rs
@@ -0,0 +1,11 @@
+use anyhow::Error;
+
+use pbs_buildcfg::configdir;
+
+const CONF_FILE: &str = configdir!("/consent.txt");
+
+/// Read the Consent config.
+pub fn config() -> Result<String, Error> {
+ let content = proxmox_sys::fs::file_read_optional_string(CONF_FILE)?.unwrap_or_default();
+ Ok(content)
+}
diff --git a/src/config/mod.rs b/src/config/mod.rs
index 324fabca..81caf0d5 100644
--- a/src/config/mod.rs
+++ b/src/config/mod.rs
@@ -15,6 +15,7 @@ use proxmox_lang::try_block;
use pbs_buildcfg::{self, configdir};
pub mod acme;
+pub mod consent;
pub mod node;
pub mod tfa;
--
2.43.0
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 2/4] ui: show consent banner before login
2024-05-22 13:19 [pbs-devel] [PATCH backup/proxmox-backup 0/4] fix #5463: add optional consent banner before login Gabriel Goller
2024-05-22 13:19 ` [pbs-devel] [PATCH proxmox-backup 1/4] api: add consent api handler and config Gabriel Goller
@ 2024-05-22 13:19 ` Gabriel Goller
2024-05-22 15:21 ` Thomas Lamprecht
2024-05-22 13:19 ` [pbs-devel] [PATCH proxmox-backup 3/4] docs: add section about consent banner Gabriel Goller
` (3 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Gabriel Goller @ 2024-05-22 13:19 UTC (permalink / raw)
To: pbs-devel
Before showing the LoginView, check if we got a non-empty consent text
from the template. If there is a non-empty text, display it in a modal.
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
www/LoginView.js | 9 +++++++++
www/index.hbs | 1 +
2 files changed, 10 insertions(+)
diff --git a/www/LoginView.js b/www/LoginView.js
index d4d8e73e..96f97da7 100644
--- a/www/LoginView.js
+++ b/www/LoginView.js
@@ -20,6 +20,15 @@ Ext.define('PBS.LoginView', {
controller: {
xclass: 'Ext.app.ViewController',
+ init: async function() {
+ if (Proxmox.consentText !== "") {
+ Ext.create('Proxmox.window.ConsentModal', {
+ autoShow: true,
+ consent: Proxmox.consentText,
+ });
+ }
+ },
+
submitForm: async function() {
var me = this;
var loginForm = me.lookupReference('loginForm');
diff --git a/www/index.hbs b/www/index.hbs
index 824268e3..8a065a94 100644
--- a/www/index.hbs
+++ b/www/index.hbs
@@ -38,6 +38,7 @@
UserName: "{{ UserName }}",
defaultLang: "{{ language }}",
CSRFPreventionToken: "{{ CSRFPreventionToken }}",
+ consentText: `{{ consentText }}`,
};
</script>
<script type="text/javascript" src="/widgettoolkit/proxmoxlib.js"></script>
--
2.43.0
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 3/4] docs: add section about consent banner
2024-05-22 13:19 [pbs-devel] [PATCH backup/proxmox-backup 0/4] fix #5463: add optional consent banner before login Gabriel Goller
2024-05-22 13:19 ` [pbs-devel] [PATCH proxmox-backup 1/4] api: add consent api handler and config Gabriel Goller
2024-05-22 13:19 ` [pbs-devel] [PATCH proxmox-backup 2/4] ui: show consent banner before login Gabriel Goller
@ 2024-05-22 13:19 ` Gabriel Goller
2024-05-22 13:19 ` [pbs-devel] [PATCH backup 4/4] window: add consent modal Gabriel Goller
` (2 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Gabriel Goller @ 2024-05-22 13:19 UTC (permalink / raw)
To: pbs-devel
Add short section on how to activate and customize consent banner.
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
docs/gui.rst | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/docs/gui.rst b/docs/gui.rst
index 9547c73f..15c885ef 100644
--- a/docs/gui.rst
+++ b/docs/gui.rst
@@ -40,6 +40,13 @@ Proxmox Backup Server supports various languages and authentication back ends
.. note:: For convenience, you can save the username on the client side, by
selecting the "Save User name" checkbox at the bottom of the window.
+Consent Banner
+^^^^^^^^^^^^^^
+
+A consent banner that has to be accepted before login can be displayed
+by creating a /etc/proxmox-backup/consent.txt file. If this file exists,
+and is not empty, the content will be displayed in a popup.
+
GUI Overview
------------
--
2.43.0
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* [pbs-devel] [PATCH backup 4/4] window: add consent modal
2024-05-22 13:19 [pbs-devel] [PATCH backup/proxmox-backup 0/4] fix #5463: add optional consent banner before login Gabriel Goller
` (2 preceding siblings ...)
2024-05-22 13:19 ` [pbs-devel] [PATCH proxmox-backup 3/4] docs: add section about consent banner Gabriel Goller
@ 2024-05-22 13:19 ` Gabriel Goller
2024-05-22 15:31 ` [pbs-devel] [PATCH backup/proxmox-backup 0/4] fix #5463: add optional consent banner before login Thomas Lamprecht
2024-06-04 12:50 ` Gabriel Goller
5 siblings, 0 replies; 15+ messages in thread
From: Gabriel Goller @ 2024-05-22 13:19 UTC (permalink / raw)
To: pbs-devel
Add consentModal that gets displayed before the login. Simply shows the
text in a scrollable box and contains a single button "OK".
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
src/Makefile | 1 +
src/window/ConsentModal.js | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 35 insertions(+)
create mode 100644 src/window/ConsentModal.js
diff --git a/src/Makefile b/src/Makefile
index 0478251..3c2fd4b 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -87,6 +87,7 @@ JSSRC= \
window/DiskSmart.js \
window/ZFSDetail.js \
window/Certificates.js \
+ window/ConsentModal.js \
window/ACMEAccount.js \
window/ACMEPluginEdit.js \
window/ACMEDomains.js \
diff --git a/src/window/ConsentModal.js b/src/window/ConsentModal.js
new file mode 100644
index 0000000..16e862c
--- /dev/null
+++ b/src/window/ConsentModal.js
@@ -0,0 +1,34 @@
+Ext.define('Proxmox.window.ConsentModal', {
+ extend: 'Ext.window.Window',
+ alias: ['widget.pmxConsentModal'],
+ mixins: ['Proxmox.Mixin.CBind'],
+
+ width: 600,
+ modal: true,
+ closable: false,
+ resizable: false,
+ alwaysOnTop: true,
+ title: gettext('Consent'),
+
+ items: [
+ {
+ xtype: 'textareafield',
+ cbind: {
+ value: '{consent}',
+ },
+ editable: false,
+ width: 600,
+ height: 400,
+ scrollable: 'y',
+ },
+ ],
+ buttons: [
+ {
+ handler: function() {
+ this.up('window').close();
+ },
+ text: gettext('OK'),
+ },
+ ],
+});
+
--
2.43.0
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 2/4] ui: show consent banner before login
2024-05-22 13:19 ` [pbs-devel] [PATCH proxmox-backup 2/4] ui: show consent banner before login Gabriel Goller
@ 2024-05-22 15:21 ` Thomas Lamprecht
2024-05-23 9:41 ` Gabriel Goller
0 siblings, 1 reply; 15+ messages in thread
From: Thomas Lamprecht @ 2024-05-22 15:21 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Gabriel Goller
Am 22/05/2024 um 15:19 schrieb Gabriel Goller:
> Before showing the LoginView, check if we got a non-empty consent text
> from the template. If there is a non-empty text, display it in a modal.
>
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---
> www/LoginView.js | 9 +++++++++
> www/index.hbs | 1 +
> 2 files changed, 10 insertions(+)
>
> diff --git a/www/LoginView.js b/www/LoginView.js
> index d4d8e73e..96f97da7 100644
> --- a/www/LoginView.js
> +++ b/www/LoginView.js
> @@ -20,6 +20,15 @@ Ext.define('PBS.LoginView', {
> controller: {
> xclass: 'Ext.app.ViewController',
>
> + init: async function() {
> + if (Proxmox.consentText !== "") {
> + Ext.create('Proxmox.window.ConsentModal', {
> + autoShow: true,
> + consent: Proxmox.consentText,
> + });
> + }
> + },
> +
> submitForm: async function() {
> var me = this;
> var loginForm = me.lookupReference('loginForm');
> diff --git a/www/index.hbs b/www/index.hbs
> index 824268e3..8a065a94 100644
> --- a/www/index.hbs
> +++ b/www/index.hbs
> @@ -38,6 +38,7 @@
> UserName: "{{ UserName }}",
> defaultLang: "{{ language }}",
> CSRFPreventionToken: "{{ CSRFPreventionToken }}",
> + consentText: `{{ consentText }}`,
My knowledge about handlebars template and our integration of them is a bit
rusty, but are we sure that above does not allow code injection that can
alter the UI in some odd way, less an issue for users but might allow easily
to replace our product trademarks and other barriers that ensure that our
product stay economically viable without having to modify the code?
> };
> </script>
> <script type="text/javascript" src="/widgettoolkit/proxmoxlib.js"></script>
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [pbs-devel] [PATCH backup/proxmox-backup 0/4] fix #5463: add optional consent banner before login
2024-05-22 13:19 [pbs-devel] [PATCH backup/proxmox-backup 0/4] fix #5463: add optional consent banner before login Gabriel Goller
` (3 preceding siblings ...)
2024-05-22 13:19 ` [pbs-devel] [PATCH backup 4/4] window: add consent modal Gabriel Goller
@ 2024-05-22 15:31 ` Thomas Lamprecht
2024-05-23 7:51 ` Dominik Csapak
2024-06-04 12:50 ` Gabriel Goller
5 siblings, 1 reply; 15+ messages in thread
From: Thomas Lamprecht @ 2024-05-22 15:31 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Gabriel Goller
Am 22/05/2024 um 15:19 schrieb Gabriel Goller:
> This has been requested many times already for all products. To keep
> it simple as it's still quite a niche feature, works with a single file:
> /etc/proxmox-backup/consent.txt. If the file exists and is not empty,
> a consent banner will be shown in front of the login view.
>
> This is just a reference implementation for pbs to get some feedback
> and check if my general approach is alright. The same implementation
> will then be ported to pve and eventually pmg.
>
> Another disclaimer: IANAL (I am not a lawyer :) ), so the wording is
> probably off.
A few general questions for you and/or the original requester of this
feature:
This is currently still missing any actual barrier as it's all frontend,
shouldn't there be a cookie that is checked on the backend side if a
consent.txt exist? If this specific consent type (RMF AC-8 for US gov)
doesn't need that, it might be worth to replace the generic text box
with a type selection for that, we could always add a "custom" type
that takes a generic text and extent that with an option about how
strict it should be checked, if we get this now.
And how should API calls made using API tokens get handled, should they
have a header signalling consent or not? If, should there be a set of
standard consents that one can explicitly consent too? As a blanket
consent to an unknown text would not be of much use.
I'd in any way limit the length of the consent text to a relatively
high boundary, like 10 KiB.
Did you think about interpreting and rendering this as Markdown?
Did you check if there already exist (FLOSS) proxies that implement
this functionality by placing it between the user and any HTTP served
page/tool/ui, as that would not require us to do anything at all
(well, besides documenting it).
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [pbs-devel] [PATCH backup/proxmox-backup 0/4] fix #5463: add optional consent banner before login
2024-05-22 15:31 ` [pbs-devel] [PATCH backup/proxmox-backup 0/4] fix #5463: add optional consent banner before login Thomas Lamprecht
@ 2024-05-23 7:51 ` Dominik Csapak
2024-05-23 9:24 ` Thomas Lamprecht
0 siblings, 1 reply; 15+ messages in thread
From: Dominik Csapak @ 2024-05-23 7:51 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Thomas Lamprecht,
Gabriel Goller
On 5/22/24 17:31, Thomas Lamprecht wrote:
> Am 22/05/2024 um 15:19 schrieb Gabriel Goller:
>> This has been requested many times already for all products. To keep
>> it simple as it's still quite a niche feature, works with a single file:
>> /etc/proxmox-backup/consent.txt. If the file exists and is not empty,
>> a consent banner will be shown in front of the login view.
>>
>> This is just a reference implementation for pbs to get some feedback
>> and check if my general approach is alright. The same implementation
>> will then be ported to pve and eventually pmg.
>>
>> Another disclaimer: IANAL (I am not a lawyer :) ), so the wording is
>> probably off.
>
> A few general questions for you and/or the original requester of this
> feature:
>
> This is currently still missing any actual barrier as it's all frontend,
> shouldn't there be a cookie that is checked on the backend side if a
> consent.txt exist? If this specific consent type (RMF AC-8 for US gov)
> doesn't need that, it might be worth to replace the generic text box
> with a type selection for that, we could always add a "custom" type
> that takes a generic text and extent that with an option about how
> strict it should be checked, if we get this now.
when checking https://csf.tools/reference/nist-sp-800-53/r5/ac/ac-8/
(not the "official" document, but very close , the original can be downloaded
in docx form here:
https://csrc.nist.rip/projects/risk-management/about-rmf/assess-step/assessment-cases-download-page)
it does not seem to be necessary for any cookie handling
since it just wants the disclaimer to be displayed before login
>
> And how should API calls made using API tokens get handled, should they
> have a header signalling consent or not? If, should there be a set of
> standard consents that one can explicitly consent too? As a blanket
> consent to an unknown text would not be of much use.
also it says that this is only for human interaction, so any api
access etc. is exempt IIUC
>
> I'd in any way limit the length of the consent text to a relatively
> high boundary, like 10 KiB.
>
> Did you think about interpreting and rendering this as Markdown?
>
> Did you check if there already exist (FLOSS) proxies that implement
> this functionality by placing it between the user and any HTTP served
> page/tool/ui, as that would not require us to do anything at all
> (well, besides documenting it).
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>
>
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [pbs-devel] [PATCH backup/proxmox-backup 0/4] fix #5463: add optional consent banner before login
2024-05-23 7:51 ` Dominik Csapak
@ 2024-05-23 9:24 ` Thomas Lamprecht
2024-05-23 12:10 ` Gabriel Goller
0 siblings, 1 reply; 15+ messages in thread
From: Thomas Lamprecht @ 2024-05-23 9:24 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Dominik Csapak,
Gabriel Goller
Am 23/05/2024 um 09:51 schrieb Dominik Csapak:
> On 5/22/24 17:31, Thomas Lamprecht wrote:
>> This is currently still missing any actual barrier as it's all frontend,
>> shouldn't there be a cookie that is checked on the backend side if a
>> consent.txt exist? If this specific consent type (RMF AC-8 for US gov)
>> doesn't need that, it might be worth to replace the generic text box
>> with a type selection for that, we could always add a "custom" type
>> that takes a generic text and extent that with an option about how
>> strict it should be checked, if we get this now.
>
> when checking https://csf.tools/reference/nist-sp-800-53/r5/ac/ac-8/
> (not the "official" document, but very close , the original can be downloaded
> in docx form here:
> https://csrc.nist.rip/projects/risk-management/about-rmf/assess-step/assessment-cases-download-page)
> it does not seem to be necessary for any cookie handling
> since it just wants the disclaimer to be displayed before login
ack, thanks for the links.
>
>>
>> And how should API calls made using API tokens get handled, should they
>> have a header signalling consent or not? If, should there be a set of
>> standard consents that one can explicitly consent too? As a blanket
>> consent to an unknown text would not be of much use.
>
>
> also it says that this is only for human interaction, so any api
> access etc. is exempt IIUC
So pretty much a worthless "keep out" sign [0], can one be even
enterprise ready without those? ;-)
[0]: https://i.imgur.com/mSHi8.jpeg
Anyhow, fine by me, but I then still would prefer having this saved
as structured data with an explicit type so that we can easily extend
this with an option for actually enforcing such a consent, if ever
requested.
Maybe we can even add it as encoded text to an existing config, for PVE
the datacenter one would be a good fit, for PMG with also have a cluster
wide one IIRC and for PBS we could just add it to the node.cfg (and cache
inside the http daemon).
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 2/4] ui: show consent banner before login
2024-05-22 15:21 ` Thomas Lamprecht
@ 2024-05-23 9:41 ` Gabriel Goller
0 siblings, 0 replies; 15+ messages in thread
From: Gabriel Goller @ 2024-05-23 9:41 UTC (permalink / raw)
To: Thomas Lamprecht; +Cc: Proxmox Backup Server development discussion
On 22.05.2024 17:21, Thomas Lamprecht wrote:
>Am 22/05/2024 um 15:19 schrieb Gabriel Goller:
>> diff --git a/www/index.hbs b/www/index.hbs
>> index 824268e3..8a065a94 100644
>> --- a/www/index.hbs
>> +++ b/www/index.hbs
>> @@ -38,6 +38,7 @@
>> UserName: "{{ UserName }}",
>> defaultLang: "{{ language }}",
>> CSRFPreventionToken: "{{ CSRFPreventionToken }}",
>> + consentText: `{{ consentText }}`,
>
>My knowledge about handlebars template and our integration of them is a bit
>rusty, but are we sure that above does not allow code injection that can
>alter the UI in some odd way, less an issue for users but might allow easily
>to replace our product trademarks and other barriers that ensure that our
>product stay economically viable without having to modify the code?
Yes it does.
For example if I paste this line into consent.txt:
${alert(1)}
it gets executed and you get the alert window.
BUT: I just discussed this a bit with Max and we found a
solution:
Using double quotes!
Handlebars per default escapes some characters on the server-side [0],
namely '"', '<' and '>'. This makes it impossible to escape this string
(As you can't get "outside" of the string without using '"'):
consentText: "{{ consentText }}",
To allow newlines in the double quotes, we just need to escape them
again, as they won't work out the box as with backticks.
With my limited knowledge of XSS Injections, I think this looks quite
good now :)
[0]: https://github.com/sunng87/handlebars-rust/blob/1c92d492a644a563ec3bd4699b6427c86bb4eae9/src/support.rs#L43
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [pbs-devel] [PATCH backup/proxmox-backup 0/4] fix #5463: add optional consent banner before login
2024-05-23 9:24 ` Thomas Lamprecht
@ 2024-05-23 12:10 ` Gabriel Goller
2024-05-23 12:42 ` Thomas Lamprecht
0 siblings, 1 reply; 15+ messages in thread
From: Gabriel Goller @ 2024-05-23 12:10 UTC (permalink / raw)
To: Thomas Lamprecht; +Cc: Proxmox Backup Server development discussion
On 23.05.2024 11:24, Thomas Lamprecht wrote:
>Am 23/05/2024 um 09:51 schrieb Dominik Csapak:
>> On 5/22/24 17:31, Thomas Lamprecht wrote:
>>> This is currently still missing any actual barrier as it's all frontend,
>>> shouldn't there be a cookie that is checked on the backend side if a
>>> consent.txt exist? If this specific consent type (RMF AC-8 for US gov)
>>> doesn't need that, it might be worth to replace the generic text box
>>> with a type selection for that, we could always add a "custom" type
>>> that takes a generic text and extent that with an option about how
>>> strict it should be checked, if we get this now.
>>
>> when checking https://csf.tools/reference/nist-sp-800-53/r5/ac/ac-8/
>> (not the "official" document, but very close , the original can be downloaded
>> in docx form here:
>> https://csrc.nist.rip/projects/risk-management/about-rmf/assess-step/assessment-cases-download-page)
>> it does not seem to be necessary for any cookie handling
>> since it just wants the disclaimer to be displayed before login
>
>ack, thanks for the links.
>
>>
>>>
>>> And how should API calls made using API tokens get handled, should they
>>> have a header signalling consent or not? If, should there be a set of
>>> standard consents that one can explicitly consent too? As a blanket
>>> consent to an unknown text would not be of much use.
>>
>>
>> also it says that this is only for human interaction, so any api
>> access etc. is exempt IIUC
>
>
>So pretty much a worthless "keep out" sign [0], can one be even
>enterprise ready without those? ;-)
>
>[0]: https://i.imgur.com/mSHi8.jpeg
>
>Anyhow, fine by me, but I then still would prefer having this saved
>as structured data with an explicit type so that we can easily extend
>this with an option for actually enforcing such a consent, if ever
>requested.
>
>Maybe we can even add it as encoded text to an existing config, for PVE
>the datacenter one would be a good fit, for PMG with also have a cluster
>wide one IIRC and for PBS we could just add it to the node.cfg (and cache
>inside the http daemon).
I don't think we will gain much from adding the text in a config file
here. The config files don't support multi-line values and thus we have to
escape all the newlines. If we do this, we would have to introduce a ui
textfield where the user can edit the consent file, otherwise he would
have to escape all the newlines manually in the node.cfg file (which is
a PITA).
I am also kind of opposed to a ui element because this is quite a niche
feature and would only clog the interface.
I don't think an option to strictly enforcing the consent won't come any
time soon, as it's quite complicated to implement and is mostly used as
a legal requirement anyway.
Another plus would be that VMWare does the same, so a user would just
have to come the .txt file /etc/proxmox-backup (+ rename it) and would
be ready to go.
If we want to add other visual configs such as optional buttons (e.g.:
"I Accept", "I Decline") I think we should add the in the txt file like
such (or something similar):
multi-line
text
I Agree|I Disagree|Ignore
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [pbs-devel] [PATCH backup/proxmox-backup 0/4] fix #5463: add optional consent banner before login
2024-05-23 12:10 ` Gabriel Goller
@ 2024-05-23 12:42 ` Thomas Lamprecht
2024-05-28 8:18 ` Gabriel Goller
0 siblings, 1 reply; 15+ messages in thread
From: Thomas Lamprecht @ 2024-05-23 12:42 UTC (permalink / raw)
To: Gabriel Goller; +Cc: Proxmox Backup Server development discussion
Am 23/05/2024 um 14:10 schrieb Gabriel Goller:
> On 23.05.2024 11:24, Thomas Lamprecht wrote:
>> Anyhow, fine by me, but I then still would prefer having this saved
>> as structured data with an explicit type so that we can easily extend
>> this with an option for actually enforcing such a consent, if ever
>> requested.
>>
>> Maybe we can even add it as encoded text to an existing config, for PVE
>> the datacenter one would be a good fit, for PMG with also have a cluster
>> wide one IIRC and for PBS we could just add it to the node.cfg (and cache
>> inside the http daemon).
>
> I don't think we will gain much from adding the text in a config file
> here. The config files don't support multi-line values and thus we have to
> escape all the newlines. If we do this, we would have to introduce a ui
Yes, that's why I wrote "as encoded text" ;-)
> textfield where the user can edit the consent file, otherwise he would
Yes, re-using an existing config would allow more easily to expose it on
the UI as we wouldn't have to add new API endpoints managing it, i.e., a
feature.
> have to escape all the newlines manually in the node.cfg file (which is
> a PITA).
For setting it via CLI it would probably best handled as manager CLI
command.
>
> I am also kind of opposed to a ui element because this is quite a niche
> feature and would only clog the interface.
I do not think the node configuration would get clogged, e.g., on the PBS
UI the Configuration -> Other -> General panel has three rows, and the
rest of that tab panel isn't packed either. Same for PVE's Node -> Options
panel.
Note that in the long run we want to bring every option to the UI sooner
or later. How soon that is depends on mostly from the potential negative
impact, not necessarily how niche that is.
And sure, looking out for the UI not getting to crowded is a good thing,
but the solution there should be to rework the UI to better lay out all
the options and form fields without overwhelming the users, e.g. by using
a different layout, advanced sections, ...
>
> I don't think an option to strictly enforcing the consent won't come any
> time soon, as it's quite complicated to implement and is mostly used as
> a legal requirement anyway.
My thinking is a bit different here, I would not have thought about adding
this before seeing the feature request including its references to
legislature of one of the biggest countries in the world, so as there
are hundreds of countries, lots of them with their own niche case, I'd
rather make this as extensible as possible from the beginning to avoid
having re-doing it later on for every other countries/agency/... niche
use case.
>
> Another plus would be that VMWare does the same, so a user would just
> have to come the .txt file /etc/proxmox-backup (+ rename it) and would
> be ready to go.
A reference to how VMWare does would be nice, besides that:
1. copying the message to a text area in the UI or using a CLI tool
to set that is hardly more work.
2. More importantly, I'd be surprised if these messages are written
on the spot by an admin on the VMWare host directly, i.e., having
no other copy. Normally, those things get drafted by a legal
department, or external counsel, and thus are available from a
better source of truth than some hypervisor hosts config.
>
> If we want to add other visual configs such as optional buttons (e.g.:
> "I Accept", "I Decline") I think we should add the in the txt file like
> such (or something similar):
>
> multi-line
> text
>
> I Agree|I Disagree|Ignore
>
So you'd prefer inventing some new brittle domain specific format/parsing
inside a generic text file that users needs to learn and might be turning
complete by accident over using a structured format from the beginning?
Note that with a UI the latter has no real downside (that cannot be
easily disarmed through said UI) but only upsides.
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [pbs-devel] [PATCH backup/proxmox-backup 0/4] fix #5463: add optional consent banner before login
2024-05-23 12:42 ` Thomas Lamprecht
@ 2024-05-28 8:18 ` Gabriel Goller
2024-05-28 8:33 ` Gabriel Goller
0 siblings, 1 reply; 15+ messages in thread
From: Gabriel Goller @ 2024-05-28 8:18 UTC (permalink / raw)
To: Thomas Lamprecht; +Cc: Proxmox Backup Server development discussion
Ok, you convinced me :)
On 23.05.2024 14:42, Thomas Lamprecht wrote:
>Am 23/05/2024 um 14:10 schrieb Gabriel Goller:
>> On 23.05.2024 11:24, Thomas Lamprecht wrote:
>>> Anyhow, fine by me, but I then still would prefer having this saved
>>> as structured data with an explicit type so that we can easily extend
>>> this with an option for actually enforcing such a consent, if ever
>>> requested.
>>>
>>> Maybe we can even add it as encoded text to an existing config, for PVE
>>> the datacenter one would be a good fit, for PMG with also have a cluster
>>> wide one IIRC and for PBS we could just add it to the node.cfg (and cache
>>> inside the http daemon).
>>
>> I don't think we will gain much from adding the text in a config file
>> here. The config files don't support multi-line values and thus we have to
>> escape all the newlines. If we do this, we would have to introduce a ui
>
>Yes, that's why I wrote "as encoded text" ;-)
>
>> textfield where the user can edit the consent file, otherwise he would
>
>Yes, re-using an existing config would allow more easily to expose it on
>the UI as we wouldn't have to add new API endpoints managing it, i.e., a
>feature.
I Agree.
>> have to escape all the newlines manually in the node.cfg file (which is
>> a PITA).
>
>For setting it via CLI it would probably best handled as manager CLI
>command.
We can do that.
>> I am also kind of opposed to a ui element because this is quite a niche
>> feature and would only clog the interface.
>
>I do not think the node configuration would get clogged, e.g., on the PBS
>UI the Configuration -> Other -> General panel has three rows, and the
>rest of that tab panel isn't packed either. Same for PVE's Node -> Options
>panel.
Either that, or somewhere in Access Control?
>Note that in the long run we want to bring every option to the UI sooner
>or later. How soon that is depends on mostly from the potential negative
>impact, not necessarily how niche that is.
>
>And sure, looking out for the UI not getting to crowded is a good thing,
>but the solution there should be to rework the UI to better lay out all
>the options and form fields without overwhelming the users, e.g. by using
>a different layout, advanced sections, ...
>
>>
>> I don't think an option to strictly enforcing the consent won't come any
>> time soon, as it's quite complicated to implement and is mostly used as
>> a legal requirement anyway.
>
>My thinking is a bit different here, I would not have thought about adding
>this before seeing the feature request including its references to
>legislature of one of the biggest countries in the world, so as there
>are hundreds of countries, lots of them with their own niche case, I'd
>rather make this as extensible as possible from the beginning to avoid
>having re-doing it later on for every other countries/agency/... niche
>use case.
>
>>
>> Another plus would be that VMWare does the same, so a user would just
>> have to come the .txt file /etc/proxmox-backup (+ rename it) and would
>> be ready to go.
>
>A reference to how VMWare does would be nice, besides that:
>1. copying the message to a text area in the UI or using a CLI tool
> to set that is hardly more work.
Agree.
>2. More importantly, I'd be surprised if these messages are written
> on the spot by an admin on the VMWare host directly, i.e., having
> no other copy. Normally, those things get drafted by a legal
> department, or external counsel, and thus are available from a
> better source of truth than some hypervisor hosts config.
Yes, you are probably right, this wouldn't mindlessly get copied from
a vmware machine to a proxmox one.
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [pbs-devel] [PATCH backup/proxmox-backup 0/4] fix #5463: add optional consent banner before login
2024-05-28 8:18 ` Gabriel Goller
@ 2024-05-28 8:33 ` Gabriel Goller
0 siblings, 0 replies; 15+ messages in thread
From: Gabriel Goller @ 2024-05-28 8:33 UTC (permalink / raw)
To: Proxmox Backup Server development discussion; +Cc: Thomas Lamprecht
Oops, forgot to add the reference, here you go!
>>>Another plus would be that VMWare does the same, so a user would just
>>>have to come the .txt file /etc/proxmox-backup (+ rename it) and would
>>>be ready to go.
>>
>>A reference to how VMWare does would be nice, besides that:
https://docs.vmware.com/en/VMware-vSphere/8.0/vsphere-esxi-host-client/GUID-E9311D4F-F8CA-4422-87A1-862D46C2E4AD.html
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [pbs-devel] [PATCH backup/proxmox-backup 0/4] fix #5463: add optional consent banner before login
2024-05-22 13:19 [pbs-devel] [PATCH backup/proxmox-backup 0/4] fix #5463: add optional consent banner before login Gabriel Goller
` (4 preceding siblings ...)
2024-05-22 15:31 ` [pbs-devel] [PATCH backup/proxmox-backup 0/4] fix #5463: add optional consent banner before login Thomas Lamprecht
@ 2024-06-04 12:50 ` Gabriel Goller
5 siblings, 0 replies; 15+ messages in thread
From: Gabriel Goller @ 2024-06-04 12:50 UTC (permalink / raw)
To: Proxmox Backup Server development discussion
Sent a v2!
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2024-06-04 12:50 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-22 13:19 [pbs-devel] [PATCH backup/proxmox-backup 0/4] fix #5463: add optional consent banner before login Gabriel Goller
2024-05-22 13:19 ` [pbs-devel] [PATCH proxmox-backup 1/4] api: add consent api handler and config Gabriel Goller
2024-05-22 13:19 ` [pbs-devel] [PATCH proxmox-backup 2/4] ui: show consent banner before login Gabriel Goller
2024-05-22 15:21 ` Thomas Lamprecht
2024-05-23 9:41 ` Gabriel Goller
2024-05-22 13:19 ` [pbs-devel] [PATCH proxmox-backup 3/4] docs: add section about consent banner Gabriel Goller
2024-05-22 13:19 ` [pbs-devel] [PATCH backup 4/4] window: add consent modal Gabriel Goller
2024-05-22 15:31 ` [pbs-devel] [PATCH backup/proxmox-backup 0/4] fix #5463: add optional consent banner before login Thomas Lamprecht
2024-05-23 7:51 ` Dominik Csapak
2024-05-23 9:24 ` Thomas Lamprecht
2024-05-23 12:10 ` Gabriel Goller
2024-05-23 12:42 ` Thomas Lamprecht
2024-05-28 8:18 ` Gabriel Goller
2024-05-28 8:33 ` Gabriel Goller
2024-06-04 12:50 ` Gabriel Goller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox