Show ****** in wifi passphrase field

Show ****** in wifi passphrase field

Shill does not send the passphrase to Chrome for security reasons, so
show ****** in the passphrase field when configuring wifi if the service
is Connectable. (Sometimes when a connection fails we do not know why
it failed and so we show the configure dialog in case it is an error in
the configuration).

BUG=264819
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=221491

[armansito, 2013-09-04 00:25:16]

https://chromiumcodereview.appspot.com/23522013/diff/3001/chrome/browser/chro...
File chrome/browser/chromeos/options/wifi_config_view.cc (right):

https://chromiumcodereview.appspot.com/23522013/diff/3001/chrome/browser/chro...
chrome/browser/chromeos/options/wifi_config_view.cc:1285:
flimflam::kConnectableProperty, &connectable);
From my understanding, passphrases are not mandatory for 802.1x, so there maybe
no password involved at all (i.e. flimflam::kPassphraseRequiredProperty would be
false) but the service might still be connectable. Would it be confusing for a
user if a fake password is displayed in that case, even though one is not set?

[stevenjb, 2013-09-04 00:41:14]

https://chromiumcodereview.appspot.com/23522013/diff/3001/chrome/browser/chro...
File chrome/browser/chromeos/options/wifi_config_view.cc (right):

https://chromiumcodereview.appspot.com/23522013/diff/3001/chrome/browser/chro...
chrome/browser/chromeos/options/wifi_config_view.cc:1285:
flimflam::kConnectableProperty, &connectable);
On 2013/09/04 00:25:16, armansito wrote:
> From my understanding, passphrases are not mandatory for 802.1x, so there
maybe
> no password involved at all (i.e. flimflam::kPassphraseRequiredProperty would
be
> false) but the service might still be connectable. Would it be confusing for a
> user if a fake password is displayed in that case, even though one is not set?

So, kPassphraseRequiredProperty does not apply to 802.1x.

PassphraseActive() will be false for TLS which uses a certificate. 

My understanding is that other 802.1x methods (PEAP, LEAP, TTLS) do generally
require a passphrase.

I think we should change Shill to either set kPassphraseRequiredProperty or
another property for 802.1x, but this seemed like a reasonable short term fix.

[armansito, 2013-09-04 02:35:21]

On 2013/09/04 00:41:14, stevenjb (chromium) wrote:
>
https://chromiumcodereview.appspot.com/23522013/diff/3001/chrome/browser/chro...
> File chrome/browser/chromeos/options/wifi_config_view.cc (right):
> 
>
https://chromiumcodereview.appspot.com/23522013/diff/3001/chrome/browser/chro...
> chrome/browser/chromeos/options/wifi_config_view.cc:1285:
> flimflam::kConnectableProperty, &connectable);
> On 2013/09/04 00:25:16, armansito wrote:
> > From my understanding, passphrases are not mandatory for 802.1x, so there
> maybe
> > no password involved at all (i.e. flimflam::kPassphraseRequiredProperty
would
> be
> > false) but the service might still be connectable. Would it be confusing for
a
> > user if a fake password is displayed in that case, even though one is not
set?
> 
> So, kPassphraseRequiredProperty does not apply to 802.1x.
> 
> PassphraseActive() will be false for TLS which uses a certificate. 
> 
> My understanding is that other 802.1x methods (PEAP, LEAP, TTLS) do generally
> require a passphrase.
> 
> I think we should change Shill to either set kPassphraseRequiredProperty or
> another property for 802.1x, but this seemed like a reasonable short term fix.

Is the passphrase textfield hidden if PassphraseActive()? I think it's ok to use
connectable as long as what the user sees make sense. I agree that
kPassphraseRequiredProperty should have a meaningful value for 802.1x, but I'm
not an expert on the wifi authentication code.

[stevenjb, 2013-09-04 21:22:39]

On 2013/09/04 02:35:21, armansito wrote:
> On 2013/09/04 00:41:14, stevenjb (chromium) wrote:
> >
>
https://chromiumcodereview.appspot.com/23522013/diff/3001/chrome/browser/chro...
> > File chrome/browser/chromeos/options/wifi_config_view.cc (right):
> > 
> >
>
https://chromiumcodereview.appspot.com/23522013/diff/3001/chrome/browser/chro...
> > chrome/browser/chromeos/options/wifi_config_view.cc:1285:
> > flimflam::kConnectableProperty, &connectable);
> > On 2013/09/04 00:25:16, armansito wrote:
> > > From my understanding, passphrases are not mandatory for 802.1x, so there
> > maybe
> > > no password involved at all (i.e. flimflam::kPassphraseRequiredProperty
> would
> > be
> > > false) but the service might still be connectable. Would it be confusing
for
> a
> > > user if a fake password is displayed in that case, even though one is not
> set?
> > 
> > So, kPassphraseRequiredProperty does not apply to 802.1x.
> > 
> > PassphraseActive() will be false for TLS which uses a certificate. 
> > 
> > My understanding is that other 802.1x methods (PEAP, LEAP, TTLS) do
generally
> > require a passphrase.
> > 
> > I think we should change Shill to either set kPassphraseRequiredProperty or
> > another property for 802.1x, but this seemed like a reasonable short term
fix.
> 
> Is the passphrase textfield hidden if PassphraseActive()? I think it's ok to
use
> connectable as long as what the user sees make sense. I agree that
> kPassphraseRequiredProperty should have a meaningful value for 802.1x, but I'm
> not an expert on the wifi authentication code.

It's disabled, which is not as good as hidden, but we want to migrate these to
HTML so that we can more easily do this sort of dynamic layout.

[armansito, 2013-09-04 22:00:14]

lgtm

[commit-bot: I haz the power, 2013-09-04 22:34:49]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a lowly provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[commit-bot: I haz the power, 2013-09-05 17:13:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stevenjb@chromium.org/23522013/8001

[commit-bot: I haz the power, 2013-09-05 19:17:49]

Change committed as 221491