Bug 148416 - FILESAVE: Read-only passwords are not requested by default on save-as
Summary: FILESAVE: Read-only passwords are not requested by default on save-as
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: All All
: low minor
Assignee: Justin L
URL:
Whiteboard: target:24.2.0
Keywords: patch
Depends on:
Blocks: Password-Protected
  Show dependency treegraph
 
Reported: 2022-04-06 09:04 UTC by Xisco Faulí
Modified: 2023-09-08 14:32 UTC (History)
7 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Xisco Faulí 2022-04-06 09:04:01 UTC
This is a follow-up of bug 144996

Steps to reproduce:
1. Open attachment 175647 [details] from bug 144996
2. Edit - Edit Mode
3. Introduce the password
4. Modify the file
5. Save to a new document
6. Close the document and open the new document

-> it's possible to edit the new document without the password

Reproduced in

Version: 7.4.0.0.alpha0+ / LibreOffice Community
Build ID: bd992ae1228b2f7e556f89f95949da0aeade5b91
CPU threads: 8; OS: Linux 5.10; UI render: default; VCL: gtk3
Locale: en-US (es_ES.UTF-8); UI: en-US
Calc: threaded
Comment 1 Roman Kuznetsov 2022-04-06 10:01:39 UTC Comment hidden (obsolete)
Comment 2 Xisco Faulí 2022-04-06 10:08:56 UTC
(In reply to Roman Kuznetsov from comment #1)
> Is the bug 142147 about the same?

I don't think so, this is about read-only protection
Comment 3 Timur 2022-04-06 13:19:45 UTC
I think it is the same, never mind if RO or open password, they are all in the same dialog.

*** This bug has been marked as a duplicate of bug 142147 ***
Comment 4 Xisco Faulí 2022-04-06 13:54:03 UTC
(In reply to Timur from comment #3)
> I think it is the same, never mind if RO or open password, they are all in
> the same dialog.
> 
> *** This bug has been marked as a duplicate of bug 142147 ***

Then it should be fine if e0cced9d4c94324e834e46d807469a0cd6c1f738 is reverted, which is not the case...
Comment 5 Justin L 2022-11-24 19:19:18 UTC
Using XLSX format is not ideal for this bug report. The password protection for the provided example is only imported starting in LO 7.3 with
commit 40f38fd16dad4374543d4a7a109b3264837ce8d1
Author: Tünde Tóth on Wed Sep 1 15:47:40 2021 +0200
    tdf#115933 XLSX import: fix permission for editing

For ODS format, this password-protected read-only is imported already in LO 3.6. Even back then a save-as doesn't auto-select password mode. Assuming inherited from OOo.

Yes, this is very similar to bug 142147, but it does not use SID_PASSWORD. In fact, I didn't see ANYTHING passing by that knew this setting was enabled.
I assume it is SID_MODIFYPASSWORDINFO.

The place to bPreselectPassword is sfx2/source/doc/objserv.cxx
Comment 6 Justin L 2022-11-25 16:52:27 UTC
The act of requesting a password in this case should be pretty simple. It is the same area as bug 142147.

We have the following functions to work with:
-IsLoadReadonly() // File - Properties - Security - Open File read only
-IsReadOnly() // Is the document currently in read-only mode?
-IsModifyPasswordEntered() // Has the user unlocked edit mode already?
-GetModifyPasswordHash() // one of two ways to require password to modify
-GetModifyPasswordInfo().hasElements() // second way to require edit password.

I'm assuming GetModifyPasswordHash is an old method. During investigation for this bug report, I only saw GetModifyPasswordInfo used.

So the question is "In which situations should we assume the user wants to enter a password?"

Note that "open read only" is hidden under a dropdown, and will ALWAYS start as unchecked. (To change this would be rather tedious I expect. 
cui/source/dialogs/passwdomdlg.cxx's m_xOpenReadonlyCB.

[The function is passed bIsPasswordToModify, but that only indicates whether to
 ENABLE the read-only part of the dialog. It is a LONG calling trail to try to
 pass any config options to it...]

Perhaps the easier question to ask is when would you NOT want to save with password.
-obviously one of the two password methods needs to exist
-but not if the user has turned off IsLoadReadonly - verify still active.
    -question: is there a way to set Modify Password WITHOUT LoadReadOnly?
-so at this point the document is going to open read-only anyway, the only question remaining is whether it should require a password.

In the spirit of bug 142147, it shouldn't matter whether the user has entered edit mode or not. Since a password exists, it should be suggested at SAVE-AS.

so bPreselectPassword &= IsLoadReadonly() && (GetModifyPasswordInfo().hasElements() || GetModifyPasswordHash())

The downside here is that there is NOTHING that hints to the user why the password is being requested. The read-only portion is minimized, and the read-only flag is off (despite being IsLoadReadonly).

I think the key to fixing this is to get that dialog to expand and default to m_xOpenReadonlyCB->set_active(IsLoadReadonly())
Comment 7 Justin L 2022-11-25 18:30:13 UTC
(In reply to Justin L from comment #6)
> I think the key to fixing this is to get that dialog to expand and default
> to m_xOpenReadonlyCB->set_active(IsLoadReadonly())
Doing so requires adding an entry to
-offapi/com/sun/star/task/DocumentPasswordRequest2.idl
-offapi/com/sun/star/task/DocumentMSPasswordRequest2.idl

It just doesn't seem worth it for this minor annoyance.
Comment 8 Justin L 2022-11-25 21:56:26 UTC
While it may not be worth submitting the patch, I decided to try and write it anyway. It seems to work nicely.  https://gerrit.libreoffice.org/c/core/+/143312
Comment 9 Commit Notification 2023-06-28 20:53:57 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/49e34144a8148bf3c77bcfd70bf6c628dcefeedd

tdf#148416 password dialog: suggest current loadreadonly status

It will be available in 24.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 10 Commit Notification 2023-06-28 20:54:00 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/73ac60c4c3a209d23642ac4d0e8c4ac6dba22d86

tdf#148416 saveas: preserve loadreadonly with password

It will be available in 24.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.