488841 - login form shown even if can_login is false for all enabled auth methods

Status: NEW

(Bugzilla :: User Accounts, defect)

Description

[Chet Hosey]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US) AppleWebKit/525.19 (KHTML, like Gecko) Chrome/1.0.154.53 Safari/525.19
Build Identifier: BUGZILLA-3_2-STABLE

I have implemented a custom login module to support SSO with my company's portal system. Users can not log in directly through Bugzilla, and my module reflects that by setting can_login to zero.

However, users are still directed to a login screen when they are not authenticated. This confuses the users. I believe that a message should be displayed if login is not possible using the current form.

I've written a patch to login.html.tmpl so that the form is only shown if the user would be able to log in (user.authorizer.can_login is true). Otherwise, a custom message is shown. My patch also touches auth.html.tmpl and Auth.pm to allow configuration of the custom message.

In total, the following files are touched:

Bugzilla/Config/Auth.pm
template/en/default/account/auth/login.html.tmpl
template/en/default/admin/params/auth.html.tmpl

This was developed against BUGZILLA-3_2-STABLE, but the files match the versions currently in HEAD.

Reproducible: Always

Steps to Reproduce:
This is only exposed when no authentication module supports direct login. Without a custom authentication module in place, it could be simulated by setting the following line within Bugzilla/Auth/Login/CGI.pm:

use constant can_login => 0;

and accessing a resource which requires login. I would be willing to describe my custom module in more detail privately if necessary, however this issue is not specific to it.
Actual Results:  
A login form is shown, which cannot be used to authenticate.

Expected Results:  
A message should be displayed to describe the required login steps.

Comment 1

[Chet Hosey]

Attachment: patch to implement custom login text

This patch is against a checkout of HEAD on 2009/04/17 at 10:00pm EST. I have used runtests.pl with no errors or warnings. The output has been verified against http://validator.w3.org/.

Comment 2

[Max Kanat-Alexander]

Comment on attachment 373311 [details] [diff] [review]
patch to implement custom login text

What I'd like to see is just a fix for the issue. This patch both fixes the issue and adds a new parameter, which should be a different bug. I'm sure that we want the bug fix, I'm not sure that we want that parameter.

Comment 3

[Chet Hosey]

Thank you for your feedback.

I'll prepare a patch that addresses the issue specifically with a default message and resubmit without the parameter.

Comment 4

[Chet Hosey]

Attachment: login.html.tmpl: show form only if login is supported by an auth module

As suggested by Max, this patch is limited to hiding the login form when no authentication module supports direct login.

A simple message is displayed in its place ("You are not logged in to [% terms.Bugzilla %].").

Comment 5

[Max Kanat-Alexander]

Comment on attachment 374136 [details] [diff] [review]
login.html.tmpl: show form only if login is supported by an auth module

>+[% IF user.authorizer.can_login %]
>+  [% PROCESS global/header.html.tmpl
>+    title = "Log in to $terms.Bugzilla",
>+    onload = "document.forms['login'].Bugzilla_login.focus()"
>+  %]
>+[% ELSE %]
>+  [% PROCESS global/header.html.tmpl
>+    title = "Log in to $terms.Bugzilla",
>+  %]
>+[% END %]

  You don't need to do that--you can just "SET onload" only if user.authorizer.can_login.



  You know, actually, I think instead of all of this what we should do is just throw an error if the user tries to access a LOGIN_REQUIRED page without being logged in, if there are no methods that can login. I'm thinking maybe somewhere in Bugzilla::Auth::login.