From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 462781FF168 for ; Mon, 14 Oct 2024 13:57:44 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id A1DF3337CC; Mon, 14 Oct 2024 13:58:12 +0200 (CEST) Message-ID: Date: Mon, 14 Oct 2024 13:57:39 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta To: Proxmox VE development discussion , Max Carrara References: <20241007150251.3295598-1-d.csapak@proxmox.com> Content-Language: en-US From: Dominik Csapak In-Reply-To: X-SPAM-LEVEL: Spam detection results: 0 AWL 0.015 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH widget-toolkit] fix external linking to products by setting cookie SameSite attribute to lax X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" On 10/8/24 14:10, Max Carrara wrote: > On Mon Oct 7, 2024 at 5:02 PM CEST, Dominik Csapak wrote: >> We introduced the 'strict' setting when browsers warned about our >> cookies not having any SameSite setting [0] >> >> While this works in general, it had an unforeseen side effect: >> >> When linking to a URL of our products, the cookie does not get sent on >> the initial page load, leading to the username/CSRFPreventionToken not >> being set in the index response. >> >> Our UI code interprets this as being logged out (e.g. because the ticket >> is not valid) and clears the cookie, displaying the login window. >> >> The MDN reference[1] says that setting it to 'lax' is similar to >> 'strict', but sends the cookie when navigating *to* our origin even from >> other sites, which is what we want when linking from elsewhere. (This >> would have also been the default if we wouldn't have set any attribute). >> >> 0: https://lore.proxmox.com/pve-devel/20230315162630.289768-1-m.carrara@proxmox.com/ >> 1: https://developer.mozilla.org/en-US/docs/Web/HTTP/Cookies#SameSite_attribute >> >> Signed-off-by: Dominik Csapak > > As mentioned off list, this seems fine to me, as our API doesn't have > any side effects during GET requests (otherwise, we'd have different > problems anyway ...) and we only modify, delete etc. via other methods. > > Though, as you probably saw in the series I had sent in back then, I > also set SameSite=Strict in a couple other places: > > - pve-apiclient/trees/master/src/PVE/APIClient/LWP.pm is not really relevant, as it's not used for any browser > - pve-http-server/trees/master/src/PVE/APIServer/Formatter.pm > - pve-http-server/trees/master/src/PVE/APIServer/Formatter/Bootstrap.pm yes those two we want to change too, i can send a followup for pve-http-server > > I guess it would make sense to set them to `lax` there too? > >> --- >> Alternatively, we could try to modify the client code to retry with the >> content of the cookie (which the client still has), to generate a >> ticket. Though I'm not sure that will work correctly, since I observed >> some even stricter behaviour in Firefox ESR, where not only the >> navigation part did not send the cookie, but also subsequent requests >> did not do that (I did not test for long, so this may have been an >> artifact of a different issue). >> >> I'd generally prefer this solution, since it's less intrusive and >> restores the behaviour we had before the change, altough I'm not against >> implementing a retry in the client code if that's preferred. >> >> src/Utils.js | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/src/Utils.js b/src/Utils.js >> index 7dd034a..b68c0f4 100644 >> --- a/src/Utils.js >> +++ b/src/Utils.js >> @@ -317,7 +317,7 @@ utilities: { >> // that way the cookie gets deleted after the browser window is closed >> if (data.ticket) { >> Proxmox.CSRFPreventionToken = data.CSRFPreventionToken; >> - Ext.util.Cookies.set(Proxmox.Setup.auth_cookie_name, data.ticket, null, '/', null, true, "strict"); >> + Ext.util.Cookies.set(Proxmox.Setup.auth_cookie_name, data.ticket, null, '/', null, true, "lax"); >> } >> >> if (data.token) { >> @@ -343,7 +343,7 @@ utilities: { >> return; >> } >> // ExtJS clear is basically the same, but browser may complain if any cookie isn't "secure" >> - Ext.util.Cookies.set(Proxmox.Setup.auth_cookie_name, "", new Date(0), null, null, true, "strict"); >> + Ext.util.Cookies.set(Proxmox.Setup.auth_cookie_name, "", new Date(0), null, null, true, "lax"); >> window.localStorage.removeItem("ProxmoxUser"); >> }, >> > > > > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel