Skip to content

Add a revealed PasswordBox style #2812

Closed

Description

@Drise13

Describe the solution you'd like
Looking for a toggle-based password box reveal style. Something similar to MahApp's revealed password box. MahApps seems to require mouse-down to reveal. I believe the correct Material interaction would be to toggle states.

Describe alternatives you've considered
We currently have a custom control that just switches between a textbox and password box, but the toggle button is separate from the input control and seems awkwardly placed.

image

Additional context
Possibly additional nice to haves would be able to directly control the icons used for each state.
Also would be nice if it supported validation rules. (Think WIFI's 8-63 character requirement)

Activity

  1. nicolaihenriksen commented on Aug 20, 2022

    @nicolaihenriksen
    Contributor

    @Drise13 This seems like a valid request, but I am wondering whether there are some security concerns to be aware of here. The PasswordBox in WPF stores the entered password in a SecureString (whether that is secure or not is another discussion). Opting to reveal the password would require pulling the password out of the SecureString and into a clear-text string which naturally will be stored in memory. This can be picked up by a process dump and memory inspection. @Keboo thoughts?

    To address the issue you have with the awkwardly placed reveal button, I think you could solve that quite easily. I would do something akin to this:

    <Grid>
      <PasswordBox Padding="0,0,30,0" />
      <TextBox Padding="0,0,30,0" />
      <Button Width="24" HorizontalAlignment="Right" />
    </Grid>

    The Padding on the PasswordBox/TextBox ensure there is room for the button "inside" the PasswordBox. Padding and Width of course need to be adjusted to the values that match your button.

    If the above approach is not feasible in your control, you could also apply the Padding on the PasswordBox/TextBox like above, and then simply use a negative Margin.Left property on your button.

  2. Keboo commented on Aug 22, 2022

    @Keboo
    Member

    I think it is reasonable to implement this within the library. A few details on what I would expect for the implementation.

    • I really like how MahApps did it by using a behavior to sync between the plain string and SecureString properties. Because the normal password is not a dependency property, registering for events is needed to manually manage synchronization between them. I would put the attached property within a new PasswordBoxAssist class.
    • I would create a separate style for this. It should be something that consumers have to opt-in for, rather than the default.
    • I would add properties PasswordBoxAssist class with several attached properties, a boolean indicating if we are in a revealed state or not, and one for the reveal icon (PackIconKind enum). For now, the icon can simply default to the standard "eye" (like in the screenshots above). But having this property allows people to change it, or toggle it based on revealed state if that is desired.
    • As for validation, that should "just work" with regular binding validation once there is a simple string attached property to bind to.
  3. Drise13 commented on Aug 23, 2022

    @Drise13
    Author

    For my specific use case, we rarely need a proper SecureString. And generally, PasswordBox (at least in my experience of it) breaks the MVVM paradigm and generally makes my life more difficult for some vague notion of "security". Like @nicolaihenriksen mentioned whether that is secure or not is another discussion. (As a side note, Dotnet even recommends against using SecureString since it doesn't do anything except implement IDisposable on anything but Framework).

    My latest "hack" to do what I want with as little effort as possible is to use a single textbox and swap between a password and standard font. A password font is one that replaces all codepoints with a ● character and that seems to do what I want. I really don't think for most of my needs I really need to visually hide the password, but that seems to be the common interface Google uses. In their Home app to setup the Wifi network, they mask it by default but don't require any authentication to unmask the password.

    This "hack" lets me using a single binding and its 100% MVVM compatible. I don't know if it would be within the MDIX purview to add a non-password "password box" (UnsecurePasswordBox maybe?) that explicitly makes the choice not to use the SecureString scheme that PasswordBox uses and be more MVVM friendly. Just a thought.

  4. nicolaihenriksen commented on Aug 23, 2022

    @nicolaihenriksen
    Contributor
    • I would create a separate style for this. It should be something that consumers have to opt-in for, rather than the default.

    Sorry about the long response below :-)

    @Keboo I agree to the opt-in part of this. What nags me a bit about this approach is that we have 4 different PasswordBox styles today: default, floating-hint, filled, and outlined (the three latter are derived styles). We would need to replicate these 4 in a "reveal flavor". And since ControlTemplates cannot be extended/modified (but only overridden) in a style, we would effectively create a copy of the default PasswordBox style and add in the reveal button (and TextBox for editing the revealed password). This means we would need to ensure these stay in sync.

    Another approach could be to wrap the PasswordBox inside of a custom control (e.g. UnsecurePasswordBox as @Drise13 suggests). I see this as having (at least) 2 advantages:

    • We can reuse the existing PasswordBox styles for the encapsulated PasswordBox and thus not need to keep 2 styles in sync. We would still need to extend the default PasswordBox style with the ability for a "reveal button", but that could be done using an internal attached property. This is potentially also a disadvantage; see below.
    • We can expose the UnsecurePasswordBox.Password string as a DependencyProperty which lends itself nicely to the MVVM paradigm. Other properties/methods may need to be delegated into the encapsulated PasswordBox directly.

    However I also see some disadvantages, including:

    • I am not sure how this works with Test Automation; I guess we may need to override OnCreateAutomationPeer() and return a PasswordBoxAutomationPeer passing in the encapsulated PasswordBox in the constructor - hopefully that works, but I don't know for sure.
    • In order for us to have the ControlTemplate include a TextBox in order to edit the revealed password, we would need to modify the default PasswordBox style to include this, which sort of defeats the opt-in functionality because the default style would now include a TextBox with the password in clear text although it may be hidden (or otherwise removed from the Visual Tree).

    Perhaps having 4 dedicated "copies" of the original PasswordBox styles is the safer approach after all...

  5. HavenDV commented on Sep 16, 2022

    @HavenDV

    Trying to use the new MaterialDesignFloatingHintRevealPasswordBox style. There seem to be problems here in some cases, it might be worth repeating the behavior of TextFieldAssist.HasClearButton
    image
    image

    <PasswordBox
        Style="{StaticResource MaterialDesignFloatingHintRevealPasswordBox}"
        Margin="0"
        FontSize="18"
        Foreground="Black"
        material:ValidationAssist.HorizontalAlignment="Right"
        material:ValidationAssist.Background="Transparent"
        material:TextFieldAssist.HasClearButton="True"
        material:PasswordBoxAssist.Password="123"
        >
        <material:HintAssist.Hint>
            <StackPanel Orientation="Horizontal">
                <material:PackIcon Kind="Key" VerticalAlignment="Center"/>
                <TextBlock Text="Password" />
            </StackPanel>
        </material:HintAssist.Hint>
    </PasswordBox>
  6. nicolaihenriksen commented on Sep 16, 2022

    @nicolaihenriksen
    Contributor

    @HavenDV I'll look into it when I have some time - hopefully sooner rather than later.

    For your particular issue, could the cause of the issue be the fact that you set an explicit FontSize? If you leave that out, does it look correct? Anyways, your suggestion about following the TextFieldAssist.HasClearButton approach seems reasonable, I will look into that.

  7. nicolaihenriksen commented on Sep 16, 2022

    @nicolaihenriksen
    Contributor

    @HavenDV I added a PR #2864 which hopefully should resolve the issue. Could you try it out once there is a nightly build available? Thanks.

    I tested with your example (and variants thereof) in the demo app, and it seems to work as expected.

  8. HavenDV commented on Sep 19, 2022

    @HavenDV

    @HavenDV I added a PR #2864 which hopefully should resolve the issue. Could you try it out once there is a nightly build available? Thanks.

    I tested with your example (and variants thereof) in the demo app, and it seems to work as expected.

    I try 4.7.0-ci306 and see the following:
    image
    image

  9. nicolaihenriksen commented on Sep 19, 2022

    @nicolaihenriksen
    Contributor

    @HavenDV Wow, that is surprising to me. When I paste your example from above into the demo tool, I get the following:

    image

    When inspecting with Snoop, it is also clear that the two buttons are placed adjacent to each other in the grid:

    image
    image
    image

    It almost seems like you have a Style that kicks in and changes the VerticalAlignment property of the reveal button (or possibly the clear button)? Could I ask you to write up a simple repro sample?
    Also, tracking the elements with Snoop like I have shown above often gives some good hints as to what is going on.

  10. HavenDV commented on Sep 20, 2022

    @HavenDV

    Could I ask you to write up a simple repro sample?

    material-ui-password-box-reveal.zip

  11. nicolaihenriksen commented on Sep 20, 2022

    @nicolaihenriksen
    Contributor

    @HavenDV Thanks a lot. So it turns out it was not your code that was overriding a style, it was the demo tool. I had not noticed this, but Fields.xaml includes this:

    <Style TargetType="{x:Type materialDesign:PackIcon}" BasedOn="{StaticResource {x:Type materialDesign:PackIcon}}">
        <Setter Property="VerticalAlignment" Value="Center"/>
        <Setter Property="Margin" Value="4 0 4 0"/>
    </Style>

    Which effectively sets the VerticalAlignment of the PackIcon to Center. I have created PR #2873 to fix the alignment directly on the "reveal button".

    In the mean time, you could just override a style in your code, by inserting something like this in a resource dictionary (here I add it directly to the resource dictionary of the PasswordBox itself):

    <PasswordBox.Resources>
        <Style TargetType="material:PackIcon" BasedOn="{StaticResource {x:Type material:PackIcon}}">
            <Setter Property="VerticalAlignment" Value="Center" />
        </Style>
    </PasswordBox.Resources>
  12. added this to the 4.6.1 milestone on Sep 22, 2022
  13. Keboo commented on Sep 26, 2022

    @Keboo
    Member

    This appears to be complete.

  14. Repository owner moved this from In Progress to Done in MDIX - Issues Managementon Sep 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

No type

Projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions