Allow enforcing share passwords only when already asking for a password #38227
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
danxuliu
requested review from
jancborchardt,
Pytal,
a team,
susnux and
nfebe
and removed request for
a team
jancborchardt
approved these changes
May 15, 2023
Merged
Pytal
approved these changes
May 18, 2023
|
So rebase and then merge? |
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
The rest of "<br/>" elements in the file appear immediately after their previous "</label>" rather than in a new line. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
The permitted contents of a paragraph are only phrasing contents, so "div" elements can not be used, although "span" can. Besides being invalid HTML it seems that the browser ends the paragraph at the div, so the label/input that appears after it is treated as being in a new paragraph, which is not indented by default, and thus is not aligned with the rest of its sibling inputs. Note that an additional div is nevertheless added once the page is loaded to be able to select the groups, but this one does not break its parent paragraph (maybe due to being added after the page load, but I do not really know). Nevertheless, it needs to be explicitly indented, and the second indentend wrapper needs to be removed, as it affects only the label but not the div/input, and therefore the label had an extra indentation over the input. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
"Enforce password protection" is a subcase of "Always ask for a password", so it should be indented to visually show the dependency. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
"Enforce password protection" is a subcase of "Always ask for a password", so it should be disabled if its parent option is unchecked. Although other dependant options in the sharing settings are fully hidden instead of just disabled this option is disabled but shown to avoid confusion when updating from a previous Nextcloud version where "Always ask for a password" was unchecked and "Enforce password protection" was checked. Besides that, due to their dependency the enforced password protection is now automatically unchecked too if its parent option is unchecked. Finally, the groups excluded from password requirements are also disabled when "Always ask for a password" is unchecked, as they might be still shown if "Enforce password protection" was checked due to an update from a previous version. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Joas Schilling <[email protected]>
nickvergessen
force-pushed
the
allow-enforcing-share-passwords-only-when-already-asking-for-a-password
branch
from
caf03b5 to
c1f3bcf
Compare
nickvergessen
deleted the
allow-enforcing-share-passwords-only-when-already-asking-for-a-password
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In admin Sharing settings,
Enforce password protectionis a subcase ofAlways ask for a password, so it should be:Always ask for a passwordis not checkedNote that in the Sharing settings other dependant options are fully hidden rather than just disabled when their parent is unchecked. However, existing instances could have
Enforce password protectionchecked andAlways ask for a passwordunchecked, so if admins are not aware of this change it could be confusing for them ifEnforce password protectionis fully gone after updating Nextcloud to the next version.Alternatively (or in addition) some kind of migration could be applied to automatically enable
Always ask for a passwordifEnforce password protectionwas checked, but I was not sure where or how to add it.Besides that change the markup was fixed, as showing
Exclude groups from password requirementscausedSet default expiration dateto be wrongly indented (apparently due to using adivinside ap, which is not allowed, as only phrasing content should be used in paragraphs).🖼️ Screenshots
How to test (scenario 1)
sharing.allow_disabled_password_enforcement_groups => true,inconfig/config.phpEnforce password protectionResult with this pull request
Set default expiration dateis properly indentedResult without this pull request
Set default expiration datehas no indentation and appears as a sibling ofAllow users to share via link and emailsinstead of a childHow to test (scenario 2)
Always ask for a passwordResult with this pull request
Enforce password protectionis enabled whenAlways ask for a passwordis checked, and disabled when it is notResult without this pull request
Enforce password protectiondoes not change