Hi,

a few things to this patch

this is not a proper git patch, so please see https://pve.proxmox.com/wiki/Developer_Documentation for how to send proper patches
(also see the notes about indentation etc.)

the commit message/subject is also not very meaningful, better would be something like:
'do not require ticket on vncwebsocket call'

with a message why this is good and necessary or which bug this fixes

--- (end of formal review, start of review of the content)

i get what you want to do, but would it not be possible to create
a vnc user and use that cookie on your client?

the way you did it can never work if the vncproxy/termproxy/etc. call is not made with the vnc@pve user anyway (and even then it will not work with the host console)

also, hardcoding a user which might already exist in an existing installation is not good

further comment inline

On 02/02/2018 09:45 AM, Entwickler (centron GmbH) wrote:
This allows Access to novnc from an external Webinterface to user vnc.


--- /usr/share/perl5/PVE/HTTPServer.pm          2018-01-17 09:18:26.000000000 
+0100
@@ -76,11 +76,16 @@
      if ($require_auth) {
-              die "No ticket\n" if !$ticket;
-
-              ($username, $age) = PVE::AccessControl::verify_ticket($ticket);
-
-              $rpcenv->set_user($username);
+             if ($rel_uri =~ /vncwebsocket/ && $method eq 'GET' && !$ticket)

the regex is wrong, e.g. if a node is named 'vncwebsocket' (i know it is unlikely, but still) all GET requests there will work unauthenticated

+             {
+                             $rpcenv->set_user("vnc\@pve");
+                             $username = "vnc\@pve";

a perl tip: if you use single quotes, you do not need to escape @/$/etc.

var $name = 'name@realm';

works

+                             $age = 60;
+             } else {
+                             die "No ticket\n" if !$ticket;
+                             ($username, $age) = 
PVE::AccessControl::verify_ticket($ticket);
+                             $rpcenv->set_user($username);
+             }
                 if ($method eq 'POST' && $rel_uri =~ 
m|^/nodes/([^/]+)/storage/([^/]+)/upload$|) {
                    my ($node, $storeid) = ($1, $2);
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to