1008736 - Settings WiFi EAP-TLS UI is confusing and invalid

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Settings, defect)

Description

[:gerard-majax]

To my knowledge, EAP-TLS authentication method does not use any login/password tuple and instead rely on a user certificate.

The current Settings UI to add a new (hidden) network allows to select WPA-EAP as a security. Once EAP is selected, one can choose the EAP method within: SIM, PEAP, TLS and TTLS.

When selecting EAP-TLS, then the UI changes and expects the users to give:
 - a login
 - a password
 - select a server certificate

This is just wrong for EAP-TLS, which expects users to enter:
 - an identity
 - a user certificate
 - a CA certificate
 - a private key
 - a passphrase

Private key and user may be bundled together (PKCS#12 for instance).

To my understanding and given how bug 745468 has been renamed, we do not (yet) support EAP-TLS. However, the UI setting is there and it may confuse users in two way:
 - letting them think EAP-TLS is supported
 - setting a broken configuration

So, if EAP-TLS is not yet supported, we should just not expose it in the settings. If it is, then this UI seems to need to be fixed.

Comment 1

[:gerard-majax]

Adding RFC section describing peer certificate validation.

Comment 2

[:gerard-majax]

UX specs given in bug 988150 does not mention presenting EAP-TLS method.

Comment 3

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

We just don't support TLS yet, Gaia will be update after bug 1002314 to show supported EAP methods based on system capabilities.

Comment 4

[:gerard-majax]

(In reply to Chuck Lee [:chucklee] from comment #3)
> We just don't support TLS yet, Gaia will be update after bug 1002314 to show
> supported EAP methods based on system capabilities.

I see your linked bug is a gecko one, quite invasive. Why don't you just remove TLS from Gaia for now ?

And that does not answers the fact that the current TLS UI is wrong.

Comment 5

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

I know UI is wrong now, and I like to fix it by Gaia's support for wifi capabilities within this release.
Then Gaia team don't have to modify code which will be removed later.

Comment 6

[:gerard-majax]

(In reply to Chuck Lee [:chucklee] from comment #5)
> I know UI is wrong now, and I like to fix it by Gaia's support for wifi
> capabilities within this release.
> Then Gaia team don't have to modify code which will be removed later.

This code is already landed: people will have to fix it anyway. And it's exposed to users, who will be mislead.

Comment 7

[Jason Smith [:jsmith]]

Based on Chuck's comments, I don't think this is a regression, but rather a new feature bug.

Comment 8

[:gerard-majax]

UX people, what do you think of this ?

Comment 9

[Stephany Wilkes]

Re-adding Omega for UX comment here. Omega, please comment or just let us know if you agree/disagree with other comments if that's the case.

Comment 10

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Ian, is this part of bug 1003054?

Comment 11

[[e-mail], [e-mail]]

Yes, per https://bugzilla.mozilla.org/show_bug.cgi?id=1002314#c8, Gaia have to reach the capability of authentication which are supported in different Gecko version, platform. See "TLS" is not in property "eapMethods" for wifi in 2.0.

Comment 12

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Let's dup the two bugs?

Comment 14

[Omega Feng [:Omega] [:馮於懋]]

ni? new UX owner Jenny.

Comment 15

[Jenny Lee]

From UX perspective, it's better to remove TLS from the list.

Comment 16

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: Pull Request

Comment 17

[[e-mail], [e-mail]]

Comment on attachment 8433987 [details] [review]
Pull Request

Hi Chuck,

Since the bug is duplicated with bug 1003054, the supported options should be referenced from capability API. We have to avoid to change the option on different Gaia version without capability reference. We're afraid that other more options needed to be change again.

Comment 18

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

It's was my original plan, but bug 1003054 seems won't done within 2.0 time flame.
So I think we might need to remove TTL from UI if UX think it should.

Comment 19

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

(In reply to Chuck Lee [:chucklee] from comment #18)
> It's was my original plan, but bug 1003054 seems won't done within 2.0 time
> flame.
> So I think we might need to remove TTL from UI if UX think it should.

Hi Chucklee,

I am not sure this patch is needed in 2.0 or not because it is currently in backlog. Let me ni? howie to decide.

If yes, you also have to remove the same part in wifi_helper, but to be honest, this feature will be put back soon when you finish the feature, so I think it is not a must-have action at this time especially we are so close to branch out.

Comment 20

[howie [:howie]]

Based on the time frame, let's not put in 2.0

Comment 21

[:gerard-majax]

Seriously, the fix is a one line until you implement proper feature detection. Let's not ship a broken v2.0, please !

Comment 22

[José Antonio Olivera Ortega [:jaoo]]

Comment on attachment 8433987 [details] [review]
Pull Request

Sorry guys but I don't feel comfortable enough with reviewing a part of the setting app that I barely know. Forwarding this request to Arthur as he might know. Thanks.

Comment 23

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8433987 [details] [review]
Pull Request

Let's hide the menu until TLS is supported.

Comment 24

[Arthur Chen [:arthurcc][inactive after 6/30]]

master: 669bb17a487e53b2c62d628d0e1d1e94968da919

Comment 25

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8433987 [details] [review]
Pull Request

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined: Users will see options not functioning
[Testing completed]: manual test
[Risk to taking this patch] (and alternatives if risky): none
[String changes made]: none

Comment 26

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/7cf86edd1054318e82099119c535c3a83ad2ecfa