Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed #13500 - Try to prevent the browser from pre-filling the LDAP password #13537

Merged
merged 1 commit into from
Aug 28, 2023

Conversation

snipe
Copy link
Owner

@snipe snipe commented Aug 28, 2023

This is an attempt to fix #13500, where the user's browser was pre-filling the ldap_pword field to be "helpful", which meant every time they saved the LDAP settings, the ldap_pword field would be overwritten in the database to their own user password. Thanks, browser! Super helpful.

@Godmartinz - if this looks okay, can we try to apply this to your new LDAP livewire stuff? (We should also maybe add a new class to the CSS to make this part of our actual CSS, versus inlining it, as those in dark mode will have a white field that will look pretty jarring. A new class like hide-readonly or something.

This PR yoinks a lot of the functionality from the user edit/create page for largely the same reasons - before adding that, admins who were editing a user wouldn't notice that the browser filled in that field with the admin's password:

input[type='text'][disabled], input[disabled], textarea[disabled], input[readonly], textarea[readonly], .form-control[disabled], .form-control[readonly], fieldset[disabled] .form-control {
background-color: white;
color: #555555;
cursor:text;
}

<input
type="password"
name="password"
class="form-control"
id="password"
value=""
maxlength="500"
autocomplete="off"
readonly
onfocus="this.removeAttribute('readonly');"
{{ ((config('app.lock_passwords') && ($user->id)) ? ' disabled' : '') }}>

@what-the-diff
Copy link

what-the-diff bot commented Aug 28, 2023

PR Summary

  • Implementation of better styling for password field
    The password field on the LDAP settings page now looks readonly. This is a part of user interface design improvements to enhance user interaction with the form.

  • Deactivation of auto-complete feature in forms
    The auto-complete attribute of the form set to 'off'. This change helps in increasing data security on the form, by preventing browsers from automatically suggesting previously entered information.

  • Addition of hidden input field for username
    A new hidden input field was added carrying the username, which is derived either from the request parameter or from the user's actual username. This may not visible in the user interface but is important for back end processes.

  • Change in the structure of HTML elements
    The structure of some HTML elements within a specific div (a block of content) on the form was modified to improve the website's overall layout and appearance.

  • Changing placeholder attribute values
    The placeholder text that users see in the input fields for 'ldap_uname', 'ldap_pword', and 'ldaptest_password' have been updated. This modification means the users see more informative or accurate instructions or information before they enter the relevant data in these fields.

  • Added 'autocomplete' and 'onfocus' attributes to password input
    The password input field not only disables autocompletion but also allows the readonly attribute to be removed upon the users focusing on that field. This means that the look of the field is readonly, but it allows typing when a user clicks to enter their password. This helps in improving data security and the usability operability with the password field.

@snipe
Copy link
Owner Author

snipe commented Aug 28, 2023

Interestingly, I don't see the hidden input trick in the user's edit screen, but people haven't complained about it auto-filling recently, so I guess that's ok.

@snipe snipe merged commit 367484a into develop Aug 28, 2023
5 checks passed
@snipe snipe deleted the fixes/13500_prevent_autocomplete_on_ldap_password branch August 28, 2023 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant