ucb/source/ucp/cmis/cmis_content.cxx | 263 ++++++++++++++++-------------- ucb/source/ucp/cmis/cmis_content.hxx | 7 ucb/source/ucp/cmis/cmis_repo_content.cxx | 112 ------------ ucb/source/ucp/cmis/cmis_repo_content.hxx | 2 4 files changed, 152 insertions(+), 232 deletions(-)
New commits: commit 59a1b0d5a368656ce4f0c283ede4b9bf6694a8d7 Author: Mike Kaganski <[email protected]> AuthorDate: Sun Oct 26 18:48:21 2025 +0500 Commit: Mike Kaganski <[email protected]> CommitDate: Sun Oct 26 17:52:37 2025 +0100 tdf#101630: Do not prompt twice for OAuth2 authentication The problem was: When adding a service for Google Drive, clicking OK in the File Services dialog would initiate the OAuth2 authentication procedure; after successful confirmation, immediately another authentication procedure started. The reason was, that the first procedure was initiated by the code in RepoContent::getRepositories, which created a session, but did not register it, nor saved its token. Then, in Content::getSession, another session had to be created, which had to authenticate again. Before this change, the registration for an OAuth2-authenticated session used a key including the repo. Therefore, RepoContent did not know which repo it had to use for registration. This change unifies the previously duplicated code used to create a session. For OAuth2-based sessions, the key for registration is now only the service's binding URL, which allows to use the same key both in RepoContent::getRepositories and Content::getSession. Now the successfully created session in getRepositories is not deleted, allowing it to be registered and reused. Also, after the OAuth2 authentication failed (which means that it was cancelled by the user), the password dialog is not shown. Change-Id: Ic84a8e034eb0900aca3b4cac0d73dd7443d6380d Reviewed-on: https://gerrit.libreoffice.org/c/core/+/193002 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> diff --git a/ucb/source/ucp/cmis/cmis_content.cxx b/ucb/source/ucp/cmis/cmis_content.cxx index 6548692b326a..80ba5af2e66a 100644 --- a/ucb/source/ucp/cmis/cmis_content.cxx +++ b/ucb/source/ucp/cmis/cmis_content.cxx @@ -256,6 +256,16 @@ namespace return aArguments; } + +bool isOAuth2(const cmis::URL& url) +{ + return url.getBindingUrl() == ONEDRIVE_BASE_URL || url.getBindingUrl() == GDRIVE_BASE_URL; +} + +OUString getSessionId(const cmis::URL& url) +{ + return isOAuth2(url) ? url.getBindingUrl() : url.getBindingUrl() + url.getRepositoryId(); +} } namespace cmis @@ -309,154 +319,165 @@ namespace cmis libcmis::SessionFactory::setProxySettings( OUSTR_TO_STDSTR( sProxy ), std::string(), std::string(), std::string() ); // Look for a cached session, key is binding url + repo id - OUString sSessionId = m_aURL.getBindingUrl( ) + m_aURL.getRepositoryId( ); if ( nullptr == m_pSession ) - m_pSession = m_pProvider->getSession( sSessionId, m_aURL.getUsername( ) ); + m_pSession = m_pProvider->getSession(getSessionId(m_aURL), m_aURL.getUsername()); if ( nullptr == m_pSession ) { - // init libcurl callback - libcmis::SessionFactory::setCurlInitProtocolsFunction(&::InitCurl_easy); + m_pSession = createSession(xEnv, *m_pProvider, m_aURL, + m_xIdentifier->getContentIdentifier(), this); + } - // Get the auth credentials - AuthProvider aAuthProvider(xEnv, m_xIdentifier->getContentIdentifier(), m_aURL.getBindingUrl()); - AuthProvider::setXEnv( xEnv ); + return m_pSession; + } - auto rUsername = OUSTR_TO_STDSTR( m_aURL.getUsername( ) ); - auto rPassword = OUSTR_TO_STDSTR( m_aURL.getPassword( ) ); + libcmis::Session* createSession(const css::uno::Reference<css::ucb::XCommandEnvironment>& xEnv, + ContentProvider& rProvider, const URL& rURL, + const OUString& rContentId, + const css::uno::Reference<css::ucb::XContent>& xContext) + { + // init libcurl callback + libcmis::SessionFactory::setCurlInitProtocolsFunction(&::InitCurl_easy); - bool bSkipInitialPWAuth = false; - if (m_aURL.getBindingUrl() == ONEDRIVE_BASE_URL - || m_aURL.getBindingUrl() == GDRIVE_BASE_URL) - { - // skip the initial username and pw-auth prompt, the only supported method is the - // auth-code-fallback one (login with your browser, copy code into the dialog) - // TODO: if LO were to listen on localhost for the request, it would be much nicer - // user experience - bSkipInitialPWAuth = true; - rPassword = aAuthProvider.getRefreshToken(rUsername); - } + // Get the auth credentials + AuthProvider aAuthProvider(xEnv, rContentId, rURL.getBindingUrl()); + AuthProvider::setXEnv( xEnv ); - bool bIsDone = false; + auto rUsername = OUSTR_TO_STDSTR( rURL.getUsername( ) ); + auto rPassword = OUSTR_TO_STDSTR( rURL.getPassword( ) ); + + bool bSkipInitialPWAuth = false; + if (isOAuth2(rURL)) + { + // skip the initial username and pw-auth prompt + bSkipInitialPWAuth = true; + rPassword = aAuthProvider.getRefreshToken(rUsername); + } - while ( !bIsDone ) + while (true) + { + if (bSkipInitialPWAuth || aAuthProvider.authenticationQuery(rUsername, rPassword)) { - if (bSkipInitialPWAuth || aAuthProvider.authenticationQuery(rUsername, rPassword)) + // Initiate a CMIS session and register it as we found nothing + libcmis::OAuth2DataPtr oauth2Data; + if ( rURL.getBindingUrl( ) == GDRIVE_BASE_URL ) + { + // reset the skip, so user gets a chance to cancel + bSkipInitialPWAuth = false; + libcmis::SessionFactory::setOAuth2AuthCodeProvider(AuthProvider::copyWebAuthCodeFallback); + oauth2Data = boost::make_shared<libcmis::OAuth2Data>( + GDRIVE_AUTH_URL, GDRIVE_TOKEN_URL, + GDRIVE_SCOPE, GDRIVE_REDIRECT_URI, + GDRIVE_CLIENT_ID, GDRIVE_CLIENT_SECRET ); + } + if ( rURL.getBindingUrl().startsWith( ALFRESCO_CLOUD_BASE_URL ) ) + oauth2Data = boost::make_shared<libcmis::OAuth2Data>( + ALFRESCO_CLOUD_AUTH_URL, ALFRESCO_CLOUD_TOKEN_URL, + ALFRESCO_CLOUD_SCOPE, ALFRESCO_CLOUD_REDIRECT_URI, + ALFRESCO_CLOUD_CLIENT_ID, ALFRESCO_CLOUD_CLIENT_SECRET ); + if ( rURL.getBindingUrl( ) == ONEDRIVE_BASE_URL ) + { + // reset the skip, so user gets a chance to cancel + bSkipInitialPWAuth = false; + libcmis::SessionFactory::setOAuth2AuthCodeProvider(AuthProvider::copyWebAuthCodeFallback); + oauth2Data = boost::make_shared<libcmis::OAuth2Data>( + ONEDRIVE_AUTH_URL, ONEDRIVE_TOKEN_URL, + ONEDRIVE_SCOPE, ONEDRIVE_REDIRECT_URI, + ONEDRIVE_CLIENT_ID, ONEDRIVE_CLIENT_SECRET ); + } + try { - // Initiate a CMIS session and register it as we found nothing - libcmis::OAuth2DataPtr oauth2Data; - if ( m_aURL.getBindingUrl( ) == GDRIVE_BASE_URL ) + std::unique_ptr<libcmis::Session> pSession(libcmis::SessionFactory::createSession( + OUSTR_TO_STDSTR( rURL.getBindingUrl( ) ), + rUsername, rPassword, OUSTR_TO_STDSTR( rURL.getRepositoryId( ) ), false, std::move(oauth2Data) )); + + if ( pSession == nullptr ) { - // reset the skip, so user gets a chance to cancel - bSkipInitialPWAuth = false; - libcmis::SessionFactory::setOAuth2AuthCodeProvider(AuthProvider::copyWebAuthCodeFallback); - oauth2Data = boost::make_shared<libcmis::OAuth2Data>( - GDRIVE_AUTH_URL, GDRIVE_TOKEN_URL, - GDRIVE_SCOPE, GDRIVE_REDIRECT_URI, - GDRIVE_CLIENT_ID, GDRIVE_CLIENT_SECRET ); + // Fail: session was not created + ucbhelper::cancelCommandExecution( + ucb::IOErrorCode_INVALID_DEVICE, + generateErrorArguments(rURL), + xEnv); } - if ( m_aURL.getBindingUrl().startsWith( ALFRESCO_CLOUD_BASE_URL ) ) - oauth2Data = boost::make_shared<libcmis::OAuth2Data>( - ALFRESCO_CLOUD_AUTH_URL, ALFRESCO_CLOUD_TOKEN_URL, - ALFRESCO_CLOUD_SCOPE, ALFRESCO_CLOUD_REDIRECT_URI, - ALFRESCO_CLOUD_CLIENT_ID, ALFRESCO_CLOUD_CLIENT_SECRET ); - if ( m_aURL.getBindingUrl( ) == ONEDRIVE_BASE_URL ) + else if (pSession->getRepository() == nullptr + && xContext->getContentType() != CMIS_REPO_TYPE) { - // reset the skip, so user gets a chance to cancel - bSkipInitialPWAuth = false; - libcmis::SessionFactory::setOAuth2AuthCodeProvider(AuthProvider::copyWebAuthCodeFallback); - oauth2Data = boost::make_shared<libcmis::OAuth2Data>( - ONEDRIVE_AUTH_URL, ONEDRIVE_TOKEN_URL, - ONEDRIVE_SCOPE, ONEDRIVE_REDIRECT_URI, - ONEDRIVE_CLIENT_ID, ONEDRIVE_CLIENT_SECRET ); + // Fail: no repository or repository is invalid + ucbhelper::cancelCommandExecution( + ucb::IOErrorCode_INVALID_DEVICE, + generateErrorArguments(rURL), + xEnv, + u"error accessing a repository"_ustr); } - try + else { - m_pSession = libcmis::SessionFactory::createSession( - OUSTR_TO_STDSTR( m_aURL.getBindingUrl( ) ), - rUsername, rPassword, OUSTR_TO_STDSTR( m_aURL.getRepositoryId( ) ), false, std::move(oauth2Data) ); - - if ( m_pSession == nullptr ) - { - // Fail: session was not created - ucbhelper::cancelCommandExecution( - ucb::IOErrorCode_INVALID_DEVICE, - generateErrorArguments(m_aURL), - xEnv); - } - else if ( m_pSession->getRepository() == nullptr ) - { - // Fail: no repository or repository is invalid - ucbhelper::cancelCommandExecution( - ucb::IOErrorCode_INVALID_DEVICE, - generateErrorArguments(m_aURL), - xEnv, - u"error accessing a repository"_ustr); - } - else + rProvider.registerSession(getSessionId(rURL), rURL.getUsername(), pSession.get()); + if (isOAuth2(rURL)) { - m_pProvider->registerSession(sSessionId, m_aURL.getUsername( ), m_pSession); - if (m_aURL.getBindingUrl() == ONEDRIVE_BASE_URL - || m_aURL.getBindingUrl() == GDRIVE_BASE_URL) - { - aAuthProvider.storeRefreshToken(rUsername, rPassword, - m_pSession->getRefreshToken()); - } + aAuthProvider.storeRefreshToken(rUsername, rPassword, + pSession->getRefreshToken()); } + } - bIsDone = true; + return pSession.release(); + } + catch( const libcmis::Exception & e ) + { + if (e.getType() == "dnsFailed") + { + uno::Any ex; + ex <<= ucb::InteractiveNetworkResolveNameException( + OStringToOUString(e.what(), RTL_TEXTENCODING_UTF8), + xContext, + task::InteractionClassification_ERROR, + rURL.getBindingUrl()); + ucbhelper::cancelCommandExecution(ex, xEnv); } - catch( const libcmis::Exception & e ) + else if (e.getType() == "connectFailed" || e.getType() == "connectTimeout") { - if (e.getType() == "dnsFailed") - { - uno::Any ex; - ex <<= ucb::InteractiveNetworkResolveNameException( - OStringToOUString(e.what(), RTL_TEXTENCODING_UTF8), - getXWeak(), - task::InteractionClassification_ERROR, - m_aURL.getBindingUrl()); - ucbhelper::cancelCommandExecution(ex, xEnv); - } - else if (e.getType() == "connectFailed" || e.getType() == "connectTimeout") - { - uno::Any ex; - ex <<= ucb::InteractiveNetworkConnectException( - OStringToOUString(e.what(), RTL_TEXTENCODING_UTF8), - getXWeak(), - task::InteractionClassification_ERROR, - m_aURL.getBindingUrl()); - ucbhelper::cancelCommandExecution(ex, xEnv); - } - else if (e.getType() == "transferFailed") - { - uno::Any ex; - ex <<= ucb::InteractiveNetworkReadException( - OStringToOUString(e.what(), RTL_TEXTENCODING_UTF8), - getXWeak(), - task::InteractionClassification_ERROR, - m_aURL.getBindingUrl()); - ucbhelper::cancelCommandExecution(ex, xEnv); - } - else if (e.getType() != "permissionDenied") + uno::Any ex; + ex <<= ucb::InteractiveNetworkConnectException( + OStringToOUString(e.what(), RTL_TEXTENCODING_UTF8), + xContext, + task::InteractionClassification_ERROR, + rURL.getBindingUrl()); + ucbhelper::cancelCommandExecution(ex, xEnv); + } + else if (e.getType() == "transferFailed") + { + uno::Any ex; + ex <<= ucb::InteractiveNetworkReadException( + OStringToOUString(e.what(), RTL_TEXTENCODING_UTF8), + xContext, + task::InteractionClassification_ERROR, + rURL.getBindingUrl()); + ucbhelper::cancelCommandExecution(ex, xEnv); + } + else if (e.getType() == "permissionDenied") + { + if (isOAuth2(rURL)) { - SAL_INFO("ucb.ucp.cmis", "Unexpected libcmis exception: " << e.what()); - throw; + // For OAuth2, this means user cancelled - don't try again + ucbhelper::cancelCommandExecution(ucb::IOErrorCode_ABORT, {}, xEnv); } } - } - else - { - // Silently fail as the user cancelled the authentication - ucbhelper::cancelCommandExecution( - ucb::IOErrorCode_ABORT, - uno::Sequence< uno::Any >( 0 ), - xEnv ); - throw uno::RuntimeException( ); + else + { + SAL_INFO("ucb.ucp.cmis", "Unexpected libcmis exception: " << e.what()); + throw; + } } } + else + { + // Silently fail as the user cancelled the authentication + ucbhelper::cancelCommandExecution( + ucb::IOErrorCode_ABORT, + uno::Sequence< uno::Any >( 0 ), + xEnv ); + throw uno::RuntimeException( ); + } } - return m_pSession; } libcmis::ObjectTypePtr const & Content::getObjectType( const uno::Reference< ucb::XCommandEnvironment >& xEnv ) diff --git a/ucb/source/ucp/cmis/cmis_content.hxx b/ucb/source/ucp/cmis/cmis_content.hxx index 7c2938c65b64..1dd1a4e08acc 100644 --- a/ucb/source/ucp/cmis/cmis_content.hxx +++ b/ucb/source/ucp/cmis/cmis_content.hxx @@ -54,11 +54,16 @@ namespace ucbhelper namespace cmis { - +inline constexpr OUString CMIS_REPO_TYPE = u"application/vnd.libreoffice.cmis-repository"_ustr; inline constexpr OUString CMIS_FILE_TYPE = u"application/vnd.libreoffice.cmis-file"_ustr; inline constexpr OUString CMIS_FOLDER_TYPE = u"application/vnd.libreoffice.cmis-folder"_ustr; class ContentProvider; +libcmis::Session* createSession(const css::uno::Reference<css::ucb::XCommandEnvironment>& xEnv, + ContentProvider& rProvider, const URL& rURL, + const OUString& rContentId, + const css::uno::Reference<css::ucb::XContent>& xContext); + class Content : public ::ucbhelper::ContentImplHelper, public css::ucb::XContentCreator, public ChildrenProvider diff --git a/ucb/source/ucp/cmis/cmis_repo_content.cxx b/ucb/source/ucp/cmis/cmis_repo_content.cxx index 647929af66d8..99d8c109d588 100644 --- a/ucb/source/ucp/cmis/cmis_repo_content.cxx +++ b/ucb/source/ucp/cmis/cmis_repo_content.cxx @@ -24,7 +24,6 @@ #include <config_oauth2.h> #include <rtl/uri.hxx> #include <sal/log.hxx> -#include <systools/curlinit.hxx> #include <tools/urlobj.hxx> #include <ucbhelper/cancelcommandexecution.hxx> #include <ucbhelper/contentidentifier.hxx> @@ -127,113 +126,10 @@ namespace cmis if ( !m_aRepositories.empty() ) return; - // init libcurl callback - libcmis::SessionFactory::setCurlInitProtocolsFunction(&::InitCurl_easy); - - // Get the auth credentials - AuthProvider authProvider( xEnv, m_xIdentifier->getContentIdentifier( ), m_aURL.getBindingUrl( ) ); - AuthProvider::setXEnv( xEnv ); - - std::string rUsername = OUSTR_TO_STDSTR( m_aURL.getUsername( ) ); - std::string rPassword = OUSTR_TO_STDSTR( m_aURL.getPassword( ) ); - - bool bIsDone = false; - - while( !bIsDone ) - { - if ( authProvider.authenticationQuery( rUsername, rPassword ) ) - { - try - { - // Create a session to get repositories - libcmis::OAuth2DataPtr oauth2Data; - if ( m_aURL.getBindingUrl( ) == GDRIVE_BASE_URL ) - { - libcmis::SessionFactory::setOAuth2AuthCodeProvider( AuthProvider::copyWebAuthCodeFallback ); - oauth2Data = boost::make_shared<libcmis::OAuth2Data>( - GDRIVE_AUTH_URL, GDRIVE_TOKEN_URL, - GDRIVE_SCOPE, GDRIVE_REDIRECT_URI, - GDRIVE_CLIENT_ID, GDRIVE_CLIENT_SECRET ); - } - if ( m_aURL.getBindingUrl().startsWith( ALFRESCO_CLOUD_BASE_URL ) ) - oauth2Data = boost::make_shared<libcmis::OAuth2Data>( - ALFRESCO_CLOUD_AUTH_URL, ALFRESCO_CLOUD_TOKEN_URL, - ALFRESCO_CLOUD_SCOPE, ALFRESCO_CLOUD_REDIRECT_URI, - ALFRESCO_CLOUD_CLIENT_ID, ALFRESCO_CLOUD_CLIENT_SECRET ); - if ( m_aURL.getBindingUrl( ) == ONEDRIVE_BASE_URL ) - { - libcmis::SessionFactory::setOAuth2AuthCodeProvider( AuthProvider::copyWebAuthCodeFallback ); - oauth2Data = boost::make_shared<libcmis::OAuth2Data>( - ONEDRIVE_AUTH_URL, ONEDRIVE_TOKEN_URL, - ONEDRIVE_SCOPE, ONEDRIVE_REDIRECT_URI, - ONEDRIVE_CLIENT_ID, ONEDRIVE_CLIENT_SECRET ); - } - - std::unique_ptr<libcmis::Session> session(libcmis::SessionFactory::createSession( - OUSTR_TO_STDSTR( m_aURL.getBindingUrl( ) ), - rUsername, rPassword, "", false, std::move(oauth2Data) )); - if (!session) - ucbhelper::cancelCommandExecution( - ucb::IOErrorCode_INVALID_DEVICE, - uno::Sequence< uno::Any >( 0 ), - xEnv ); - m_aRepositories = session->getRepositories( ); - - bIsDone = true; - } - catch ( const libcmis::Exception& e ) - { - SAL_INFO( "ucb.ucp.cmis", "Error getting repositories: " << e.what() ); - - if (e.getType() == "dnsFailed") - { - uno::Any ex; - ex <<= ucb::InteractiveNetworkResolveNameException( - OStringToOUString(e.what(), RTL_TEXTENCODING_UTF8), - getXWeak(), - task::InteractionClassification_ERROR, - m_aURL.getBindingUrl()); - ucbhelper::cancelCommandExecution(ex, xEnv); - } - else if (e.getType() == "connectFailed" || e.getType() == "connectTimeout") - { - uno::Any ex; - ex <<= ucb::InteractiveNetworkConnectException( - OStringToOUString(e.what(), RTL_TEXTENCODING_UTF8), - getXWeak(), - task::InteractionClassification_ERROR, - m_aURL.getBindingUrl()); - ucbhelper::cancelCommandExecution(ex, xEnv); - } - else if (e.getType() == "transferFailed") - { - uno::Any ex; - ex <<= ucb::InteractiveNetworkReadException( - OStringToOUString(e.what(), RTL_TEXTENCODING_UTF8), - getXWeak(), - task::InteractionClassification_ERROR, - m_aURL.getBindingUrl()); - ucbhelper::cancelCommandExecution(ex, xEnv); - } - else if (e.getType() != "permissionDenied") - { - ucbhelper::cancelCommandExecution( - ucb::IOErrorCode_INVALID_DEVICE, - uno::Sequence< uno::Any >( 0 ), - xEnv ); - } - } - } - else - { - // Throw user cancelled exception - ucbhelper::cancelCommandExecution( - ucb::IOErrorCode_ABORT, - uno::Sequence< uno::Any >( 0 ), - xEnv, - u"Authentication cancelled"_ustr ); - } - } + libcmis::Session* session = createSession(xEnv, *m_pProvider, m_aURL, + m_xIdentifier->getContentIdentifier(), this); + assert(session); + m_aRepositories = session->getRepositories(); } libcmis::RepositoryPtr RepoContent::getRepository( const uno::Reference< ucb::XCommandEnvironment > & xEnv ) diff --git a/ucb/source/ucp/cmis/cmis_repo_content.hxx b/ucb/source/ucp/cmis/cmis_repo_content.hxx index 0f78bf0be14d..74e85520fb2a 100644 --- a/ucb/source/ucp/cmis/cmis_repo_content.hxx +++ b/ucb/source/ucp/cmis/cmis_repo_content.hxx @@ -41,8 +41,6 @@ namespace ucbhelper namespace cmis { -inline constexpr OUString CMIS_REPO_TYPE = u"application/vnd.libreoffice.cmis-repository"_ustr; - class ContentProvider; class RepoContent : public ::ucbhelper::ContentImplHelper, public ChildrenProvider
