Simplify SignContents_Impl by moving readonly check to caller methods

Change-Id: I95d9b12d7e1fc715363396e10bc4adc869978c44
Reviewed-on: https://gerrit.libreoffice.org/54478
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Samuel Mehrbrodt <Samuel.Mehrbrodt@cib.de>

2 files changed, 147 insertions, 127 deletions

diff --git a/sfx2/source/doc/docfile.cxx b/sfx2/source/doc/docfile.cxx
index acd54f1fc8c4..9498b287231a 100644
--- a/sfx2/source/doc/docfile.cxx
+++ b/sfx2/source/doc/docfile.cxx
@@ -3680,61 +3680,86 @@ bool SfxMedium::SignContents_Impl(const Reference<XCertificate> xCert, const OUS
             comphelper::getProcessComponentContext(), aODFVersion, bHasValidDocumentSignature ) );
 
     uno::Reference< embed::XStorage > xWriteableZipStor;
-    // Signing is not modification of the document, as seen by the user
-    // ("only a saved document can be signed"). So allow signing in the
-    // "opened read-only, but not physically-read-only" case.
-    if (!IsOriginallyReadOnly())
+
+    // we can reuse the temporary file if there is one already
+    CreateTempFile( false );
+    GetMedium_Impl();
+
+    try
     {
-        // we can reuse the temporary file if there is one already
-        CreateTempFile( false );
-        GetMedium_Impl();
+        if ( !pImpl->xStream.is() )
+            throw uno::RuntimeException();
 
+        bool bODF = GetFilter()->IsOwnFormat();
         try
         {
-            if ( !pImpl->xStream.is() )
-                throw uno::RuntimeException();
+            xWriteableZipStor = ::comphelper::OStorageHelper::GetStorageOfFormatFromStream( ZIP_STORAGE_FORMAT_STRING, pImpl->xStream );
+        }
+        catch (const io::IOException& rException)
+        {
+            if (bODF)
+                SAL_WARN("sfx.doc", "ODF stream is not a zip storage: " << rException);
+        }
 
-            bool bODF = GetFilter()->IsOwnFormat();
-            try
-            {
-                xWriteableZipStor = ::comphelper::OStorageHelper::GetStorageOfFormatFromStream( ZIP_STORAGE_FORMAT_STRING, pImpl->xStream );
-            }
-            catch (const io::IOException& rException)
-            {
-                if (bODF)
-                    SAL_WARN("sfx.doc", "ODF stream is not a zip storage: " << rException);
-            }
+        if ( !xWriteableZipStor.is() && bODF )
+            throw uno::RuntimeException();
 
-            if ( !xWriteableZipStor.is() && bODF )
+        uno::Reference< embed::XStorage > xMetaInf;
+        uno::Reference<container::XNameAccess> xNameAccess(xWriteableZipStor, uno::UNO_QUERY);
+        if (xNameAccess.is() && xNameAccess->hasByName("META-INF"))
+        {
+            xMetaInf = xWriteableZipStor->openStorageElement(
+                                            "META-INF",
+                                            embed::ElementModes::READWRITE );
+            if ( !xMetaInf.is() )
                 throw uno::RuntimeException();
+        }
+
+        if ( bScriptingContent )
+        {
+            // If the signature has already the document signature it will be removed
+            // after the scripting signature is inserted.
+            uno::Reference< io::XStream > xStream(
+                xMetaInf->openStreamElement( xSigner->getScriptingContentSignatureDefaultStreamName(),
+                                                embed::ElementModes::READWRITE ),
+                uno::UNO_SET_THROW );
 
-            uno::Reference< embed::XStorage > xMetaInf;
-            uno::Reference<container::XNameAccess> xNameAccess(xWriteableZipStor, uno::UNO_QUERY);
-            if (xNameAccess.is() && xNameAccess->hasByName("META-INF"))
+            if ( xSigner->signScriptingContent( GetZipStorageToSign_Impl(), xStream ) )
             {
-                xMetaInf = xWriteableZipStor->openStorageElement(
-                                                "META-INF",
-                                                embed::ElementModes::READWRITE );
-                if ( !xMetaInf.is() )
-                    throw uno::RuntimeException();
-            }
+                // remove the document signature if any
+                OUString aDocSigName = xSigner->getDocumentContentSignatureDefaultStreamName();
+                if ( !aDocSigName.isEmpty() && xMetaInf->hasByName( aDocSigName ) )
+                    xMetaInf->removeElement( aDocSigName );
+
+                uno::Reference< embed::XTransactedObject > xTransact( xMetaInf, uno::UNO_QUERY_THROW );
+                xTransact->commit();
+                xTransact.set( xWriteableZipStor, uno::UNO_QUERY_THROW );
+                xTransact->commit();
 
-            if ( bScriptingContent )
+                // the temporary file has been written, commit it to the original file
+                Commit();
+                bChanges = true;
+            }
+        }
+        else
+        {
+            if (xMetaInf.is())
             {
-                // If the signature has already the document signature it will be removed
-                // after the scripting signature is inserted.
-                uno::Reference< io::XStream > xStream(
-                    xMetaInf->openStreamElement( xSigner->getScriptingContentSignatureDefaultStreamName(),
-                                                    embed::ElementModes::READWRITE ),
-                    uno::UNO_SET_THROW );
+                // ODF.
+                uno::Reference< io::XStream > xStream;
+                if (GetFilter() && GetFilter()->IsOwnFormat())
+                    xStream.set(xMetaInf->openStreamElement(xSigner->getDocumentContentSignatureDefaultStreamName(), embed::ElementModes::READWRITE), uno::UNO_SET_THROW);
 
-                if ( xSigner->signScriptingContent( GetZipStorageToSign_Impl(), xStream ) )
-                {
-                    // remove the document signature if any
-                    OUString aDocSigName = xSigner->getDocumentContentSignatureDefaultStreamName();
-                    if ( !aDocSigName.isEmpty() && xMetaInf->hasByName( aDocSigName ) )
-                        xMetaInf->removeElement( aDocSigName );
+                bool bSuccess = false;
+                if (xCert.is())
+                    bSuccess = xSigner->signDocumentContentWithCertificate(
+                        GetZipStorageToSign_Impl(), xStream, xCert, aSignatureLineId);
+                else
+                    bSuccess = xSigner->signDocumentContent(GetZipStorageToSign_Impl(),
+                                                            xStream);
 
+                if (bSuccess)
+                {
                     uno::Reference< embed::XTransactedObject > xTransact( xMetaInf, uno::UNO_QUERY_THROW );
                     xTransact->commit();
                     xTransact.set( xWriteableZipStor, uno::UNO_QUERY_THROW );
@@ -3745,104 +3770,50 @@ bool SfxMedium::SignContents_Impl(const Reference<XCertificate> xCert, const OUS
                     bChanges = true;
                 }
             }
-            else
+            else if (xWriteableZipStor.is())
             {
-                if (xMetaInf.is())
-                {
-                    // ODF.
-                    uno::Reference< io::XStream > xStream;
-                    if (GetFilter() && GetFilter()->IsOwnFormat())
-                        xStream.set(xMetaInf->openStreamElement(xSigner->getDocumentContentSignatureDefaultStreamName(), embed::ElementModes::READWRITE), uno::UNO_SET_THROW);
-
-                    bool bSuccess = false;
-                    if (xCert.is())
-                        bSuccess = xSigner->signDocumentContentWithCertificate(
-                            GetZipStorageToSign_Impl(), xStream, xCert, aSignatureLineId);
-                    else
-                        bSuccess = xSigner->signDocumentContent(GetZipStorageToSign_Impl(),
-                                                                xStream);
+                // OOXML.
+                uno::Reference<io::XStream> xStream;
 
-                    if (bSuccess)
-                    {
-                        uno::Reference< embed::XTransactedObject > xTransact( xMetaInf, uno::UNO_QUERY_THROW );
-                        xTransact->commit();
-                        xTransact.set( xWriteableZipStor, uno::UNO_QUERY_THROW );
-                        xTransact->commit();
-
-                        // the temporary file has been written, commit it to the original file
-                        Commit();
-                        bChanges = true;
-                    }
-                }
-                else if (xWriteableZipStor.is())
+                bool bSuccess = false;
+                if (xCert.is())
                 {
-                    // OOXML.
-                    uno::Reference<io::XStream> xStream;
-
-                    bool bSuccess = false;
-                    if (xCert.is())
-                    {
-                        bSuccess = xSigner->signDocumentContentWithCertificate(
-                            GetZipStorageToSign_Impl(/*bReadOnly=*/false), xStream, xCert, aSignatureLineId);
-                    }
-                    else
-                    {
-                        // We need read-write to be able to add the signature relation.
-                        bSuccess =xSigner->signDocumentContent(
-                            GetZipStorageToSign_Impl(/*bReadOnly=*/false), xStream);
-                    }
-
-                    if (bSuccess)
-                    {
-                        uno::Reference<embed::XTransactedObject> xTransact(xWriteableZipStor, uno::UNO_QUERY_THROW);
-                        xTransact->commit();
-
-                        // the temporary file has been written, commit it to the original file
-                        Commit();
-                        bChanges = true;
-                    }
+                    bSuccess = xSigner->signDocumentContentWithCertificate(
+                        GetZipStorageToSign_Impl(/*bReadOnly=*/false), xStream, xCert, aSignatureLineId);
                 }
                 else
                 {
-                    // Something not ZIP based: e.g. PDF.
-                    std::unique_ptr<SvStream> pStream(utl::UcbStreamHelper::CreateStream(GetName(), StreamMode::READ | StreamMode::WRITE));
-                    uno::Reference<io::XStream> xStream(new utl::OStreamWrapper(*pStream));
-                    if (xSigner->signDocumentContent(uno::Reference<embed::XStorage>(), xStream))
-                        bChanges = true;
+                    // We need read-write to be able to add the signature relation.
+                    bSuccess =xSigner->signDocumentContent(
+                        GetZipStorageToSign_Impl(/*bReadOnly=*/false), xStream);
                 }
-            }
-        }
-        catch ( const uno::Exception& )
-        {
-            SAL_WARN( "sfx.doc", "Couldn't use signing functionality!" );
-        }
 
-        CloseAndRelease();
-    }
-    else
-    {
-        try
-        {
-            if ( bScriptingContent )
-                xSigner->showScriptingContentSignatures( GetZipStorageToSign_Impl(), uno::Reference< io::XInputStream >() );
-            else
-            {
-                uno::Reference<embed::XStorage> xStorage = GetZipStorageToSign_Impl();
-                if (xStorage.is())
-                    xSigner->showDocumentContentSignatures(xStorage, uno::Reference<io::XInputStream>());
-                else
+                if (bSuccess)
                 {
-                    std::unique_ptr<SvStream> pStream(utl::UcbStreamHelper::CreateStream(GetName(), StreamMode::READ));
-                    uno::Reference<io::XInputStream> xStream(new utl::OStreamWrapper(*pStream));
-                    xSigner->showDocumentContentSignatures(uno::Reference<embed::XStorage>(), xStream);
+                    uno::Reference<embed::XTransactedObject> xTransact(xWriteableZipStor, uno::UNO_QUERY_THROW);
+                    xTransact->commit();
+
+                    // the temporary file has been written, commit it to the original file
+                    Commit();
+                    bChanges = true;
                 }
             }
-        }
-        catch( const uno::Exception& )
-        {
-            SAL_WARN( "sfx.doc", "Couldn't use signing functionality!" );
+            else
+            {
+                // Something not ZIP based: e.g. PDF.
+                std::unique_ptr<SvStream> pStream(utl::UcbStreamHelper::CreateStream(GetName(), StreamMode::READ | StreamMode::WRITE));
+                uno::Reference<io::XStream> xStream(new utl::OStreamWrapper(*pStream));
+                if (xSigner->signDocumentContent(uno::Reference<embed::XStorage>(), xStream))
+                    bChanges = true;
+            }
         }
     }
+    catch ( const uno::Exception& )
+    {
+        SAL_WARN( "sfx.doc", "Couldn't use signing functionality!" );
+    }
+
+    CloseAndRelease();
 
     ResetError();
 
diff --git a/sfx2/source/doc/objserv.cxx b/sfx2/source/doc/objserv.cxx
index fb4bb7ca7a28..c1b81cbf62e0 100644
--- a/sfx2/source/doc/objserv.cxx
+++ b/sfx2/source/doc/objserv.cxx
@@ -1496,6 +1496,46 @@ void SfxObjectShell::AfterSigning(bool bSignSuccess, bool bSignScriptingContent)
         EnableSetModified();
 }
 
+bool SfxObjectShell::CheckIsReadonly(bool bSignScriptingContent)
+{
+    if (GetMedium()->IsOriginallyReadOnly())
+    {
+        // If the file is physically read-only, we just show the existing signatures
+        try
+        {
+            OUString aODFVersion(
+                comphelper::OStorageHelper::GetODFVersionFromStorage(GetStorage()));
+            uno::Reference<security::XDocumentDigitalSignatures> xSigner(
+                security::DocumentDigitalSignatures::createWithVersionAndValidSignature(
+                    comphelper::getProcessComponentContext(), aODFVersion, HasValidSignatures()));
+            if (bSignScriptingContent)
+                xSigner->showScriptingContentSignatures(GetMedium()->GetZipStorageToSign_Impl(),
+                                                        uno::Reference<io::XInputStream>());
+            else
+            {
+                uno::Reference<embed::XStorage> xStorage = GetMedium()->GetZipStorageToSign_Impl();
+                if (xStorage.is())
+                    xSigner->showDocumentContentSignatures(xStorage,
+                                                           uno::Reference<io::XInputStream>());
+                else
+                {
+                    std::unique_ptr<SvStream> pStream(
+                        utl::UcbStreamHelper::CreateStream(GetName(), StreamMode::READ));
+                    uno::Reference<io::XInputStream> xStream(new utl::OStreamWrapper(*pStream));
+                    xSigner->showDocumentContentSignatures(uno::Reference<embed::XStorage>(),
+                                                           xStream);
+                }
+            }
+        }
+        catch (const uno::Exception&)
+        {
+            SAL_WARN("sfx.doc", "Couldn't use signing functionality!");
+        }
+        return true;
+    }
+    return false;
+}
+
 bool SfxObjectShell::HasValidSignatures()
 {
     return pImpl->nDocumentSignatureState == SignatureState::OK
@@ -1513,6 +1553,9 @@ void SfxObjectShell::SignDocumentContent()
     if (!PrepareForSigning())
         return;
 
+    if (CheckIsReadonly(false))
+        return;
+
     OUString aODFVersion(comphelper::OStorageHelper::GetODFVersionFromStorage(GetStorage()));
     bool bSignSuccess = GetMedium()->SignContents_Impl(
         Reference<XCertificate>(), "", false, aODFVersion, HasValidSignatures());
@@ -1526,6 +1569,9 @@ void SfxObjectShell::SignSignatureLine(const OUString& aSignatureLineId,
     if (!PrepareForSigning())
         return;
 
+    if (CheckIsReadonly(false))
+        return;
+
     OUString aODFVersion(comphelper::OStorageHelper::GetODFVersionFromStorage(GetStorage()));
     bool bSignSuccess = GetMedium()->SignContents_Impl(
         xCert, aSignatureLineId, false, aODFVersion, HasValidSignatures());
@@ -1543,6 +1589,9 @@ void SfxObjectShell::SignScriptingContent()
     if (!PrepareForSigning())
         return;
 
+    if (CheckIsReadonly(true))
+        return;
+
     OUString aODFVersion(comphelper::OStorageHelper::GetODFVersionFromStorage(GetStorage()));
     bool bSignSuccess = GetMedium()->SignContents_Impl(
         Reference<XCertificate>(), OUString(), true, aODFVersion, HasValidSignatures());