From ffda9f174349674926d87559c3665463675605c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20B=C3=BC=C3=9Femeyer?= Date: Mon, 21 Oct 2024 19:10:54 +0200 Subject: [PATCH 01/13] WIP auto use user token when sharing token is not sufficient for a request --- frontend/javascripts/admin/api/token.ts | 35 ++++++++++++++++++++----- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/frontend/javascripts/admin/api/token.ts b/frontend/javascripts/admin/api/token.ts index 54f730d40e7..73c8c4c8971 100644 --- a/frontend/javascripts/admin/api/token.ts +++ b/frontend/javascripts/admin/api/token.ts @@ -1,6 +1,7 @@ import { location } from "libs/window"; import Request from "libs/request"; import * as Utils from "libs/utils"; +import Toast from "libs/toast"; let tokenPromise: Promise; @@ -33,22 +34,42 @@ export function getSharingTokenFromUrlParameters(): string | null | undefined { return null; } -export function doWithToken(fn: (token: string) => Promise, tries: number = 1): Promise { - const sharingToken = getSharingTokenFromUrlParameters(); +function removeSharingTokenFromURLParameters() { + const urlObj = new URL(window.location.href); + if (urlObj.searchParams.has("token")) { + urlObj.searchParams.delete("token"); + window.history.replaceState({}, document.title, urlObj.toString()); // somehow it is replaced back to the state where the token exists :/// + Toast.info("Token URL token was outdated and therefore removed."); + } +} + +export async function doWithToken( + fn: (token: string) => Promise, + tries: number = 1, + useURLTokenIfAvailable: boolean = true, +): Promise { + let token = useURLTokenIfAvailable ? getSharingTokenFromUrlParameters() : null; - if (sharingToken != null) { - return fn(sharingToken); + if (token == null) { + tokenPromise = tokenPromise == null ? requestUserToken() : tokenPromise; + } else { + tokenPromise = Promise.resolve(token); } - if (!tokenPromise) tokenPromise = requestUserToken(); - return tokenPromise.then(fn).catch((error) => { + return tokenPromise.then(fn).catch(async (error) => { if (error.status === 403) { console.warn("Token expired. Requesting new token..."); tokenPromise = requestUserToken(); // If three new tokens did not fix the 403, abort, otherwise we'll get into an endless loop here if (tries < 3) { - return doWithToken(fn, tries + 1); + // If using the url sharing token failed, we try the user specific token instead. + const result = await doWithToken(fn, tries + 1, false); + // Upon successful retry with own token, discard the url token. + if (useURLTokenIfAvailable) { + removeSharingTokenFromURLParameters(); + } + return result; } } From 5e322f49fa1e7b51764c31c499abfb06aea2e8a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20B=C3=BC=C3=9Femeyer?= Date: Tue, 22 Oct 2024 08:30:51 +0200 Subject: [PATCH 02/13] remove token via urlmanager to keep removal (otherwise the urlmanager would have always restored the token in the url) --- frontend/javascripts/admin/api/token.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/frontend/javascripts/admin/api/token.ts b/frontend/javascripts/admin/api/token.ts index 73c8c4c8971..222d51a84bb 100644 --- a/frontend/javascripts/admin/api/token.ts +++ b/frontend/javascripts/admin/api/token.ts @@ -1,7 +1,8 @@ -import { location } from "libs/window"; +import window, { location } from "libs/window"; import Request from "libs/request"; import * as Utils from "libs/utils"; import Toast from "libs/toast"; +import UrlManager from "oxalis/controller/url_manager"; let tokenPromise: Promise; @@ -38,8 +39,8 @@ function removeSharingTokenFromURLParameters() { const urlObj = new URL(window.location.href); if (urlObj.searchParams.has("token")) { urlObj.searchParams.delete("token"); - window.history.replaceState({}, document.title, urlObj.toString()); // somehow it is replaced back to the state where the token exists :/// - Toast.info("Token URL token was outdated and therefore removed."); + UrlManager.changeBaseUrl(urlObj.pathname + urlObj.search); + Toast.info("Removed token from URL and trying using your user token instead..."); } } From ac1a8986862b8993d0f5f1f2a4c6dc39b4eded7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20B=C3=BC=C3=9Femeyer?= Date: Tue, 22 Oct 2024 08:32:20 +0200 Subject: [PATCH 03/13] remove unused window import --- frontend/javascripts/admin/api/token.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/javascripts/admin/api/token.ts b/frontend/javascripts/admin/api/token.ts index 222d51a84bb..f736cbe3571 100644 --- a/frontend/javascripts/admin/api/token.ts +++ b/frontend/javascripts/admin/api/token.ts @@ -1,4 +1,4 @@ -import window, { location } from "libs/window"; +import { location } from "libs/window"; import Request from "libs/request"; import * as Utils from "libs/utils"; import Toast from "libs/toast"; @@ -36,7 +36,7 @@ export function getSharingTokenFromUrlParameters(): string | null | undefined { } function removeSharingTokenFromURLParameters() { - const urlObj = new URL(window.location.href); + const urlObj = new URL(location.href); if (urlObj.searchParams.has("token")) { urlObj.searchParams.delete("token"); UrlManager.changeBaseUrl(urlObj.pathname + urlObj.search); From 51667e87f5a8da62fff44bbadbf9d2ae1369233b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20B=C3=BC=C3=9Femeyer?= Date: Tue, 22 Oct 2024 08:33:47 +0200 Subject: [PATCH 04/13] local var renaming --- frontend/javascripts/admin/api/token.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/frontend/javascripts/admin/api/token.ts b/frontend/javascripts/admin/api/token.ts index f736cbe3571..2a23954bb2e 100644 --- a/frontend/javascripts/admin/api/token.ts +++ b/frontend/javascripts/admin/api/token.ts @@ -36,10 +36,10 @@ export function getSharingTokenFromUrlParameters(): string | null | undefined { } function removeSharingTokenFromURLParameters() { - const urlObj = new URL(location.href); - if (urlObj.searchParams.has("token")) { - urlObj.searchParams.delete("token"); - UrlManager.changeBaseUrl(urlObj.pathname + urlObj.search); + const url = new URL(location.href); + if (url.searchParams.has("token")) { + url.searchParams.delete("token"); + UrlManager.changeBaseUrl(url.pathname + url.search); Toast.info("Removed token from URL and trying using your user token instead..."); } } From 6ec4428ba21a434e150dfbeb723abf60625de65f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20B=C3=BC=C3=9Femeyer?= Date: Tue, 22 Oct 2024 09:09:05 +0200 Subject: [PATCH 05/13] remove token expiration toast --- frontend/javascripts/admin/api/token.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/frontend/javascripts/admin/api/token.ts b/frontend/javascripts/admin/api/token.ts index 2a23954bb2e..08a9d1114d1 100644 --- a/frontend/javascripts/admin/api/token.ts +++ b/frontend/javascripts/admin/api/token.ts @@ -44,6 +44,16 @@ function removeSharingTokenFromURLParameters() { } } +function removeErrorToast(error: any) { + if ("errors" in error && Array.isArray(error.errors)) { + error.errors.forEach((errorText: string) => { + if (errorText.includes("Token may be expired")) { + Toast.close(errorText); + } + }); + } +} + export async function doWithToken( fn: (token: string) => Promise, tries: number = 1, @@ -68,6 +78,7 @@ export async function doWithToken( const result = await doWithToken(fn, tries + 1, false); // Upon successful retry with own token, discard the url token. if (useURLTokenIfAvailable) { + removeErrorToast(error); removeSharingTokenFromURLParameters(); } return result; From 0cecca235cf990c976b67009a74b04f31f5a77e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20B=C3=BC=C3=9Femeyer?= Date: Tue, 22 Oct 2024 09:18:07 +0200 Subject: [PATCH 06/13] add changelog entry --- CHANGELOG.unreleased.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md index da92fa46249..6937a55144c 100644 --- a/CHANGELOG.unreleased.md +++ b/CHANGELOG.unreleased.md @@ -25,6 +25,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released ### Fixed - Fixed a bug during dataset upload in case the configured `datastore.baseFolder` is an absolute path. [#8098](https://github.com/scalableminds/webknossos/pull/8098) [#8103](https://github.com/scalableminds/webknossos/pull/8103) +- When trying to save an annotation opened via a link including a sharing token, the token is automatically discarded in case it is insufficient for update actions but the users token is. [#8139](https://github.com/scalableminds/webknossos/pull/8139) - Fixed that the skeleton search did not automatically expand groups that contained the selected tree [#8129](https://github.com/scalableminds/webknossos/pull/8129) - Fixed a bug that zarr streaming version 3 returned the shape of mag (1, 1, 1) / the finest mag for all mags. [#8116](https://github.com/scalableminds/webknossos/pull/8116) - Fixed sorting of mags in outbound zarr streaming. [#8125](https://github.com/scalableminds/webknossos/pull/8125) From e574d8a6d5312d5e3391019b9bdcdb1698e8df41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20B=C3=BC=C3=9Femeyer?= Date: Tue, 22 Oct 2024 14:28:17 +0200 Subject: [PATCH 07/13] undo removing token from url upon successful request with user token --- frontend/javascripts/admin/api/token.ts | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/frontend/javascripts/admin/api/token.ts b/frontend/javascripts/admin/api/token.ts index 08a9d1114d1..cb36bfa5dab 100644 --- a/frontend/javascripts/admin/api/token.ts +++ b/frontend/javascripts/admin/api/token.ts @@ -2,11 +2,11 @@ import { location } from "libs/window"; import Request from "libs/request"; import * as Utils from "libs/utils"; import Toast from "libs/toast"; -import UrlManager from "oxalis/controller/url_manager"; let tokenPromise: Promise; let tokenRequestPromise: Promise | null; +let shouldUseURLToken: boolean = true; function requestUserToken(): Promise { if (tokenRequestPromise) { @@ -35,15 +35,6 @@ export function getSharingTokenFromUrlParameters(): string | null | undefined { return null; } -function removeSharingTokenFromURLParameters() { - const url = new URL(location.href); - if (url.searchParams.has("token")) { - url.searchParams.delete("token"); - UrlManager.changeBaseUrl(url.pathname + url.search); - Toast.info("Removed token from URL and trying using your user token instead..."); - } -} - function removeErrorToast(error: any) { if ("errors" in error && Array.isArray(error.errors)) { error.errors.forEach((errorText: string) => { @@ -59,7 +50,8 @@ export async function doWithToken( tries: number = 1, useURLTokenIfAvailable: boolean = true, ): Promise { - let token = useURLTokenIfAvailable ? getSharingTokenFromUrlParameters() : null; + let token = + useURLTokenIfAvailable && shouldUseURLToken ? getSharingTokenFromUrlParameters() : null; if (token == null) { tokenPromise = tokenPromise == null ? requestUserToken() : tokenPromise; @@ -78,8 +70,11 @@ export async function doWithToken( const result = await doWithToken(fn, tries + 1, false); // Upon successful retry with own token, discard the url token. if (useURLTokenIfAvailable) { + shouldUseURLToken = false; + Toast.info( + "Initial request using the URL token failed. Your personal token will be used from now on.", + ); removeErrorToast(error); - removeSharingTokenFromURLParameters(); } return result; } From 273c2332696766674d26c376549fc54d3f85cd36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20B=C3=BC=C3=9Femeyer?= Date: Wed, 23 Oct 2024 13:09:14 +0200 Subject: [PATCH 08/13] dont show auto error toast when failing saved and only show manual error toast created by save_saga --- frontend/javascripts/admin/api/token.ts | 14 -------------- frontend/javascripts/libs/request.ts | 12 ++++++++++-- .../javascripts/oxalis/model/sagas/save_saga.ts | 3 +++ 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/frontend/javascripts/admin/api/token.ts b/frontend/javascripts/admin/api/token.ts index cb36bfa5dab..8a6c0ba3f18 100644 --- a/frontend/javascripts/admin/api/token.ts +++ b/frontend/javascripts/admin/api/token.ts @@ -35,16 +35,6 @@ export function getSharingTokenFromUrlParameters(): string | null | undefined { return null; } -function removeErrorToast(error: any) { - if ("errors" in error && Array.isArray(error.errors)) { - error.errors.forEach((errorText: string) => { - if (errorText.includes("Token may be expired")) { - Toast.close(errorText); - } - }); - } -} - export async function doWithToken( fn: (token: string) => Promise, tries: number = 1, @@ -71,10 +61,6 @@ export async function doWithToken( // Upon successful retry with own token, discard the url token. if (useURLTokenIfAvailable) { shouldUseURLToken = false; - Toast.info( - "Initial request using the URL token failed. Your personal token will be used from now on.", - ); - removeErrorToast(error); } return result; } diff --git a/frontend/javascripts/libs/request.ts b/frontend/javascripts/libs/request.ts index 1b1271e4846..aff50101f95 100644 --- a/frontend/javascripts/libs/request.ts +++ b/frontend/javascripts/libs/request.ts @@ -311,7 +311,11 @@ class Request { ...message, key: json.status.toString(), })); - if (showErrorToast) Toast.messages(messages); + if (showErrorToast) { + Toast.messages(messages); // Toast.error already logs the error + } else { + console.error(messages); + } // Check whether the error chain mentions an url which belongs // to a datastore. Then, ping the datastore pingMentionedDataStores(text); @@ -319,7 +323,11 @@ class Request { /* eslint-disable-next-line prefer-promise-reject-errors */ return Promise.reject({ ...json, url: requestedUrl }); } catch (_jsonError) { - if (showErrorToast) Toast.error(text); + if (showErrorToast) { + Toast.error(text); // Toast.error already logs the error + } else { + console.error(text); + } /* eslint-disable-next-line prefer-promise-reject-errors */ return Promise.reject({ diff --git a/frontend/javascripts/oxalis/model/sagas/save_saga.ts b/frontend/javascripts/oxalis/model/sagas/save_saga.ts index e9e09a12a32..d2acc8ca949 100644 --- a/frontend/javascripts/oxalis/model/sagas/save_saga.ts +++ b/frontend/javascripts/oxalis/model/sagas/save_saga.ts @@ -196,6 +196,9 @@ export function* sendRequestToServer( method: "POST", data: compactedSaveQueue, compress: process.env.NODE_ENV === "production", + // Suppressing error toast, as the doWithToken retry with personal token functionality should not show an error. + // Instead the error is logged and toggleErrorHighlighting should take care of showing an error to the user. + showErrorToast: false, }, ); const endTime = Date.now(); From 3410ad70a7725866dc997120f622c2c408d9b8b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20B=C3=BC=C3=9Femeyer?= Date: Thu, 24 Oct 2024 08:57:28 +0200 Subject: [PATCH 09/13] remove unused import --- frontend/javascripts/admin/api/token.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/frontend/javascripts/admin/api/token.ts b/frontend/javascripts/admin/api/token.ts index 8a6c0ba3f18..3dfdd5e4509 100644 --- a/frontend/javascripts/admin/api/token.ts +++ b/frontend/javascripts/admin/api/token.ts @@ -1,7 +1,6 @@ import { location } from "libs/window"; import Request from "libs/request"; import * as Utils from "libs/utils"; -import Toast from "libs/toast"; let tokenPromise: Promise; From 034fe9e04bec8dd714eca0b24e21e25c71c8ffa5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20B=C3=BC=C3=9Femeyer?= Date: Thu, 24 Oct 2024 09:15:33 +0200 Subject: [PATCH 10/13] fix save saga tests --- frontend/javascripts/test/sagas/save_saga.spec.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/frontend/javascripts/test/sagas/save_saga.spec.ts b/frontend/javascripts/test/sagas/save_saga.spec.ts index 677568ef279..4707cf28bee 100644 --- a/frontend/javascripts/test/sagas/save_saga.spec.ts +++ b/frontend/javascripts/test/sagas/save_saga.spec.ts @@ -130,6 +130,7 @@ test("SaveSaga should send request to server", (t) => { method: "POST", data: saveQueueWithVersions, compress: false, + showErrorToast: false, }), ); }); @@ -147,6 +148,7 @@ test("SaveSaga should retry update actions", (t) => { method: "POST", data: saveQueueWithVersions, compress: false, + showErrorToast: false, }, ); const saga = sendRequestToServer(TRACING_TYPE, tracingId); @@ -187,6 +189,7 @@ test("SaveSaga should escalate on permanent client error update actions", (t) => method: "POST", data: saveQueueWithVersions, compress: false, + showErrorToast: false, }), ); saga.throw({ From 1a3675f9abbc65ff7b2b5e6ead2b18498538ff4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20B=C3=BC=C3=9Femeyer?= Date: Thu, 24 Oct 2024 16:49:19 +0200 Subject: [PATCH 11/13] apply some coderabbit suggestions --- frontend/javascripts/admin/api/token.ts | 2 +- frontend/javascripts/libs/request.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/frontend/javascripts/admin/api/token.ts b/frontend/javascripts/admin/api/token.ts index 3dfdd5e4509..9adae198807 100644 --- a/frontend/javascripts/admin/api/token.ts +++ b/frontend/javascripts/admin/api/token.ts @@ -38,7 +38,7 @@ export async function doWithToken( fn: (token: string) => Promise, tries: number = 1, useURLTokenIfAvailable: boolean = true, -): Promise { +): Promise { let token = useURLTokenIfAvailable && shouldUseURLToken ? getSharingTokenFromUrlParameters() : null; diff --git a/frontend/javascripts/libs/request.ts b/frontend/javascripts/libs/request.ts index aff50101f95..25bf31657e5 100644 --- a/frontend/javascripts/libs/request.ts +++ b/frontend/javascripts/libs/request.ts @@ -312,7 +312,7 @@ class Request { key: json.status.toString(), })); if (showErrorToast) { - Toast.messages(messages); // Toast.error already logs the error + Toast.messages(messages); // Note: Toast.error internally logs to console } else { console.error(messages); } @@ -324,9 +324,9 @@ class Request { return Promise.reject({ ...json, url: requestedUrl }); } catch (_jsonError) { if (showErrorToast) { - Toast.error(text); // Toast.error already logs the error + Toast.error(text); // Note: Toast.error internally logs to console } else { - console.error(text); + console.error(`Request failed for ${requestedUrl}:`, text); } /* eslint-disable-next-line prefer-promise-reject-errors */ From 69ec689e65ef25d05d14c2f7096337b800784f13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20B=C3=BC=C3=9Femeyer?= Date: Thu, 24 Oct 2024 17:01:14 +0200 Subject: [PATCH 12/13] fix typechecking --- frontend/javascripts/admin/admin_rest_api.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/javascripts/admin/admin_rest_api.ts b/frontend/javascripts/admin/admin_rest_api.ts index 5af922c7a3a..6c3b4ef8e78 100644 --- a/frontend/javascripts/admin/admin_rest_api.ts +++ b/frontend/javascripts/admin/admin_rest_api.ts @@ -2035,7 +2035,7 @@ export function computeAdHocMesh( }, }, ); - const neighbors = Utils.parseMaybe(headers.neighbors) || []; + const neighbors = (Utils.parseMaybe(headers.neighbors) as number[] | null) || []; return { buffer, neighbors, From 1660fa7f4bae48e72973cb76381b5fa84dd48b27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20B=C3=BC=C3=9Femeyer?= Date: Thu, 24 Oct 2024 17:22:14 +0200 Subject: [PATCH 13/13] apply code rabbit suggestion --- frontend/javascripts/admin/api/token.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/frontend/javascripts/admin/api/token.ts b/frontend/javascripts/admin/api/token.ts index 9adae198807..3a430d55756 100644 --- a/frontend/javascripts/admin/api/token.ts +++ b/frontend/javascripts/admin/api/token.ts @@ -2,6 +2,8 @@ import { location } from "libs/window"; import Request from "libs/request"; import * as Utils from "libs/utils"; +const MAX_TOKEN_RETRY_ATTEMPTS = 3; + let tokenPromise: Promise; let tokenRequestPromise: Promise | null; @@ -50,11 +52,13 @@ export async function doWithToken( return tokenPromise.then(fn).catch(async (error) => { if (error.status === 403) { - console.warn("Token expired. Requesting new token..."); + console.warn( + `Token expired (attempt ${tries}/${MAX_TOKEN_RETRY_ATTEMPTS}). Requesting new token...`, + ); tokenPromise = requestUserToken(); // If three new tokens did not fix the 403, abort, otherwise we'll get into an endless loop here - if (tries < 3) { + if (tries < MAX_TOKEN_RETRY_ATTEMPTS) { // If using the url sharing token failed, we try the user specific token instead. const result = await doWithToken(fn, tries + 1, false); // Upon successful retry with own token, discard the url token.