Closed Bug 1179160 Opened 4 years ago Closed 4 years ago

The login form should not use type="email" when LDAP has LDAPmailattribute set

Categories

(Bugzilla :: User Interface, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Bugzilla 5.0

People

(Reporter: colin.joy, Assigned: LpSolit)

References

Details

Attachments

(1 file, 1 obsolete file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/43.0.2357.130 Safari/537.36

Steps to reproduce:

1. Configure LDAP for authentication with an uid attribute other than e-mail.
The user login could be any alphanumeric string, not necessarily connected with the e-mail address.
2. Try to log in



Actual results:

The login form markup contains type="email", causing client side form validation to fail.


Expected results:

Client side validation should always match the configured server side validation / settings. The type=email attribute should not be set unless sure that the login is meant to confirm with the spec ( https://html.spec.whatwg.org/multipage/forms.html#valid-e-mail-address ).

Currently, client side validation is added "UNLESS Param('emailsuffix')", but that setting is orthogonal to the login validation. It is possible to use e-mails without an additional suffix and yet have alphanumeric logins that are not e-mail addresses.
see also: Bug 686333
See Also: → 686333
type="email" makes perfect sense because the backend code expects to get a valid email address.
Status: UNCONFIRMED → RESOLVED
Closed: 4 years ago
Resolution: --- → DUPLICATE
See Also: 686333
Duplicate of bug: 1164398
I ran into this bug this morning, and I think it's not a duplicate of bug 1164398 - that bug is about handling responses from active directory / LDAP, whereas this bug is to do with the form validation before LDAP queries are even made.

I have created a small patch to enable LDAP / Active Directory logins in our situation, which is characterised by having login names that a) are not email address or b) transformable into email addresses by appending a suffix.  For example, if we have a user called "Joe User", their active directory username is 'joeu' but their email address will be ju@domain.name.

The attached patch adds a new Boolean configuration parameter called "login_is_email", default value 1.  It has been tested in our environment but nowhere else.  In addition, I'm not familiar with Perl nor with Bugzilla's code base, so there may be other subtle issues I've overlooked.  Nevertheless I'm attaching the patch in the hope that it will be useful.
Reopening as I got no answer from the reporter of bug 1164398. I will fix this problem here.
Status: RESOLVED → REOPENED
Ever confirmed: true
Flags: blocking5.0.1?
Resolution: DUPLICATE → ---
Target Milestone: --- → Bugzilla 5.0
Assignee: ui → LpSolit
Status: REOPENED → ASSIGNED
Summary: Login Form uses type="email" when it should not → The login form should not use type="email" when LDAP has LDAPmailattribute set
Attached patch patch, v1Splinter Review
Colin and/or scholi, could you test this patch, please? If it fixes your problem, please set the feedback flag to +, else set it to -.
Attachment #8636685 - Flags: review?(dkl)
Attachment #8636685 - Flags: feedback?(scholi)
Attachment #8636685 - Flags: feedback?(colin.joy)
Attachment #8631368 - Attachment is obsolete: true
Comment on attachment 8636685 [details] [diff] [review]
patch, v1

Review of attachment 8636685 [details] [diff] [review]:
-----------------------------------------------------------------

thanks for addressing the issue.

verified change against release-5.0-stable.
works for our use case.
Attachment #8636685 - Flags: feedback?(colin.joy) → feedback+
I also tried this patch.
Indeed, it fixes the issue.
Comment on attachment 8636685 [details] [diff] [review]
patch, v1

Review of attachment 8636685 [details] [diff] [review]:
-----------------------------------------------------------------

Works as expected
Attachment #8636685 - Flags: feedback?(scholi) → feedback+
Comment on attachment 8636685 [details] [diff] [review]
patch, v1

gerv, this patch has already been tested by LDAP users. It just needs a format review. :)
Attachment #8636685 - Flags: review?(dkl) → review?(gerv)
(In reply to Frédéric Buclin from comment #9)
> gerv, this patch has already been tested by LDAP users. It just needs a
> format review. :)

s/format/formal/ :(
Comment on attachment 8636685 [details] [diff] [review]
patch, v1

Review of attachment 8636685 [details] [diff] [review]:
-----------------------------------------------------------------

<sigh> This is going to bitrot bug 218917 :-(

Gerv
Attachment #8636685 - Flags: review?(gerv) → review+
Flags: approval?
Flags: approval5.0?
Flags: blocking5.0.1?
Flags: blocking5.0.1+
Flags: approval?
Flags: approval5.0?
Flags: approval5.0+
Flags: approval+
To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   5d46fc8..baed571  master -> master

To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   ae716b2..a2ae154  5.0 -> 5.0
Status: ASSIGNED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
Duplicate of this bug: 1195594
You need to log in before you can comment on or make changes to this bug.