From d6a965b8a693bd74c23f25150f9d7f908b12e712 Mon Sep 17 00:00:00 2001 From: Ashod Nakashian Date: Sun, 19 Apr 2026 10:41:00 -0400 Subject: [PATCH 01/15] wsd: convert Authorization::Type to STATE_ENUM Change-Id: I63a7cbca24222c3dcf4d034d387777a70bd826d3 Signed-off-by: Ashod Nakashian --- common/Authorization.hpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/common/Authorization.hpp b/common/Authorization.hpp index d7aab3b8f190b..5697c90d33f13 100644 --- a/common/Authorization.hpp +++ b/common/Authorization.hpp @@ -13,6 +13,8 @@ #pragma once +#include + #include namespace Poco @@ -30,13 +32,12 @@ class URI; class Authorization { public: - enum class Type : char - { - None, ///< Unlike Expired, this implies no Authorization needed. - Token, - Header, - Expired ///< The server is rejecting the current authorization key. - }; + STATE_ENUM(Type, + None, ///< Unlike Expired, this implies no Authorization needed. + Token, ///< Valid access_token -> "Authorization: Bearer ..." header. + Header, ///< Valid access_header -> Custom header(s). + Expired ///< The server is rejecting the current authorization key. + ); private: std::string _data; From 42091805bca750b83f4f2fbcf4a1f023ba9a8763 Mon Sep 17 00:00:00 2001 From: Ashod Nakashian Date: Fri, 24 Apr 2026 05:29:31 -0400 Subject: [PATCH 02/15] browser: support non-wopi url and undefined accessTokenTtl Change-Id: Ieb4cc1a38a1e6322dd319c98df91c4d4a9fc93cf Signed-off-by: Ashod Nakashian --- browser/js/global.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/browser/js/global.js b/browser/js/global.js index d3413cef86fc5..713f9b9ba51ef 100644 --- a/browser/js/global.js +++ b/browser/js/global.js @@ -248,7 +248,7 @@ class InitializerBase { window.hexifyUrl = false; window.versionPath = ""; window.accessToken = element.dataset.accessToken; - window.accessTokenTTL = element.dataset.accessTokenTtl; + window.accessTokenTTL = element.dataset.accessTokenTtl || '0'; window.noAuthHeader = element.dataset.noAuthHeader; window.accessHeader = element.dataset.accessHeader; window.postMessageOriginExt = ""; @@ -1917,7 +1917,7 @@ function showWelcomeSVG() { global.webserver = global.webserver.replace(/\/*$/, ''); // Remove trailing slash. } - var docParams, wopiParams; + var docParams = '', wopiParams; var filePath = global.coolParams.get('file_path'); global.wopiSrc = global.coolParams.get('WOPISrc'); if (global.wopiSrc != '') { @@ -1981,7 +1981,8 @@ function showWelcomeSVG() { wopiSrc += '&RouteToken=' + global.routeToken; } - return root + '/ws' + wopiSrc + '&' + docParams; + var separator = wopiSrc ? '&' : (docParams ? '?' : ''); + return root + '/ws' + wopiSrc + separator + docParams; }; // Form a valid WS URL to the host with the given path and From 305c1b66e7784706df47f975319e961b95ac3a8c Mon Sep 17 00:00:00 2001 From: Ashod Nakashian Date: Wed, 22 Apr 2026 22:34:47 -0400 Subject: [PATCH 03/15] wsd: better RequestDetails serialization Change-Id: I3ffbeb3adbf25d80c8d47821aa1ccbdf4ee7b8e7 Signed-off-by: Ashod Nakashian --- wsd/RequestDetails.hpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/wsd/RequestDetails.hpp b/wsd/RequestDetails.hpp index 3316d78590f91..6a6c1a39cb981 100644 --- a/wsd/RequestDetails.hpp +++ b/wsd/RequestDetails.hpp @@ -370,14 +370,15 @@ class RequestDetails [[nodiscard]] std::string toString() const { std::ostringstream oss; - oss << _uriString << ' ' << nameShort(_method) - << (_isProxy?"Proxy":"") - << (_isWebSocket?"WebSocket":""); + oss << nameShort(_method) << ' ' << _uriString << ' ' << (_isProxy ? "Proxy" : "") + << (_isWebSocket ? "WebSocket" : ""); oss << ", host: " << _hostUntrusted; - oss << ", path: " << _pathSegs.size(); + oss << ", " << _pathSegs.size() << " path segments: "; for (std::size_t i = 0; i < _pathSegs.size(); ++i) - oss << "\n[" << i << "] '" << _pathSegs[i] << '\''; - oss << "\nfull URI: " << _uriString; + { + oss << '[' << i << "] '" << _pathSegs[i] << "', "; + } + return oss.str(); } }; From 3517078f9038228c9e4bee91b3593cb18ecf726f Mon Sep 17 00:00:00 2001 From: Ashod Nakashian Date: Thu, 23 Apr 2026 05:31:55 +0000 Subject: [PATCH 04/15] browser: use legacy WS URL path for non-WOPI documents Change-Id: I49d08ef5ab4594132765b3ade2e9f1cf1e13fbf9 Signed-off-by: Ashod Nakashian --- browser/js/global.js | 2 +- browser/src/app/Socket.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/browser/js/global.js b/browser/js/global.js index 713f9b9ba51ef..b7b78b8aea6b1 100644 --- a/browser/js/global.js +++ b/browser/js/global.js @@ -2042,7 +2042,7 @@ function showWelcomeSVG() { global.socket = new global.FakeWebSocket(); global.TheFakeWebSocket = global.socket; } else { - if (global.enableExperimentalFeatures) { + if (global.enableExperimentalFeatures && global.wopiSrc) { var websocketURI = global.makeWopiCoolWsUrl(global.makeWsUrl('/cool'), docParams); } else { // The URL may already contain a query (e.g., 'http://server.tld/foo/wopi/files/bar?desktop=baz') - then just append more params diff --git a/browser/src/app/Socket.ts b/browser/src/app/Socket.ts index 38970dbf64fdf..f43d24aa56f3d 100644 --- a/browser/src/app/Socket.ts +++ b/browser/src/app/Socket.ts @@ -236,8 +236,8 @@ class Socket { } private getWebSocketBaseURI(map: MapInterface): string { - if (window.enableExperimentalFeatures) { - // Use the new Cool WS URL. + if (window.enableExperimentalFeatures && map.options.wopiSrc) { + // Use the new Cool WS URL for WOPI documents. return window.makeWopiCoolWsUrl( window.makeWsUrl('/cool'), $.param(map.options.docParams), From dbc383a4c3146e8bd15649e301f826fe4196d945 Mon Sep 17 00:00:00 2001 From: Ashod Nakashian Date: Mon, 22 Sep 2025 18:55:01 -0400 Subject: [PATCH 05/15] wsd: resetaccesstoken supports access_token_ttl Adds an optional expiry time to the resetaccesstoken command. Authorization object also adds support to tracking this expiry time. Change-Id: I2c3b810bf73d6e03fc1ef22fd8fb6c08ee797efb Signed-off-by: Ashod Nakashian --- browser/html/framed.doc.html | 11 ++++++++--- browser/src/map/handler/Map.WOPI.js | 3 ++- common/Authorization.cpp | 13 ++++++++++++- common/Authorization.hpp | 20 ++++++++++++++++---- kit/ChildSession.cpp | 1 - wsd/ClientSession.cpp | 14 +++++++++++--- wsd/protocol.txt | 3 ++- 7 files changed, 51 insertions(+), 14 deletions(-) diff --git a/browser/html/framed.doc.html b/browser/html/framed.doc.html index 57239e6c86a27..7d75761cc46f6 100644 --- a/browser/html/framed.doc.html +++ b/browser/html/framed.doc.html @@ -293,9 +293,9 @@ post({'MessageId': messageId, 'Values': values}); } - function reset_access_token(accesstoken) { + function reset_access_token(accesstoken, ttl) { post({'MessageId': 'Reset_Access_Token', - 'Values': { 'token': accesstoken, } + 'Values': { 'token': accesstoken, 'ttl': ttl } }); } @@ -597,9 +597,14 @@

Various other messages to post

New Access-Token

+ + +
- +

User State

diff --git a/browser/src/map/handler/Map.WOPI.js b/browser/src/map/handler/Map.WOPI.js index 948fba0e89f7e..390df8a0d4efc 100644 --- a/browser/src/map/handler/Map.WOPI.js +++ b/browser/src/map/handler/Map.WOPI.js @@ -596,7 +596,8 @@ window.L.Map.WOPI = window.L.Handler.extend({ this._postViewsMessage('Get_Views_Resp'); } else if (msg.MessageId === 'Reset_Access_Token') { - app.socket.sendMessage('resetaccesstoken ' + msg.Values.token); + var ttl = msg.Values && msg.Values.ttl ? msg.Values['ttl'] : '0'; + app.socket.sendMessage('resetaccesstoken ' + msg.Values.token + ' ' + ttl); } else if (msg.MessageId === 'Action_Save') { var dontTerminateEdit = msg.Values && msg.Values['DontTerminateEdit']; diff --git a/common/Authorization.cpp b/common/Authorization.cpp index 9e76003935499..3dbcdc3b287d6 100644 --- a/common/Authorization.cpp +++ b/common/Authorization.cpp @@ -17,7 +17,9 @@ #include #include "Authorization.hpp" + #include +#include #include #include @@ -101,6 +103,7 @@ void Authorization::authorizeRequest(Poco::Net::HTTPRequest& request) const Authorization Authorization::create(const Poco::URI& uri) { bool noHeader = false; + duration expiryEpoch = duration::zero(); Authorization::Type type = Authorization::Type::None; std::string decoded; for (const auto& param : uri.getQueryParameters()) @@ -119,10 +122,18 @@ Authorization Authorization::create(const Poco::URI& uri) noHeader = true; } } + else if (param.first == "access_token_ttl") + { + expiryEpoch = duration(NumUtil::u64FromString(param.second, 0)); + } } if (!decoded.empty()) - return Authorization(type, std::move(decoded), noHeader); + { + Authorization auth(type, std::move(decoded), noHeader); + auth.setExpiryEpoch(expiryEpoch); + return auth; + } return Authorization(); } diff --git a/common/Authorization.hpp b/common/Authorization.hpp index 5697c90d33f13..17ddd170b043a 100644 --- a/common/Authorization.hpp +++ b/common/Authorization.hpp @@ -15,6 +15,7 @@ #include +#include #include namespace Poco @@ -31,6 +32,8 @@ class URI; /// Class to keep the authorization data, which can be either access_token or access_header. class Authorization { + using duration = std::chrono::milliseconds; + public: STATE_ENUM(Type, None, ///< Unlike Expired, this implies no Authorization needed. @@ -42,11 +45,11 @@ class Authorization private: std::string _data; Type _type; + duration _expiryEpoch; ///< Milliseconds from the epoch when the access_token will expire. bool _noHeader; Authorization() - : _type(Type::None) - , _noHeader(false) + : Authorization(Type::None, std::string(), false) { } @@ -54,6 +57,7 @@ class Authorization Authorization(Type type, std::string data, bool noHeader) : _data(std::move(data)) , _type(type) + , _expiryEpoch(duration::zero()) , _noHeader(noHeader) { } @@ -63,17 +67,25 @@ class Authorization static Authorization create(const Poco::URI& uri); static Authorization create(const std::string& uri); - void resetAccessToken(std::string accessToken) + void resetAccessToken(std::string accessToken, duration expiryEpoch) { _type = Type::Token; _data = std::move(accessToken); + _expiryEpoch = expiryEpoch; } /// Expire the Authorization data. void expire() { _type = Type::Expired; } + void setExpiryEpoch(duration epochMs) { _expiryEpoch = epochMs; } + /// Returns true iff the Authorization data is invalid. - bool isExpired() const { return _type == Type::Expired; } + bool isExpired() const + { + return _type == Type::Expired || + (_expiryEpoch > duration::zero() && + std::chrono::system_clock::now().time_since_epoch() > _expiryEpoch); + } /// Set the access_token parameter to the given URI. void authorizeURI(Poco::URI& uri) const; diff --git a/kit/ChildSession.cpp b/kit/ChildSession.cpp index bcfde62fa6de1..3e3f58b0a9bde 100644 --- a/kit/ChildSession.cpp +++ b/kit/ChildSession.cpp @@ -19,7 +19,6 @@ #include "ChildSession.hpp" #include -#include #include #include #include diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp index 1b096f7e9f945..6a3acd9f3be5b 100644 --- a/wsd/ClientSession.cpp +++ b/wsd/ClientSession.cpp @@ -51,6 +51,8 @@ #include #include +#include +#include #include #include #include @@ -1349,14 +1351,20 @@ bool ClientSession::_handleInput(const char *buffer, int length) #endif else if (tokens.equals(0, "resetaccesstoken")) { - if (tokens.size() != 2) + if (tokens.size() <= 1 || tokens.size() > 3) { LOG_ERR("Bad syntax for: " << tokens[0]); - sendTextFrameAndLogError("error: cmd=resetaccesstoken kind=syntax"); + sendTextFrameAndLogError("error: cmd=" + tokens[0] + " kind=syntax"); return false; } - _auth.resetAccessToken(tokens[1]); + // Get the access_token_ttl, if provided. 0 means unknown/missing. + const auto expiryEpoch = std::chrono::milliseconds( + (tokens.size() == 3) ? NumUtil::u64FromString(tokens[2], 0) : 0); + + LOG_DBG("Resetting access token for " << getName() << " with expiry at " << expiryEpoch + << ": " << tokens[1]); + _auth.resetAccessToken(tokens[1], expiryEpoch); return true; } #if !MOBILEAPP && !WASMAPP diff --git a/wsd/protocol.txt b/wsd/protocol.txt index e9287581756d9..378234e0ffd45 100644 --- a/wsd/protocol.txt +++ b/wsd/protocol.txt @@ -470,12 +470,13 @@ getslide hash= part= width= height=: The device pixel ratio, typically window.devicePixelRatio, which adjusts the rendering quality for HiDPI displays. -resetaccesstoken > +resetaccesstoken > [] This command ensures that the client uses the updated access token for further requests. Resets the access token for authentication or session management. - : The new access token that replaces the previous one. + - : The time from Unix/Javascript epoch when the access token will expire. Optional. 0 means unknown/missing. renamefile > From fcf159c90bce5af95bf22b106aae6e31c4b78aa8 Mon Sep 17 00:00:00 2001 From: Ashod Nakashian Date: Sun, 19 Apr 2026 11:31:05 -0400 Subject: [PATCH 06/15] wsd: Authorization supports refreshing tokens With tests. Change-Id: I37debc0123e46e582d6506c57326405cdb22d946 Signed-off-by: Ashod Nakashian --- common/Authorization.cpp | 14 ++++- common/Authorization.hpp | 46 +++++++++++++- test/RequestDetailsTests.cpp | 119 +++++++++++++++++++++++++++++++++++ 3 files changed, 175 insertions(+), 4 deletions(-) diff --git a/common/Authorization.cpp b/common/Authorization.cpp index 3dbcdc3b287d6..d4adba8273a2c 100644 --- a/common/Authorization.cpp +++ b/common/Authorization.cpp @@ -131,7 +131,19 @@ Authorization Authorization::create(const Poco::URI& uri) if (!decoded.empty()) { Authorization auth(type, std::move(decoded), noHeader); - auth.setExpiryEpoch(expiryEpoch); + if (expiryEpoch > duration::zero()) + { + if (type != Authorization::Type::Token) + { + LOG_WRN("Ignoring invalid access_token_ttl with [" + << name(type) << "] authorization type in uri [" << uri.toString() << ']'); + } + else + { + auth.setExpiryEpoch(expiryEpoch); + } + } + return auth; } diff --git a/common/Authorization.hpp b/common/Authorization.hpp index 17ddd170b043a..386b977b617d4 100644 --- a/common/Authorization.hpp +++ b/common/Authorization.hpp @@ -13,6 +13,7 @@ #pragma once +#include #include #include @@ -39,6 +40,7 @@ class Authorization None, ///< Unlike Expired, this implies no Authorization needed. Token, ///< Valid access_token -> "Authorization: Bearer ..." header. Header, ///< Valid access_header -> Custom header(s). + TokenRefresh, ///< Pending a Token refresh from integration. Expired ///< The server is rejecting the current authorization key. ); @@ -46,6 +48,8 @@ class Authorization std::string _data; Type _type; duration _expiryEpoch; ///< Milliseconds from the epoch when the access_token will expire. + std::chrono::steady_clock::time_point _tokenRefreshStartTime; ///< Only when refreshing. + std::chrono::seconds _tokenRefreshTimeout; ///< Maximum time to wait for Token refresh. bool _noHeader; Authorization() @@ -58,6 +62,8 @@ class Authorization : _data(std::move(data)) , _type(type) , _expiryEpoch(duration::zero()) + , _tokenRefreshStartTime(duration::zero()) + , _tokenRefreshTimeout(std::chrono::seconds::zero()) , _noHeader(noHeader) { } @@ -74,12 +80,46 @@ class Authorization _expiryEpoch = expiryEpoch; } + /// Returns true iff Type is Token and we passed the expiry-epoch, or + /// we are refreshing already. + bool needTokenRefresh() const + { + return _type == Type::TokenRefresh || + (_type == Type::Token && _expiryEpoch > duration::zero() && + std::chrono::system_clock::now().time_since_epoch() > _expiryEpoch); + } + + /// Start waiting for a token refresh. + void startTokenRefresh(const std::chrono::seconds timeout) + { + LOG_ASSERT_MSG(_type == Type::Token, "Token refresh is meaningful only for access_token"); + _type = Type::TokenRefresh; + _tokenRefreshStartTime = std::chrono::steady_clock::now(); + _tokenRefreshTimeout = timeout; + } + + /// Returns true iff we are refreshing the token. + bool isRefreshingToken() const { return _type == Type::TokenRefresh; } + + /// Returns true if the timeout has elapsed without a refresh. + bool isTokenRefreshTimedOut( + const std::chrono::steady_clock::time_point now = std::chrono::steady_clock::now()) const + { + return isRefreshingToken() && (now - _tokenRefreshStartTime) >= _tokenRefreshTimeout; + } + + /// Sets the Token's expiry time from the epoch. + void setExpiryEpoch(duration epochMs) + { + LOG_ASSERT_MSG(epochMs == duration::zero() || _type == Type::Token, + "Token expiry is meaningful only for access_token"); + _expiryEpoch = epochMs; + } + /// Expire the Authorization data. void expire() { _type = Type::Expired; } - void setExpiryEpoch(duration epochMs) { _expiryEpoch = epochMs; } - - /// Returns true iff the Authorization data is invalid. + /// Returns true if Type is Expired or we passed the expiry-epoch. bool isExpired() const { return _type == Type::Expired || diff --git a/test/RequestDetailsTests.cpp b/test/RequestDetailsTests.cpp index bdeefbaa67262..afb7ef50f7344 100644 --- a/test/RequestDetailsTests.cpp +++ b/test/RequestDetailsTests.cpp @@ -38,6 +38,7 @@ class RequestDetailsTests : public CPPUNIT_NS::TestFixture CPPUNIT_TEST(testRequestDetails); CPPUNIT_TEST(testCoolWs); CPPUNIT_TEST(testAuthorization); + CPPUNIT_TEST(testAuthorizationExpiry); CPPUNIT_TEST(testSanitizePercent); CPPUNIT_TEST_SUITE_END(); @@ -49,6 +50,7 @@ class RequestDetailsTests : public CPPUNIT_NS::TestFixture void testRequestDetails(); void testCoolWs(); void testAuthorization(); + void testAuthorizationExpiry(); void testSanitizePercent(); }; @@ -1349,6 +1351,123 @@ void RequestDetailsTests::testAuthorization() } } +void RequestDetailsTests::testAuthorizationExpiry() +{ + constexpr std::string_view testname = __func__; + + using duration = std::chrono::milliseconds; + + // A token with no expiry should not be expired. + { + Authorization auth(Authorization::Type::Token, "tok1", false); + LOK_ASSERT(!auth.isExpired()); + LOK_ASSERT(!auth.needTokenRefresh()); + } + + // A token with a far-future expiry should not be expired. + { + Authorization auth(Authorization::Type::Token, "tok2", false); + const auto futureMs = std::chrono::system_clock::now().time_since_epoch() + + std::chrono::hours(1); + auth.setExpiryEpoch(std::chrono::duration_cast(futureMs)); + LOK_ASSERT(!auth.isExpired()); + LOK_ASSERT(!auth.needTokenRefresh()); + } + + // A token with a past expiry should be expired. + { + Authorization auth(Authorization::Type::Token, "tok3", false); + const auto pastMs = std::chrono::system_clock::now().time_since_epoch() - + std::chrono::seconds(1); + auth.setExpiryEpoch(std::chrono::duration_cast(pastMs)); + LOK_ASSERT(auth.isExpired()); + LOK_ASSERT(auth.needTokenRefresh()); + // Regression: a naturally-expired Token (startTokenRefresh never called) must + // not report a refresh-wait timeout, otherwise the poll-loop kills the session. + LOK_ASSERT(!auth.isRefreshingToken()); + LOK_ASSERT(!auth.isTokenRefreshTimedOut()); + } + + // expire() should mark as expired regardless of TTL. + { + Authorization auth(Authorization::Type::Token, "tok4", false); + LOK_ASSERT(!auth.isExpired()); + auth.expire(); + LOK_ASSERT(auth.isExpired()); + } + + // resetAccessToken should clear expired state. + { + Authorization auth(Authorization::Type::Token, "tok5", false); + auth.expire(); + LOK_ASSERT(auth.isExpired()); + const auto futureMs = std::chrono::system_clock::now().time_since_epoch() + + std::chrono::hours(1); + auth.resetAccessToken("tok5_new", + std::chrono::duration_cast(futureMs)); + LOK_ASSERT(!auth.isExpired()); + LOK_ASSERT(!auth.needTokenRefresh()); + } + + // resetAccessToken with a past expiry should be expired. + { + Authorization auth(Authorization::Type::Token, "tok6", false); + const auto pastMs = std::chrono::system_clock::now().time_since_epoch() - + std::chrono::seconds(1); + auth.resetAccessToken("tok6_new", + std::chrono::duration_cast(pastMs)); + LOK_ASSERT(auth.isExpired()); + } + + // Authorization::create with access_token_ttl should set expiry. + { + const auto futureMs = std::chrono::duration_cast( + std::chrono::system_clock::now().time_since_epoch() + std::chrono::hours(1)); + const std::string uri = "http://host/wopi/files/0?access_token=secret&access_token_ttl=" + + std::to_string(futureMs.count()); + Authorization auth = Authorization::create(uri); + LOK_ASSERT(!auth.isExpired()); + } + + // Authorization::create with past access_token_ttl should be expired. + { + const auto pastMs = std::chrono::duration_cast( + std::chrono::system_clock::now().time_since_epoch() - std::chrono::seconds(1)); + const std::string uri = "http://host/wopi/files/0?access_token=secret&access_token_ttl=" + + std::to_string(pastMs.count()); + Authorization auth = Authorization::create(uri); + LOK_ASSERT(auth.isExpired()); + } + + // Token refresh: startTokenRefresh, isRefreshingToken, isTokenRefreshTimedOut. + { + Authorization auth(Authorization::Type::Token, "tok7", false); + LOK_ASSERT(!auth.isRefreshingToken()); + + auth.startTokenRefresh(std::chrono::seconds(1)); + LOK_ASSERT(auth.isRefreshingToken()); + LOK_ASSERT(auth.needTokenRefresh()); + + // Should not have timed out yet. + LOK_ASSERT(!auth.isTokenRefreshTimedOut()); + + // Reset should clear the refreshing state. + const auto futureMs = std::chrono::system_clock::now().time_since_epoch() + + std::chrono::hours(1); + auth.resetAccessToken("tok7_new", + std::chrono::duration_cast(futureMs)); + LOK_ASSERT(!auth.isRefreshingToken()); + LOK_ASSERT(!auth.needTokenRefresh()); + } + + // Type::None should not be expired. + { + Authorization auth(Authorization::Type::None, "", false); + LOK_ASSERT(!auth.isExpired()); + LOK_ASSERT(!auth.needTokenRefresh()); + } +} + void RequestDetailsTests::testSanitizePercent() { constexpr std::string_view testname = __func__; From c2ed1559124ddedc5e4b346579f30b6b9d347adb Mon Sep 17 00:00:00 2001 From: Ashod Nakashian Date: Mon, 20 Apr 2026 19:32:28 -0400 Subject: [PATCH 07/15] wsd: dump state for Authorization Change-Id: Ibedbf4126bdb45422a99c94b9bb52e557bd4b6ee Signed-off-by: Ashod Nakashian --- common/Authorization.hpp | 18 +++++++++ test/RequestDetailsTests.cpp | 77 ++++++++++++++++++++++++++++++++++++ wsd/ClientSession.cpp | 7 +++- 3 files changed, 100 insertions(+), 2 deletions(-) diff --git a/common/Authorization.hpp b/common/Authorization.hpp index 386b977b617d4..9a828d284bd54 100644 --- a/common/Authorization.hpp +++ b/common/Authorization.hpp @@ -13,6 +13,7 @@ #pragma once +#include #include #include @@ -132,6 +133,23 @@ class Authorization /// Set the Authorization: header in request. void authorizeRequest(Poco::Net::HTTPRequest& request) const; + + void dumpState(std::ostream& os, const std::string& indent = "\n ") const + { + const auto now = std::chrono::system_clock::now(); + + os << indent << "Authorization: " << (Anonymizer::enabled() ? "" : _data); + os << indent << "\ttype: " << name(_type); + os << indent << "\texpiryEpoch (TTL): " << _expiryEpoch + << Util::getTimeForLog( + now, std::chrono::system_clock::time_point( + std::chrono::duration_cast( + _expiryEpoch))); + os << indent + << "\ttokenRefreshStartTime: " << Util::getTimeForLog(now, _tokenRefreshStartTime); + os << indent << "\ttokenRefreshTimeout: " << _tokenRefreshTimeout; + os << indent << "\theader: " << (_noHeader ? "No" : "Yes"); + } }; /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/test/RequestDetailsTests.cpp b/test/RequestDetailsTests.cpp index afb7ef50f7344..3e4c0feb1c857 100644 --- a/test/RequestDetailsTests.cpp +++ b/test/RequestDetailsTests.cpp @@ -39,6 +39,7 @@ class RequestDetailsTests : public CPPUNIT_NS::TestFixture CPPUNIT_TEST(testCoolWs); CPPUNIT_TEST(testAuthorization); CPPUNIT_TEST(testAuthorizationExpiry); + CPPUNIT_TEST(testAuthorizationDumpState); CPPUNIT_TEST(testSanitizePercent); CPPUNIT_TEST_SUITE_END(); @@ -51,6 +52,7 @@ class RequestDetailsTests : public CPPUNIT_NS::TestFixture void testCoolWs(); void testAuthorization(); void testAuthorizationExpiry(); + void testAuthorizationDumpState(); void testSanitizePercent(); }; @@ -1468,6 +1470,81 @@ void RequestDetailsTests::testAuthorizationExpiry() } } +void RequestDetailsTests::testAuthorizationDumpState() +{ + constexpr std::string_view testname = __func__; + + using duration = std::chrono::milliseconds; + + // Token type should dump type and data. + { + Authorization auth(Authorization::Type::Token, "secret123", false); + std::ostringstream oss; + auth.dumpState(oss); + const std::string dump = oss.str(); + LOK_ASSERT_MESSAGE("Should contain token data", dump.find("secret123") != std::string::npos); + LOK_ASSERT_MESSAGE("Should contain type", dump.find("Token") != std::string::npos); + LOK_ASSERT_MESSAGE("Should contain header info", dump.find("header:") != std::string::npos); + } + + // Expired type should show Expired. + { + Authorization auth(Authorization::Type::Token, "tok", false); + auth.expire(); + std::ostringstream oss; + auth.dumpState(oss); + LOK_ASSERT_MESSAGE("Should show Expired", oss.str().find("Expired") != std::string::npos); + } + + // TokenRefresh type should show TokenRefresh. + { + Authorization auth(Authorization::Type::Token, "tok", false); + auth.startTokenRefresh(std::chrono::seconds(10)); + std::ostringstream oss; + auth.dumpState(oss); + LOK_ASSERT_MESSAGE("Should show TokenRefresh", + oss.str().find("TokenRefresh") != std::string::npos); + } + + // None type should show None. + { + Authorization auth(Authorization::Type::None, "", false); + std::ostringstream oss; + auth.dumpState(oss); + LOK_ASSERT_MESSAGE("Should show None", oss.str().find("None") != std::string::npos); + } + + // Header type should show Header. + { + Authorization auth(Authorization::Type::Header, "Authorization: Basic abc==", false); + std::ostringstream oss; + auth.dumpState(oss); + LOK_ASSERT_MESSAGE("Should show Header", oss.str().find("Header") != std::string::npos); + } + + // With expiry set, dump should contain TTL info. + { + Authorization auth(Authorization::Type::Token, "tok", false); + const auto futureMs = std::chrono::system_clock::now().time_since_epoch() + + std::chrono::hours(1); + auth.setExpiryEpoch(std::chrono::duration_cast(futureMs)); + std::ostringstream oss; + auth.dumpState(oss); + LOK_ASSERT_MESSAGE("Should contain TTL info", oss.str().find("TTL") != std::string::npos); + LOK_ASSERT_MESSAGE("Should contain 'later' for future expiry", + oss.str().find("later") != std::string::npos); + } + + // noHeader flag should show "No". + { + Authorization auth(Authorization::Type::Token, "tok", true); + std::ostringstream oss; + auth.dumpState(oss); + LOK_ASSERT_MESSAGE("Should show header: No", + oss.str().find("header: No") != std::string::npos); + } +} + void RequestDetailsTests::testSanitizePercent() { constexpr std::string_view testname = __func__; diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp index 6a3acd9f3be5b..b6437f15054b1 100644 --- a/wsd/ClientSession.cpp +++ b/wsd/ClientSession.cpp @@ -3616,8 +3616,11 @@ void ClientSession::dumpState(std::ostream& os) os << "\t\tisLive: " << isLive() << "\n\t\tisViewLoaded: " << isViewLoaded() << "\n\t\tisDocumentOwner: " << isDocumentOwner() - << "\n\t\tstate: " << name(_state) - << "\n\t\tkeyEvents: " << _keyEvents + << "\n\t\tstate: " << name(_state); + + _auth.dumpState(os); + + os << "\n\t\tkeyEvents: " << _keyEvents // << "\n\t\tvisibleArea: " << _clientVisibleArea << "\n\t\tclientSelectedPart: " << _clientSelectedPart << "\n\t\ttile size Pixel: " << _tileWidthPixel << 'x' << _tileHeightPixel From 1437dd41f4ccd566eb9333afa29ff5b9cdab534b Mon Sep 17 00:00:00 2001 From: Ashod Nakashian Date: Mon, 20 Apr 2026 06:16:26 -0400 Subject: [PATCH 08/15] wsd: use Authorization::isValid() instead of isExpired() isValid covers the new Refresh-Token state. With tests. Change-Id: Ic9a8ebd1010195118fc143f74f9a598bd35a5092 Signed-off-by: Ashod Nakashian --- common/Authorization.hpp | 13 ++++++ test/RequestDetailsTests.cpp | 82 ++++++++++++++++++++++++++++++++++++ wsd/DocumentBroker.cpp | 16 +++---- 3 files changed, 103 insertions(+), 8 deletions(-) diff --git a/common/Authorization.hpp b/common/Authorization.hpp index 9a828d284bd54..6840d8340def2 100644 --- a/common/Authorization.hpp +++ b/common/Authorization.hpp @@ -128,6 +128,19 @@ class Authorization std::chrono::system_clock::now().time_since_epoch() > _expiryEpoch); } + /// Returns true iff neither expired nor refreshing. + bool isValid() const + { + if (_type == Type::Expired || _type == Type::TokenRefresh) + { + return false; + } + + // Check TTL-based expiry only for tokens with a known expiry. + return _expiryEpoch <= duration::zero() || + std::chrono::system_clock::now().time_since_epoch() < _expiryEpoch; + } + /// Set the access_token parameter to the given URI. void authorizeURI(Poco::URI& uri) const; diff --git a/test/RequestDetailsTests.cpp b/test/RequestDetailsTests.cpp index 3e4c0feb1c857..3633033f1d95e 100644 --- a/test/RequestDetailsTests.cpp +++ b/test/RequestDetailsTests.cpp @@ -39,6 +39,7 @@ class RequestDetailsTests : public CPPUNIT_NS::TestFixture CPPUNIT_TEST(testCoolWs); CPPUNIT_TEST(testAuthorization); CPPUNIT_TEST(testAuthorizationExpiry); + CPPUNIT_TEST(testAuthorizationIsValid); CPPUNIT_TEST(testAuthorizationDumpState); CPPUNIT_TEST(testSanitizePercent); @@ -52,6 +53,7 @@ class RequestDetailsTests : public CPPUNIT_NS::TestFixture void testCoolWs(); void testAuthorization(); void testAuthorizationExpiry(); + void testAuthorizationIsValid(); void testAuthorizationDumpState(); void testSanitizePercent(); }; @@ -1470,6 +1472,86 @@ void RequestDetailsTests::testAuthorizationExpiry() } } +void RequestDetailsTests::testAuthorizationIsValid() +{ + constexpr std::string_view testname = __func__; + + using duration = std::chrono::milliseconds; + + // Type::Token with no expiry is valid. + { + Authorization auth(Authorization::Type::Token, "tok", false); + LOK_ASSERT(auth.isValid()); + } + + // Type::Header is valid. + { + Authorization auth(Authorization::Type::Header, "Authorization: Basic abc==", false); + LOK_ASSERT(auth.isValid()); + } + + // Type::None is valid (no credentials to use). + { + Authorization auth(Authorization::Type::None, "", false); + LOK_ASSERT(auth.isValid()); + } + + // Type::Expired is not valid. + { + Authorization auth(Authorization::Type::Token, "tok", false); + auth.expire(); + LOK_ASSERT(!auth.isValid()); + } + + // Token with past expiry is not valid. + { + Authorization auth(Authorization::Type::Token, "tok", false); + const auto pastMs = std::chrono::system_clock::now().time_since_epoch() - + std::chrono::seconds(1); + auth.setExpiryEpoch(std::chrono::duration_cast(pastMs)); + LOK_ASSERT(!auth.isValid()); + } + + // Token with future expiry is valid. + { + Authorization auth(Authorization::Type::Token, "tok", false); + const auto futureMs = std::chrono::system_clock::now().time_since_epoch() + + std::chrono::hours(1); + auth.setExpiryEpoch(std::chrono::duration_cast(futureMs)); + LOK_ASSERT(auth.isValid()); + } + + // TokenRefresh state is not valid (mid-refresh, can't use for WOPI calls). + { + Authorization auth(Authorization::Type::Token, "tok", false); + auth.startTokenRefresh(std::chrono::seconds(30)); + LOK_ASSERT(!auth.isValid()); + } + + // After resetAccessToken, should be valid again. + { + Authorization auth(Authorization::Type::Token, "tok", false); + auth.expire(); + LOK_ASSERT(!auth.isValid()); + const auto futureMs = std::chrono::system_clock::now().time_since_epoch() + + std::chrono::hours(1); + auth.resetAccessToken("newtok", std::chrono::duration_cast(futureMs)); + LOK_ASSERT(auth.isValid()); + } + + // create() with access_token produces a valid auth. + { + Authorization auth = Authorization::create("http://host/wopi/files/0?access_token=secret"); + LOK_ASSERT(auth.isValid()); + } + + // create() without access_token produces Type::None. + { + Authorization auth = Authorization::create("http://host/wopi/files/0"); + LOK_ASSERT(auth.isValid()); + } +} + void RequestDetailsTests::testAuthorizationDumpState() { constexpr std::string_view testname = __func__; diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp index 19fea99b2000b..f6fbc0c8da390 100644 --- a/wsd/DocumentBroker.cpp +++ b/wsd/DocumentBroker.cpp @@ -608,7 +608,7 @@ void DocumentBroker::pollThread() { // Retry uploading, if the last one failed and we can try again. const auto session = getWriteableSession(); - if (session && !session->getAuthorization().isExpired()) + if (session && session->getAuthorization().isValid()) { checkAndUploadToStorage(session, /*justSaved=*/false); } @@ -767,7 +767,7 @@ void DocumentBroker::pollThread() LOG_ERR("No write-able session to unlock with"); _lockCtx->bumpTimer(); } - else if (session->getAuthorization().isExpired()) + else if (!session->getAuthorization().isValid()) { LOG_ERR("No write-able session with valid authorization to unlock with"); _lockCtx->bumpTimer(); @@ -2340,7 +2340,7 @@ bool DocumentBroker::updateStorageLockState(ClientSession& session, StorageBase: LOG_TRC("Requesting async " << StorageBase::nameShort(lock) << "ing of [" << _docKey << "] by session #" << session.getId()); - if (session.getAuthorization().isExpired()) + if (!session.getAuthorization().isValid()) { error = "Expired authorization token"; return false; @@ -2371,7 +2371,7 @@ bool DocumentBroker::updateStorageLockStateAsync(const std::shared_ptrgetId()); - if (session->getAuthorization().isExpired()) + if (!session->getAuthorization().isValid()) { error = "Expired authorization token"; return false; @@ -3447,7 +3447,7 @@ std::shared_ptr DocumentBroker::getFirstAuthorizedSession() const for (const auto& sessionIt : _sessions) { const auto& session = sessionIt.second; - if (!session->getAuthorization().isExpired()) + if (session->getAuthorization().isValid()) { return session; } @@ -3469,7 +3469,7 @@ std::shared_ptr DocumentBroker::getWriteableSession() const // with a valid authorization token, or the first. // Note that isViewLoaded() precludes inWaitDisconnected(). if (!savingSession || (session->isViewLoaded() && session->isEditable() && - !session->getAuthorization().isExpired())) + session->getAuthorization().isValid())) { savingSession = session; } @@ -3496,7 +3496,7 @@ void DocumentBroker::refreshLock() LOG_ERR("No write-able session to refresh lock with"); _lockCtx->bumpTimer(); } - else if (session->getAuthorization().isExpired()) + else if (!session->getAuthorization().isValid()) { LOG_ERR("No write-able session with valid authorization to refresh lock with"); _lockCtx->bumpTimer(); @@ -3779,7 +3779,7 @@ void DocumentBroker::autoSaveAndStop(const std::string_view reason) { // Nothing to save. Try to upload if necessary. const auto session = getWriteableSession(); - if (session && !session->getAuthorization().isExpired()) + if (session && session->getAuthorization().isValid()) { checkAndUploadToStorage(session, /*justSaved=*/false); if (isAsyncUploading()) From a9166f3a4ce0ef0c8b8be017ee14039ec4cbad17 Mon Sep 17 00:00:00 2001 From: Ashod Nakashian Date: Thu, 9 Apr 2026 01:24:09 -0400 Subject: [PATCH 09/15] browser: reset token expiry timer on Reset_Access_Token Change-Id: Ie9b20763c9cfb1e115156b8c063f9d6e37fa73a3 Signed-off-by: Ashod Nakashian --- browser/src/app/Socket.ts | 24 +++++++++++++----------- browser/src/map/handler/Map.WOPI.js | 10 ++++++++-- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/browser/src/app/Socket.ts b/browser/src/app/Socket.ts index f43d24aa56f3d..0f34089d43742 100644 --- a/browser/src/app/Socket.ts +++ b/browser/src/app/Socket.ts @@ -296,22 +296,24 @@ class Socket { this._connectCount++; this._faultInjection(); - if ( - map.options.docParams.access_token && - parseInt(map.options.docParams.access_token_ttl as string) - ) { + this.resetTokenExpiryTimer(); + + // process messages for early socket connection + this._emptyQueue(); + } + + public resetTokenExpiryTimer(): void { + clearTimeout(this._accessTokenExpireTimeout); // Always clear the old timer. + const ttl = parseInt( + this._map.options.docParams.access_token_ttl as string, + ); + if (this._map.options.docParams.access_token && ttl) { const tokenExpiryWarning = 900 * 1000; // Warn when 15 minutes remain - clearTimeout(this._accessTokenExpireTimeout); this._accessTokenExpireTimeout = setTimeout( this._sessionExpiredWarning.bind(this), - parseInt(map.options.docParams.access_token_ttl as string) - - Date.now() - - tokenExpiryWarning, + ttl - Date.now() - tokenExpiryWarning, ); } - - // process messages for early socket connection - this._emptyQueue(); } public close(code?: number, reason?: string): void { diff --git a/browser/src/map/handler/Map.WOPI.js b/browser/src/map/handler/Map.WOPI.js index 390df8a0d4efc..4f781710817f8 100644 --- a/browser/src/map/handler/Map.WOPI.js +++ b/browser/src/map/handler/Map.WOPI.js @@ -596,8 +596,14 @@ window.L.Map.WOPI = window.L.Handler.extend({ this._postViewsMessage('Get_Views_Resp'); } else if (msg.MessageId === 'Reset_Access_Token') { - var ttl = msg.Values && msg.Values.ttl ? msg.Values['ttl'] : '0'; - app.socket.sendMessage('resetaccesstoken ' + msg.Values.token + ' ' + ttl); + if (msg.Values) { + // No ttl implies no expiry tracking, matching the legacy + // single-arg form of the resetaccesstoken protocol command. + var ttl = msg.Values.ttl ? msg.Values['ttl'] : '0'; + app.socket.sendMessage('resetaccesstoken ' + msg.Values.token + ' ' + ttl); + this._map.options.docParams.access_token_ttl = ttl; + app.socket.resetTokenExpiryTimer(); + } } else if (msg.MessageId === 'Action_Save') { var dontTerminateEdit = msg.Values && msg.Values['DontTerminateEdit']; From c64d6759dad17efc0456593ec2b1f50f29cf8771 Mon Sep 17 00:00:00 2001 From: Ashod Nakashian Date: Thu, 9 Apr 2026 01:29:21 -0400 Subject: [PATCH 10/15] browser: send App_TokenExpiring PostMessage to host When the token expiry warning timer fires, send an App_TokenExpiring PostMessage to the integration. This prompts the integration to take action and keep the user's session valid. Change-Id: Iee7d838c173d40b33954807aa92ca6fc1e5ac7eb Signed-off-by: Ashod Nakashian --- browser/src/app/Socket.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/browser/src/app/Socket.ts b/browser/src/app/Socket.ts index 0f34089d43742..223b8c21c7b22 100644 --- a/browser/src/app/Socket.ts +++ b/browser/src/app/Socket.ts @@ -656,6 +656,17 @@ class Socket { const timerepr = dateTime.toLocaleDateString(String.locale, dateOptions); this._map.fire('warn', { msg: expirymsg.replace('{time}', timerepr) }); + // Notify the host so it can refresh the token programmatically. + const remainingMs = + parseInt(this._map.options.docParams.access_token_ttl as string) - + Date.now(); + this._map.fire('postMessage', { + msgId: 'App_TokenExpiring', + args: { + Timeout: Math.max(remainingMs, 0), + }, + }); + // If user still doesn't refresh the session, warn again periodically this._accessTokenExpireTimeout = setTimeout( this._sessionExpiredWarning.bind(this), From 9de6c14b9198a9247ef446d0ea2cf1539bcdae91 Mon Sep 17 00:00:00 2001 From: Ashod Nakashian Date: Thu, 9 Apr 2026 01:37:53 -0400 Subject: [PATCH 11/15] wsd: retry save on upload failure after requesting token refresh When a PutFile gets an unauthorized respose, instead of immediately invalidating the token and going read-only, notify the client via a 'tokenexpired' message. The client sends App_TokenExpired PostMessage to the host, giving it a chance to send a fresh token via Reset_Access_Token. Change-Id: I183a442ddf7908065d89d2f334ea9647e34d683e Signed-off-by: Ashod Nakashian --- browser/src/app/Socket.ts | 7 +++ wsd/ClientSession.cpp | 4 ++ wsd/ClientSession.hpp | 17 ++++++++ wsd/DocumentBroker.cpp | 89 ++++++++++++++++++++++++++++++++------- wsd/DocumentBroker.hpp | 4 ++ 5 files changed, 105 insertions(+), 16 deletions(-) diff --git a/browser/src/app/Socket.ts b/browser/src/app/Socket.ts index 223b8c21c7b22..c580c93ada5ef 100644 --- a/browser/src/app/Socket.ts +++ b/browser/src/app/Socket.ts @@ -1155,6 +1155,13 @@ class Socket { } else if (textMsg.startsWith('migrate:') && window.indirectSocket) { this._onMigrateMsg(textMsg); return; + } else if (textMsg.startsWith('tokenexpired')) { + // Server got a 401 on save. Ask the host for a fresh token. + this._map.fire('postMessage', { + msgId: 'App_TokenExpired', + args: {}, + }); + return; } else if (textMsg.startsWith('close: ')) { this._onCloseMsg(textMsg); return; diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp index b6437f15054b1..b0aa41a740be1 100644 --- a/wsd/ClientSession.cpp +++ b/wsd/ClientSession.cpp @@ -1365,6 +1365,10 @@ bool ClientSession::_handleInput(const char *buffer, int length) LOG_DBG("Resetting access token for " << getName() << " with expiry at " << expiryEpoch << ": " << tokens[1]); _auth.resetAccessToken(tokens[1], expiryEpoch); + + // Notify the DocumentBroker in case a save is waiting for a token refresh. + docBroker->onTokenRefreshed(client_from_this()); + return true; } #if !MOBILEAPP && !WASMAPP diff --git a/wsd/ClientSession.hpp b/wsd/ClientSession.hpp index cdab3fa7502cd..7b7c3482575cb 100644 --- a/wsd/ClientSession.hpp +++ b/wsd/ClientSession.hpp @@ -199,6 +199,23 @@ class ClientSession final : public Session _auth.expire(); } + /// Start waiting for a token refresh from the host. + void startTokenRefresh(const std::chrono::seconds timeout) + { + LOG_DBG("Session [" << getId() << "] starting token refresh wait"); + _auth.startTokenRefresh(timeout); + } + + /// Returns true iff this session is waiting for a token refresh. + bool isRefreshingToken() const { return _auth.isRefreshingToken(); } + + /// Returns true if the token refresh wait has timed out. + bool isTokenRefreshTimedOut( + const std::chrono::steady_clock::time_point now = std::chrono::steady_clock::now()) const + { + return _auth.isTokenRefreshTimedOut(now); + } + /// Set WOPI fileinfo object void setWopiFileInfo(std::unique_ptr wopiFileInfo) { _wopiFileInfo = std::move(wopiFileInfo); } diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp index f6fbc0c8da390..ef18ed45257ee 100644 --- a/wsd/DocumentBroker.cpp +++ b/wsd/DocumentBroker.cpp @@ -513,6 +513,22 @@ void DocumentBroker::pollThread() _uploadRequest.reset(); } + // Check if any session's token refresh wait has timed out. + for (const auto& it : _sessions) + { + const auto& session = it.second; + if (session->isTokenRefreshTimedOut(now)) + { + LOG_WRN("Token refresh timed out for session [" + << session->getId() << "] on docKey [" << _docKey << ']'); + session->sendTextFrameAndLogError( + "error: cmd=storage kind=saveunauthorized"); + session->invalidateAuthorizationToken(); + broadcastSaveResult(false, "Invalid or expired access token"); + break; // Only one refresh at a time. + } + } + // Check if there are queued activities. if (!_renameFilename.empty() && !_renameSessionId.empty()) { @@ -2675,6 +2691,24 @@ void DocumentBroker::handleSaveResponse(const std::shared_ptr& se // This is called when either we just got save response, or, // there was nothing to save and want to check for uploading. +void DocumentBroker::onTokenRefreshed(const std::shared_ptr& session) +{ + ASSERT_CORRECT_THREAD(); + assert(session && "Expected a valid session for onTokenRefreshed"); + + if (!_storageManager.lastUploadSuccessful()) + { + LOG_INF("Token refreshed for session [" << session->getId() << "] on docKey [" << _docKey + << "]. Retrying upload"); + checkAndUploadToStorage(session, /*justSaved=*/false); + } + else + { + LOG_INF("Token refreshed for session [" << session->getId() << "] on docKey [" << _docKey + << "] but no upload needs retrying"); + } +} + void DocumentBroker::checkAndUploadToStorage(const std::shared_ptr& session, bool justSaved) { @@ -3293,27 +3327,41 @@ void DocumentBroker::handleUploadToStorageFailed(const StorageBase::UploadResult { LOG_DBG("Last upload result: UNAUTHORIZED"); const auto session = _uploadRequest->session(); - if (session) + if (session && !session->isRefreshingToken()) { - LOG_ERR( - "Cannot upload docKey [" - << _docKey << "] to storage URI [" << _uploadRequest->uriAnonym() << "] of " - << _storageManager.getSizeAsUploaded() - << " bytes. Invalid or expired access token. Notifying client and invalidating the " - "authorization token of session [" - << session->getId() << ']'); - session->sendTextFrameAndLogError("error: cmd=storage kind=saveunauthorized"); - session->invalidateAuthorizationToken(); + // First attempt: ask the host for a fresh token before giving up. + LOG_WRN("Cannot upload docKey [" + << _docKey << "] to storage URI [" << _uploadRequest->uriAnonym() + << "]. Invalid or expired access token. " + "Requesting token refresh from session [" + << session->getId() << ']'); + session->sendTextFrame("tokenexpired"); + session->startTokenRefresh(); } else { - LOG_ERR("Cannot upload docKey [" - << _docKey << "] to storage URI [" << _uploadRequest->uriAnonym() << "] of " - << _storageManager.getSizeAsUploaded() - << " bytes. Invalid or expired access token. The client session is closed."); - } + // No session, or already retried once. + if (session) + { + LOG_ERR("Cannot upload docKey [" + << _docKey << "] to storage URI [" << _uploadRequest->uriAnonym() << "] of " + << _storageManager.getSizeAsUploaded() + << " bytes. Invalid or expired access token. Notifying client and " + "invalidating the authorization token of session [" + << session->getId() << ']'); + session->sendTextFrameAndLogError("error: cmd=storage kind=saveunauthorized"); + session->invalidateAuthorizationToken(); + } + else + { + LOG_ERR("Cannot upload docKey [" + << _docKey << "] to storage URI [" << _uploadRequest->uriAnonym() << "] of " + << _storageManager.getSizeAsUploaded() + << " bytes. Invalid or expired access token. The client session is closed"); + } - broadcastSaveResult(false, "Invalid or expired access token"); + broadcastSaveResult(false, "Invalid or expired access token"); + } } else if (uploadResult.getResult() == StorageBase::UploadResult::Result::FAILED) { @@ -3996,6 +4044,15 @@ std::size_t DocumentBroker::removeSession(const std::shared_ptr& ASSERT_CORRECT_THREAD(); LOG_ASSERT_MSG(session, "Got null ClientSession"); + + // Cancel any pending token refresh for this session. + if (session->isRefreshingToken()) + { + LOG_WRN("Session [" << session->getId() << "] removed while waiting for token refresh"); + session->invalidateAuthorizationToken(); + broadcastSaveResult(false, "Session closed during token refresh"); + } + const std::string id = session->getId(); try { diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp index 72b71edd2bf2b..1f62e8a01b8e3 100644 --- a/wsd/DocumentBroker.hpp +++ b/wsd/DocumentBroker.hpp @@ -335,6 +335,10 @@ class DocumentBroker : public std::enable_shared_from_this /// The current state of uploading must be introspected separately. void checkAndUploadToStorage(const std::shared_ptr& session, bool justSaved); + /// Called when a session receives a fresh access token. + /// If a save is pending retry after 401, retries the upload. + void onTokenRefreshed(const std::shared_ptr& session); + /// Upload the document to Storage if it needs persisting. /// Results are logged and broadcast to users. void uploadToStorage(const std::shared_ptr& session, bool force); From af953d5df20f1df31b01f11bad4d77ff9c6d2b83 Mon Sep 17 00:00:00 2001 From: Ashod Nakashian Date: Mon, 20 Apr 2026 19:48:36 -0400 Subject: [PATCH 12/15] wsd: add new configuration for access_token management This adds default lifetime (when access_token_ttl is missing), a timeout for refreshing tokens that are about to expire, and finally a time offset between the host and us. Change-Id: Id2f6601c19234b34de3c12cf7129b8098f6db439 Signed-off-by: Ashod Nakashian --- common/Authorization.cpp | 43 ++++++++++++++++++++++++++++++---------- common/Authorization.hpp | 4 ++++ common/ConfigUtil.cpp | 2 ++ coolwsd.xml.in | 5 +++++ wsd/ClientSession.cpp | 6 ++++-- wsd/DocumentBroker.cpp | 6 +++++- 6 files changed, 53 insertions(+), 13 deletions(-) diff --git a/common/Authorization.cpp b/common/Authorization.cpp index d4adba8273a2c..57c222544a501 100644 --- a/common/Authorization.cpp +++ b/common/Authorization.cpp @@ -18,6 +18,7 @@ #include "Authorization.hpp" +#include #include #include #include @@ -100,10 +101,31 @@ void Authorization::authorizeRequest(Poco::Net::HTTPRequest& request) const } } +Authorization::duration Authorization::adjustExpiryEpoch(duration rawExpiryEpoch) +{ + if (rawExpiryEpoch == duration::zero()) + { + // No TTL provided; apply default lifetime if configured. + CONFIG_STATIC const int defaultLifetimeMins = + ConfigUtil::getInt("storage.wopi.access_token.default_lifetime_mins", 0); + if (defaultLifetimeMins > 0) + { + const auto expiry = std::chrono::duration_cast( + std::chrono::system_clock::now().time_since_epoch() + + std::chrono::minutes(defaultLifetimeMins)); + LOG_TRC("No access_token_ttl provided, using default lifetime of " + << defaultLifetimeMins << "m, expiry at " << expiry); + return expiry; + } + } + + return rawExpiryEpoch; +} + Authorization Authorization::create(const Poco::URI& uri) { bool noHeader = false; - duration expiryEpoch = duration::zero(); + duration rawExpiryEpoch = duration::zero(); Authorization::Type type = Authorization::Type::None; std::string decoded; for (const auto& param : uri.getQueryParameters()) @@ -124,25 +146,26 @@ Authorization Authorization::create(const Poco::URI& uri) } else if (param.first == "access_token_ttl") { - expiryEpoch = duration(NumUtil::u64FromString(param.second, 0)); + rawExpiryEpoch = duration(NumUtil::u64FromString(param.second, 0)); } } if (!decoded.empty()) { Authorization auth(type, std::move(decoded), noHeader); - if (expiryEpoch > duration::zero()) + if (type == Authorization::Type::Token) { - if (type != Authorization::Type::Token) - { - LOG_WRN("Ignoring invalid access_token_ttl with [" - << name(type) << "] authorization type in uri [" << uri.toString() << ']'); - } - else + const duration adjusted = adjustExpiryEpoch(rawExpiryEpoch); + if (adjusted > duration::zero()) { - auth.setExpiryEpoch(expiryEpoch); + auth.setExpiryEpoch(adjusted); } } + else if (rawExpiryEpoch > duration::zero()) + { + LOG_WRN("Ignoring invalid access_token_ttl with [" + << name(type) << "] authorization type in uri [" << uri.toString() << ']'); + } return auth; } diff --git a/common/Authorization.hpp b/common/Authorization.hpp index 6840d8340def2..313c79e1a0cdf 100644 --- a/common/Authorization.hpp +++ b/common/Authorization.hpp @@ -109,6 +109,10 @@ class Authorization return isRefreshingToken() && (now - _tokenRefreshStartTime) >= _tokenRefreshTimeout; } + /// Apply config adjustments (timezone offset, default lifetime) to a raw TTL value. + /// Returns the adjusted expiry epoch in milliseconds, or zero if no expiry. + static duration adjustExpiryEpoch(duration rawExpiryEpoch); + /// Sets the Token's expiry time from the epoch. void setExpiryEpoch(duration epochMs) { diff --git a/common/ConfigUtil.cpp b/common/ConfigUtil.cpp index e2c9615ce7972..73acdf0b29bd4 100644 --- a/common/ConfigUtil.cpp +++ b/common/ConfigUtil.cpp @@ -279,6 +279,8 @@ static const std::unordered_map DefAppConfig = { { "storage.ssl.cipher_list", "" }, // { "storage.ssl.enable" - deliberately not set; for back-compat { "storage.ssl.key_file_path", "" }, + { "storage.wopi.access_token.default_lifetime_mins", "0" }, + { "storage.wopi.access_token.refresh_timeout_secs", "60" }, { "storage.wopi.alias_groups[@mode]", "first" }, { "storage.wopi.is_legacy_server", "false" }, { "storage.wopi.locking.refresh", "900" }, diff --git a/coolwsd.xml.in b/coolwsd.xml.in index fda6021cef901..64d6ffa388561 100644 --- a/coolwsd.xml.in +++ b/coolwsd.xml.in @@ -308,6 +308,11 @@ false + + + 0 + 60 + true diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp index b0aa41a740be1..1107d30f06c60 100644 --- a/wsd/ClientSession.cpp +++ b/wsd/ClientSession.cpp @@ -1358,9 +1358,11 @@ bool ClientSession::_handleInput(const char *buffer, int length) return false; } - // Get the access_token_ttl, if provided. 0 means unknown/missing. - const auto expiryEpoch = std::chrono::milliseconds( + // Get the access_token_ttl, if provided. 0 means no expiry tracking + // (the legacy single-arg form of the command implied this too). + const auto rawExpiryEpoch = std::chrono::milliseconds( (tokens.size() == 3) ? NumUtil::u64FromString(tokens[2], 0) : 0); + const auto expiryEpoch = Authorization::adjustExpiryEpoch(rawExpiryEpoch); LOG_DBG("Resetting access token for " << getName() << " with expiry at " << expiryEpoch << ": " << tokens[1]); diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp index ef18ed45257ee..c0076d31e34d6 100644 --- a/wsd/DocumentBroker.cpp +++ b/wsd/DocumentBroker.cpp @@ -3336,7 +3336,11 @@ void DocumentBroker::handleUploadToStorageFailed(const StorageBase::UploadResult "Requesting token refresh from session [" << session->getId() << ']'); session->sendTextFrame("tokenexpired"); - session->startTokenRefresh(); + + CONFIG_STATIC const auto refreshTimeout = + ConfigUtil::getConfigValue( + "storage.wopi.access_token.refresh_timeout_secs", 60); + session->startTokenRefresh(refreshTimeout); } else { From e227b7d3c2de55b53d52d50a5062817ed46b9cc8 Mon Sep 17 00:00:00 2001 From: Ashod Nakashian Date: Wed, 15 Apr 2026 07:27:16 -0400 Subject: [PATCH 13/15] wsd: test: access_token_ttl tests Change-Id: I0b213c975fd5265b5ffe1fa12cb5759a9782ff1f Signed-off-by: Ashod Nakashian --- test/UnitWOPI.cpp | 452 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 449 insertions(+), 3 deletions(-) diff --git a/test/UnitWOPI.cpp b/test/UnitWOPI.cpp index 60a0d0afaf62f..11ea330e813ca 100644 --- a/test/UnitWOPI.cpp +++ b/test/UnitWOPI.cpp @@ -26,6 +26,7 @@ #include +#include #include #include #include @@ -334,8 +335,6 @@ class UnitOverload : public WopiTestServer /// unavailable, we are still able to unload. class UnitWopiUnavailable : public WopiTestServer { - using Base = WopiTestServer; - STATE_ENUM(Phase, Load, WaitLoadStatus, Done) _phase; @@ -612,12 +611,459 @@ class UnitWopiHttpHeaders : public WopiTestServer } }; +/// This tests that loading a document with access_token_ttl +/// works correctly and the token expiry is tracked. +class UnitWopiAccessTokenTtl : public WopiTestServer +{ + STATE_ENUM(Phase, Load, WaitLoadStatus, Modify, Save, Done) + _phase; + +public: + UnitWopiAccessTokenTtl() + : WopiTestServer("UnitWopiAccessTokenTtl") + , _phase(Phase::Load) + { + } + + /// The document is loaded. + bool onDocumentLoaded(const std::string& message) override + { + TST_LOG("Got: [" << message << ']'); + LOK_ASSERT_STATE(_phase, Phase::WaitLoadStatus); + + TRANSITION_STATE(_phase, Phase::Modify); + + // Modify the currently opened document; type 'a'. + WSD_CMD("key type=input char=97 key=0"); + WSD_CMD("key type=up char=0 key=512"); + + return true; + } + + bool onDocumentModified(const std::string& message) override + { + TST_LOG("Got: [" << message << ']'); + LOK_ASSERT_STATE(_phase, Phase::Modify); + + TRANSITION_STATE(_phase, Phase::Save); + WSD_CMD("save dontTerminateEdit=0 dontSaveIfUnmodified=0"); + + return true; + } + + std::unique_ptr + assertPutFileRequest(const Poco::Net::HTTPRequest& /*request*/) override + { + LOK_ASSERT_STATE(_phase, Phase::Save); + TRANSITION_STATE(_phase, Phase::Done); + + passTest("PutFile succeeded with access_token_ttl"); + return nullptr; + } + + void invokeWSDTest() override + { + switch (_phase) + { + case Phase::Load: + { + TRANSITION_STATE(_phase, Phase::WaitLoadStatus); + + // TTL 15 minutes from now, in milliseconds (as per WOPI spec). + const uint64_t expiryTimeMs = + static_cast(time(nullptr) + (15 * 60)) * 1000; + initWebsocket("/wopi/files/0?access_token=secret&access_token_ttl=" + + std::to_string(expiryTimeMs)); + + WSD_CMD("load url=" + getWopiSrc()); + break; + } + case Phase::WaitLoadStatus: + case Phase::Modify: + case Phase::Save: + case Phase::Done: + { + // just wait for the results + break; + } + } + } +}; + +/// Test the save -> 401 -> tokenexpired -> resetaccesstoken -> retry -> success flow. +class UnitWopiTokenRefreshOnSave : public WopiTestServer +{ + STATE_ENUM(Phase, Load, WaitLoadStatus, Modify, WaitSave, WaitTokenExpired, Done) + _phase; + + int _putFileCount; + + static constexpr auto OriginalToken = "original_token_123"; + static constexpr auto RefreshedToken = "refreshed_token_456"; + +public: + UnitWopiTokenRefreshOnSave() + : WopiTestServer("UnitWopiTokenRefreshOnSave") + , _phase(Phase::Load) + , _putFileCount(0) + { + } + + bool onDocumentLoaded(const std::string& message) override + { + TST_LOG("Got: [" << message << ']'); + LOK_ASSERT_STATE(_phase, Phase::WaitLoadStatus); + + TRANSITION_STATE(_phase, Phase::Modify); + + // Modify the document; type 'a'. + WSD_CMD("key type=input char=97 key=0"); + WSD_CMD("key type=up char=0 key=512"); + + return true; + } + + bool onDocumentModified(const std::string& message) override + { + TST_LOG("Got: [" << message << ']'); + LOK_ASSERT_STATE(_phase, Phase::Modify); + + TRANSITION_STATE(_phase, Phase::WaitSave); + WSD_CMD("save dontTerminateEdit=0 dontSaveIfUnmodified=0"); + + return true; + } + + std::unique_ptr + assertPutFileRequest(const Poco::Net::HTTPRequest& request) override + { + ++_putFileCount; + TST_LOG("PutFile #" << _putFileCount << " URI: " << request.getURI()); + + if (_putFileCount == 1) + { + // First PutFile: verify original token and return 401. + for (const auto& param : Poco::URI(request.getURI()).getQueryParameters()) + { + if (param.first == "access_token") + { + LOK_ASSERT_EQUAL_STR(std::string(OriginalToken), param.second); + break; + } + } + + TST_LOG("Returning 401 to trigger token refresh"); + return std::make_unique(http::StatusCode::Unauthorized); + } + + // Second PutFile: verify the refreshed token is used. + LOK_ASSERT_EQUAL(2, _putFileCount); + for (const auto& param : Poco::URI(request.getURI()).getQueryParameters()) + { + if (param.first == "access_token") + { + LOK_ASSERT_EQUAL_STR(std::string(RefreshedToken), param.second); + break; + } + } + + TRANSITION_STATE(_phase, Phase::Done); + passTest("PutFile retry with refreshed token succeeded"); + return nullptr; + } + + bool onFilterSendWebSocketMessage(const std::string_view message, const WSOpCode /* code */, + const bool /* flush */, int& /*unitReturn*/) override + { + if (message == "tokenexpired") + { + TST_LOG("Got tokenexpired, sending resetaccesstoken with new token and TTL"); + LOK_ASSERT_STATE(_phase, Phase::WaitSave); + TRANSITION_STATE(_phase, Phase::WaitTokenExpired); + + // TTL 30 minutes from now, in milliseconds. + const uint64_t expiryMs = + static_cast(time(nullptr) + 30 * 60) * 1000; + WSD_CMD("resetaccesstoken " + std::string(RefreshedToken) + ' ' + + std::to_string(expiryMs)); + } + + return false; + } + + void invokeWSDTest() override + { + switch (_phase) + { + case Phase::Load: + { + TRANSITION_STATE(_phase, Phase::WaitLoadStatus); + + initWebsocket("/wopi/files/0?access_token=" + std::string(OriginalToken) + + "&access_token_ttl=0"); + + WSD_CMD("load url=" + getWopiSrc()); + break; + } + case Phase::WaitLoadStatus: + case Phase::Modify: + case Phase::WaitSave: + case Phase::WaitTokenExpired: + case Phase::Done: + { + // just wait for the results + break; + } + } + } +}; + +/// Test the save -> 401 -> tokenexpired -> no refresh -> timeout -> saveunauthorized flow. +/// We return 401 on the only PutFile attempt, send tokenexpired, and never reply with +/// resetaccesstoken. The auth state stays in TokenRefresh (so isValid() is false and +/// no further upload is attempted), and the poll loop's isTokenRefreshTimedOut() check +/// fires after refresh_timeout_secs to broadcast saveunauthorized. +class UnitWopiTokenRefreshTimeout : public WopiTestServer +{ + STATE_ENUM(Phase, Load, WaitLoadStatus, Modify, WaitSave, WaitTimeout, Done) + _phase; + int _defLifetimeMins; + int _refreshTimeoutSecs; + bool _gotTokenExpired; + +public: + UnitWopiTokenRefreshTimeout(int defLifetimeMins, int refreshTimeoutSecs) + : WopiTestServer("UnitWopiTokenRefreshTimeout") + , _phase(Phase::Load) + , _defLifetimeMins(defLifetimeMins) + , _refreshTimeoutSecs(refreshTimeoutSecs) + , _gotTokenExpired(false) + { + } + + void configure(Poco::Util::LayeredConfiguration& config) override + { + WopiTestServer::configure(config); + + config.setUInt("storage.wopi.access_token.default_lifetime_mins", _defLifetimeMins); + config.setUInt("storage.wopi.access_token.refresh_timeout_secs", _refreshTimeoutSecs); + } + + bool onDocumentLoaded(const std::string& message) override + { + TST_LOG("Got: [" << message << ']'); + LOK_ASSERT_STATE(_phase, Phase::WaitLoadStatus); + + TRANSITION_STATE(_phase, Phase::Modify); + + // Modify the document; type 'a'. + WSD_CMD("key type=input char=97 key=0"); + WSD_CMD("key type=up char=0 key=512"); + + return true; + } + + bool onDocumentModified(const std::string& message) override + { + TST_LOG("Got: [" << message << ']'); + LOK_ASSERT_STATE(_phase, Phase::Modify); + + TRANSITION_STATE(_phase, Phase::WaitSave); + WSD_CMD("save dontTerminateEdit=0 dontSaveIfUnmodified=0"); + + return true; + } + + std::unique_ptr + assertPutFileRequest(const Poco::Net::HTTPRequest& /*request*/) override + { + // Return 401. handleUploadToStorageFailed sends tokenexpired and switches + // the auth to Type::TokenRefresh, which makes isValid() false -- no further + // PutFile is attempted while we wait for the host. Since the host never + // replies with resetaccesstoken, the poll loop eventually fires + // isTokenRefreshTimedOut() and broadcasts saveunauthorized. + TST_LOG("Returning 401 (no token refresh will arrive)"); + return std::make_unique(http::StatusCode::Unauthorized); + } + + bool onFilterSendWebSocketMessage(const std::string_view message, const WSOpCode /* code */, + const bool /* flush */, int& /*unitReturn*/) override + { + if (message == "tokenexpired") + { + TST_LOG("Got tokenexpired, deliberately NOT sending resetaccesstoken"); + LOK_ASSERT_STATE(_phase, Phase::WaitSave); + TRANSITION_STATE(_phase, Phase::WaitTimeout); + _gotTokenExpired = true; + // Let the refresh-request time out. + } + + return false; + } + + bool onDocumentError(const std::string& message) override + { + TST_LOG("Got error: [" << message << ']'); + + if (message.find("kind=saveunauthorized") != std::string::npos) + { + LOK_ASSERT_STATE(_phase, Phase::WaitTimeout); + LOK_ASSERT_MESSAGE("tokenexpired should have been sent before timeout", _gotTokenExpired); + + TRANSITION_STATE(_phase, Phase::Done); + passTest("Token refresh timeout correctly produced saveunauthorized"); + return true; + } + + return false; + } + + void invokeWSDTest() override + { + switch (_phase) + { + case Phase::Load: + { + TRANSITION_STATE(_phase, Phase::WaitLoadStatus); + + initWebsocket("/wopi/files/0?access_token=anything&access_token_ttl=0"); + + WSD_CMD("load url=" + getWopiSrc()); + break; + } + case Phase::WaitLoadStatus: + case Phase::Modify: + case Phase::WaitSave: + case Phase::WaitTimeout: + case Phase::Done: + { + // just wait for the results + break; + } + } + } +}; + +/// Verify the unauthorized (expired token) retry cap. +/// PutFile always returns 401. We answer every tokenexpired with a fresh +/// resetaccesstoken. After MaxTokenRefreshAttempts (3) cycles wsd must stop +/// asking for refreshes and broadcast saveunauthorized instead of sending a +/// 4th tokenexpired. +class UnitWopiTokenRefreshCap : public WopiTestServer +{ + STATE_ENUM(Phase, Load, WaitLoadStatus, Modify, WaitSave, Done) _phase; + + static constexpr int MaxAttempts = 3; // Must match DocumentBroker MaxTokenRefreshAttempts. + + int _tokenExpiredCount; + bool _gotSaveUnauthorized; + +public: + UnitWopiTokenRefreshCap() + : WopiTestServer("UnitWopiTokenRefreshCap") + , _phase(Phase::Load) + , _tokenExpiredCount(0) + , _gotSaveUnauthorized(false) + { + } + + bool onDocumentLoaded(const std::string& message) override + { + TST_LOG("Got: [" << message << ']'); + LOK_ASSERT_STATE(_phase, Phase::WaitLoadStatus); + + TRANSITION_STATE(_phase, Phase::Modify); + WSD_CMD("key type=input char=97 key=0"); + WSD_CMD("key type=up char=0 key=512"); + return true; + } + + bool onDocumentModified(const std::string& message) override + { + TST_LOG("Got: [" << message << ']'); + LOK_ASSERT_STATE(_phase, Phase::Modify); + + TRANSITION_STATE(_phase, Phase::WaitSave); + WSD_CMD("save dontTerminateEdit=0 dontSaveIfUnmodified=0"); + return true; + } + + std::unique_ptr + assertPutFileRequest(const Poco::Net::HTTPRequest& /*request*/) override + { + TST_LOG("Returning 401 to keep triggering tokenexpired"); + return std::make_unique(http::StatusCode::Unauthorized); + } + + bool onFilterSendWebSocketMessage(const std::string_view message, const WSOpCode /* code */, + const bool /* flush */, int& /*unitReturn*/) override + { + if (message == "tokenexpired") + { + ++_tokenExpiredCount; + TST_LOG("Got tokenexpired #" << _tokenExpiredCount); + + // The cap must stop us at exactly kMaxAttempts. A 4th tokenexpired + // means the cap didn't engage. + LOK_ASSERT_MESSAGE("tokenexpired exceeded MaxTokenRefreshAttempts", + _tokenExpiredCount <= MaxAttempts); + + // Reply with a fresh resetaccesstoken so the auth flips back to Token + // and the next save attempt would re-enter the refresh branch -- if + // not for the cap. + WSD_CMD("resetaccesstoken refreshed_" + std::to_string(_tokenExpiredCount) + " 0"); + } + + return false; + } + + bool onDocumentError(const std::string& message) override + { + TST_LOG("Got error: [" << message << ']'); + + if (message.find("kind=saveunauthorized") != std::string::npos) + { + LOK_ASSERT_EQUAL(MaxAttempts, _tokenExpiredCount); + _gotSaveUnauthorized = true; + TRANSITION_STATE(_phase, Phase::Done); + passTest("Retry cap correctly produced saveunauthorized after " + + std::to_string(MaxAttempts) + " refreshes"); + return true; + } + + return false; + } + + void invokeWSDTest() override + { + switch (_phase) + { + case Phase::Load: + { + TRANSITION_STATE(_phase, Phase::WaitLoadStatus); + initWebsocket("/wopi/files/0?access_token=anything&access_token_ttl=0"); + WSD_CMD("load url=" + getWopiSrc()); + break; + } + case Phase::WaitLoadStatus: + case Phase::Modify: + case Phase::WaitSave: + case Phase::Done: + { + break; + } + } + } +}; + UnitBase** unit_create_wsd_multi(void) { return new UnitBase* [] { new UnitWOPI(), new UnitWOPILoadEncoded(), - /*new UnitOverload(),*/ new UnitWopiUnavailable(), new UnitWopiHttpHeaders(), nullptr + /*new UnitOverload(),*/ new UnitWopiUnavailable(), new UnitWopiHttpHeaders(), + new UnitWopiAccessTokenTtl(), new UnitWopiTokenRefreshOnSave(), + new UnitWopiTokenRefreshTimeout(0, 5), new UnitWopiTokenRefreshCap(), nullptr }; } From b6845b4f4ff80af72ffb7a29316c6d50e58c3e32 Mon Sep 17 00:00:00 2001 From: Ashod Nakashian Date: Sat, 25 Apr 2026 14:50:02 -0400 Subject: [PATCH 14/15] browser: cap the access_token expiry warning We give up warning of the access_token expiry roughly 5 minutes after it expires. Change-Id: I06c1b252f291985a4fa0d0090a55c00a415b53a6 Signed-off-by: Ashod Nakashian --- browser/src/app/Socket.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/browser/src/app/Socket.ts b/browser/src/app/Socket.ts index c580c93ada5ef..44bbffcbac44d 100644 --- a/browser/src/app/Socket.ts +++ b/browser/src/app/Socket.ts @@ -41,6 +41,7 @@ class Socket { private _inLayerTransaction: boolean; private _slurpDuringTransaction: boolean; private _accessTokenExpireTimeout: TimeoutHdl | undefined; + private _accessTokenExpireWarningCount: number = 0; private _reconnecting: boolean; private _slurpTimer: TimeoutHdl | undefined; private _renderEventTimer: TimeoutHdl | undefined; @@ -304,6 +305,7 @@ class Socket { public resetTokenExpiryTimer(): void { clearTimeout(this._accessTokenExpireTimeout); // Always clear the old timer. + this._accessTokenExpireWarningCount = 0; const ttl = parseInt( this._map.options.docParams.access_token_ttl as string, ); @@ -667,11 +669,16 @@ class Socket { }, }); - // If user still doesn't refresh the session, warn again periodically - this._accessTokenExpireTimeout = setTimeout( - this._sessionExpiredWarning.bind(this), - 120 * 1000, - ); + // If user still doesn't refresh the session, warn again periodically. + // Cap at 10 retries (~20 minutes, i.e. ~5 minutes after the access_token + // expires) so we don't spam the host indefinitely. + this._accessTokenExpireWarningCount++; + if (this._accessTokenExpireWarningCount < 10) { + this._accessTokenExpireTimeout = setTimeout( + this._sessionExpiredWarning.bind(this), + 120 * 1000, + ); + } } public setUnloading(): void { From 573f2e74f122833f90733ae2f3bbdc27e3b3e380 Mon Sep 17 00:00:00 2001 From: Ashod Nakashian Date: Sat, 25 Apr 2026 16:52:53 -0400 Subject: [PATCH 15/15] wsd: cap the resetaccesstoken retry count We retry resetaccesstoken up to 3 times before we give up and invalidate the access_token. Change-Id: I0a5fa9cad90bac9b3a397024b5a5e15f59f4a418 Signed-off-by: Ashod Nakashian --- wsd/ClientSession.cpp | 1 + wsd/ClientSession.hpp | 14 +++++++++++++- wsd/DocumentBroker.cpp | 20 ++++++++++++++++---- 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp index 1107d30f06c60..753d0b6899fb5 100644 --- a/wsd/ClientSession.cpp +++ b/wsd/ClientSession.cpp @@ -99,6 +99,7 @@ ClientSession::ClientSession(const std::shared_ptr& ws , _additionalFileUrisPublic(additionalFileUrisPublic) , _serverURL(requestDetails) , _auth(Authorization::create(uriPublic)) + , _tokenRefreshAttempts(0) , _docBroker(docBroker) , _lastStateTime(std::chrono::steady_clock::now()) , _clientVisibleArea(0, 0, 0, 0) diff --git a/wsd/ClientSession.hpp b/wsd/ClientSession.hpp index 7b7c3482575cb..60bcf6e2ad16d 100644 --- a/wsd/ClientSession.hpp +++ b/wsd/ClientSession.hpp @@ -202,10 +202,18 @@ class ClientSession final : public Session /// Start waiting for a token refresh from the host. void startTokenRefresh(const std::chrono::seconds timeout) { - LOG_DBG("Session [" << getId() << "] starting token refresh wait"); + LOG_DBG("Session [" << getId() << "] starting token refresh wait (attempt #" + << (_tokenRefreshAttempts + 1) << ')'); + ++_tokenRefreshAttempts; _auth.startTokenRefresh(timeout); } + /// Returns the number of consecutive refresh attempts since the last successful upload. + int tokenRefreshAttempts() const { return _tokenRefreshAttempts; } + + /// Reset the consecutive token-refresh attempt counter. Call on successful upload. + void resetTokenRefreshAttempts() { _tokenRefreshAttempts = 0; } + /// Returns true iff this session is waiting for a token refresh. bool isRefreshingToken() const { return _auth.isRefreshingToken(); } @@ -464,6 +472,10 @@ class ClientSession final : public Session /// Authorization data - either access_token or access_header. Authorization _auth; + /// Number of consecutive token-refresh attempts triggered by upload 401s + /// since the last successful upload. Bounds the refresh-on-unauthorize retry loop. + int _tokenRefreshAttempts; + /// Rotating clipboard remote access identifiers - protected by GlobalSessionMapMutex std::string _clipboardKeys[2]; diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp index c0076d31e34d6..c047e99fd105a 100644 --- a/wsd/DocumentBroker.cpp +++ b/wsd/DocumentBroker.cpp @@ -3044,6 +3044,11 @@ void DocumentBroker::handleUploadToStorageSuccessful(const StorageBase::UploadRe assert(_uploadRequest && "Expected to have a valid UploadRequest instance"); LOG_DBG("Last upload result: OK"); + if (const auto session = _uploadRequest->session()) + { + session->resetTokenRefreshAttempts(); + } + #if !MOBILEAPP WopiStorage* wopiStorage = dynamic_cast(_storage.get()); if (wopiStorage != nullptr) @@ -3327,14 +3332,20 @@ void DocumentBroker::handleUploadToStorageFailed(const StorageBase::UploadResult { LOG_DBG("Last upload result: UNAUTHORIZED"); const auto session = _uploadRequest->session(); - if (session && !session->isRefreshingToken()) + + // Bound the refresh-on-unauthorized retry loop: a successful resetaccesstoken + // flips state back to Token, so isRefreshingToken() alone can't gate further attempts. + constexpr int MaxTokenRefreshAttempts = 3; + if (session && !session->isRefreshingToken() && + session->tokenRefreshAttempts() < MaxTokenRefreshAttempts) { - // First attempt: ask the host for a fresh token before giving up. + // Ask the host for a fresh token before giving up. LOG_WRN("Cannot upload docKey [" << _docKey << "] to storage URI [" << _uploadRequest->uriAnonym() << "]. Invalid or expired access token. " "Requesting token refresh from session [" - << session->getId() << ']'); + << session->getId() << "] (attempt " << (session->tokenRefreshAttempts() + 1) + << " of " << MaxTokenRefreshAttempts << ')'); session->sendTextFrame("tokenexpired"); CONFIG_STATIC const auto refreshTimeout = @@ -3344,7 +3355,8 @@ void DocumentBroker::handleUploadToStorageFailed(const StorageBase::UploadResult } else { - // No session, or already retried once. + // No session, mid-refresh (refresh arrived but upload failed again), + // or hit the retry cap. if (session) { LOG_ERR("Cannot upload docKey ["