View Issue Details

IDProjectCategoryView StatusLast Update
0004465mantisbtsecuritypublic2014-09-23 18:05
Reporterhugopedersen Assigned Todregad  
PrioritynormalSeverityfeatureReproducibilityalways
Status closedResolutionfixed 
PlatformMantisOSBugtrackerOS Version0.19.0rc1
Target Version1.2.10Fixed in Version1.2.10 
Summary0004465: Turn off 'save login' feature
Description

I think it would be nice to be able to configure that the 'save login' feature should be enabled or disabled.
I have to edit the login_page.php when there is a new version to remove this option since I have computers where different users with different access levels uses Mantis. But if some one has ticked the 'save login' then all use this name and access level.

Tagspatch
Attached Files
login_page.1.52.4.1.6.2+.php.patch (689 bytes)   
--- login_page.php	24 Jul 2006 01:22:25 -0000	1.52.4.1.6.2
+++ login_page.php	31 Jan 2007 18:08:50 -0000
@@ -97,14 +97,19 @@
 		<input type="password" name="password" size="16" maxlength="32" />
 	</td>
 </tr>
-<tr class="row-1">
-	<td class="category">
-		<?php echo lang_get( 'save_login' ) ?>
-	</td>
-	<td>
-		<input type="checkbox" name="perm_login" />
-	</td>
-</tr>
+<?php
+	if ( ON == config_get( 'allow_save_login' ) ) {
+    ?>  <tr class="row-1">
+    	    <td class="category">
+    		    <?php echo lang_get( 'save_login' ) ?>
+    		    </td>
+    		<td>
+    		    <input type="checkbox" name="perm_login" />
+    		</td>
+		</tr>
+	<?php
+	}
+?>
login_page.1.52.4.1.6.2.php (6,887 bytes)   
<?php
	# Mantis - a php based bugtracking system
	# Copyright (C) 2000 - 2002  Kenzaburo Ito - kenito@300baud.org
	# Copyright (C) 2002 - 2004  Mantis Team   - mantisbt-dev@lists.sourceforge.net
	# This program is distributed under the terms and conditions of the GPL
	# See the README and LICENSE files for details

	# --------------------------------------------------------
	# $Id: login_page.php,v 1.52.4.1.6.2 2006/07/24 01:22:25 thraxisp Exp $
	# --------------------------------------------------------

	# Login page POSTs results to login.php
	# Check to see if the user is already logged in

	require_once( 'core.php' );

	if ( auth_is_user_authenticated() && !current_user_is_anonymous() ) {
		print_header_redirect( config_get( 'default_home_page' ) );
	}

	$f_error		= gpc_get_bool( 'error' );
	$f_cookie_error	= gpc_get_bool( 'cookie_error' );
	$f_return		= gpc_get_string( 'return', '' );

	# Check for HTTP_AUTH. HTTP_AUTH is handled in login.php

	if ( HTTP_AUTH == config_get( 'login_method' ) ) {
		$t_uri = "login.php";

		if ( !$f_return && ON == config_get( 'allow_anonymous_login' ) ) {
			$t_uri = "login_anon.php";
		}

		if ( $f_return ) {
			$t_uri .= "?return=" . urlencode( $f_return );
		}

		print_header_redirect( $t_uri );
		exit;
	}

	html_page_top1();
	html_page_top2a();

	echo '<br /><div align="center">';

	# Display short greeting message
	# echo lang_get( 'login_page_info' ) . '<br />';

	# Only echo error message if error variable is set
	if ( $f_error ) {
		echo '<font color="red">' . lang_get( 'login_error' ) . '</font>';
	}
	if ( $f_cookie_error ) {
		echo lang_get( 'login_cookies_disabled' ) . '<br />';
	}

	echo '</div>';
?>

<!-- Login Form BEGIN -->
<br />
<div align="center">
<form name="login_form" method="post" action="login.php">
<table class="width50" cellspacing="1">
<tr>
	<td class="form-title">
		<?php
			if ( !is_blank( $f_return ) ) {
			?>
				<input type="hidden" name="return" value="<?php echo string_html_specialchars( $f_return ) ?>" />
				<?php
			}
			echo lang_get( 'login_title' ) ?>
	</td>
	<td class="right">
	<?php
		if ( ON == config_get( 'allow_anonymous_login' ) ) {
			print_bracket_link( 'login_anon.php', lang_get( 'login_anonymously' ) );
		}
	?>
	</td>
</tr>
<tr class="row-1">
	<td class="category" width="25%">
		<?php echo lang_get( 'username' ) ?>
	</td>
	<td width="75%">
		<input type="text" name="username" size="32" maxlength="32" />
	</td>
</tr>
<tr class="row-2">
	<td class="category">
		<?php echo lang_get( 'password' ) ?>
	</td>
	<td>
		<input type="password" name="password" size="16" maxlength="32" />
	</td>
</tr>
<?php
	if ( ON == config_get( 'allow_save_login' ) ) {
    ?>  <tr class="row-1">
    	    <td class="category">
    		    <?php echo lang_get( 'save_login' ) ?>
    		    </td>
    		<td>
    		    <input type="checkbox" name="perm_login" />
    		</td>
		</tr>
	<?php
	}
?>
<tr>
	<td class="center" colspan="2">
		<input type="submit" class="button" value="<?php echo lang_get( 'login_button' ) ?>" />
	</td>
</tr>
</table>
</form>
</div>

<?php
	PRINT '<br /><div align="center">';
	print_signup_link();
	PRINT '&nbsp;';
	print_lost_password_link();
	PRINT '</div>';

	#
	# Do some checks to warn administrators of possible security holes.
	# Since this is considered part of the admin-checks, the strings are not translated.
	#

	# Warning, if plain passwords are selected
	if ( config_get( 'login_method' ) === PLAIN ) {
		echo '<div class="warning" align="center">';
		echo '<p><font color="red"><strong>WARNING:</strong> Plain password authentication is used, this will expose your passwords to administrators.</font></p>';
		echo '</div>';
	}

	# Generate a warning if administrator/root is valid.
	$t_admin_user_id = user_get_id_by_name( 'administrator' );
	if ( $t_admin_user_id !== false ) {
		if ( user_is_enabled( $t_admin_user_id ) && auth_does_password_match( $t_admin_user_id, 'root' ) ) {
			echo '<div class="warning" align="center">';
			echo '<p><font color="red"><strong>WARNING:</strong> You should disable the default "administrator" account or change its password.</font></p>';
			echo '</div>';
		}
	}

	# Check if the admin directory is available and is readable.
	$t_admin_dir = dirname( __FILE__ ) . DIRECTORY_SEPARATOR . 'admin' . DIRECTORY_SEPARATOR;
	if ( is_dir( $t_admin_dir ) && is_readable( $t_admin_dir ) ) {
		echo '<div class="warning" align="center">', "\n";
		echo '<p><font color="red"><strong>WARNING:</strong> Admin directory should be removed.</font></p>', "\n";
		echo '</div>', "\n";

		# since admin directory and db_upgrade lists are available check for missing db upgrades
		# Check for db upgrade for versions < 1.0.0 using old upgrader
		$t_db_version = config_get( 'database_version' , 0 );
		# if db version is 0, we haven't moved to new installer.
		if ( $t_db_version == 0 ) {
			if ( db_table_exists( config_get( 'mantis_upgrade_table' ) ) ) {
				$query = "SELECT COUNT(*) from " . config_get( 'mantis_upgrade_table' ) . ";";
				$result = db_query( $query );
				if ( db_num_rows( $result ) < 1 ) {
					$t_upgrade_count = 0;
				} else {
					$t_upgrade_count = (int)db_result( $result );
				}
			} else {
				$t_upgrade_count = 0;
			}

			if ( $t_upgrade_count > 0 ) { # table exists, check for number of updates
				require_once( 'admin/upgrade_inc.php' );
				$t_upgrades_reqd = $upgrade_set->count_items();
			} else {
				$t_upgrades_reqd = 1000; # arbitrarily large number to force an upgrade
			}

			if ( ( $t_upgrade_count != $t_upgrades_reqd ) &&
					( $t_upgrade_count != ( $t_upgrades_reqd + 10 ) ) ) { # there are 10 optional data escaping fixes that may be present
				echo '<div class="warning" align="center">';
				echo '<p><font color="red"><strong>WARNING:</strong> The database structure may be out of date. Please upgrade <a href="admin/upgrade.php">here</a> before logging in.</font></p>';
				echo '</div>';
			}
		}

		# Check for db upgrade for versions > 1.0.0 using new installer and schema
		require_once( 'admin/schema.php' );
		$t_upgrades_reqd = sizeof( $upgrade ) - 1;

		if ( ( 0 < $t_db_version ) &&
				( $t_db_version != $t_upgrades_reqd ) ) {
			echo '<div class="warning" align="center">';
			echo '<p><font color="red"><strong>WARNING:</strong> The database structure may be out of date. Please upgrade <a href="admin/install.php">here</a> before logging in.</font></p>';
			echo '</div>';
		}
	}
?>

<!-- Autofocus JS -->
<?php if ( ON == config_get( 'use_javascript' ) ) { ?>
<script type="text/javascript" language="JavaScript">
<!--
	window.document.login_form.username.focus();
// -->
</script>
<?php } ?>

<?php html_page_bottom1a( __FILE__ ) ?>
login_page.1.52.4.1.6.2.php (6,887 bytes)   
0001-Fix-4465-Add-config-to-disable-save-login-feature.patch (1,964 bytes)   
From 7f3d743d2c606612b601bf28e40480f7917668d3 Mon Sep 17 00:00:00 2001
From: Damien Regad <damien.regad@merckgroup.com>
Date: Fri, 23 Mar 2012 15:32:54 +0100
Subject: [PATCH] Fix #4465: Add config to disable 'save login' feature

To increase security, the administrator may want to prevent users from
using a 'permanent' cookie, thus forcing them to authenticate each time
they start a new session.

The new config option 'g_allow_permanent_cookie' enables this.
---
 config_defaults_inc.php |    9 +++++++++
 login_page.php          |    9 ++++++++-
 2 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/config_defaults_inc.php b/config_defaults_inc.php
index 8c5df3d..f03f1f2 100644
--- a/config_defaults_inc.php
+++ b/config_defaults_inc.php
@@ -304,6 +304,15 @@
 	 */
 	$g_max_lost_password_in_progress_count = 3;
 
+	/**
+	 * Allow users to opt for a 'permanent' cookie when logging in
+	 * Controls the display of the 'Remember my login in this browser' checkbox
+	 * on the login page
+	 * @see $g_cookie_time_length
+	 * @global int $g_allow_permanent_cookie
+	 */
+	$g_allow_permanent_cookie = ON;
+
 	/***************************
 	 * MantisBT Email Settings *
 	 ***************************/
diff --git a/login_page.php b/login_page.php
index 9b5edf0..c7b65dc 100644
--- a/login_page.php
+++ b/login_page.php
@@ -139,6 +139,9 @@
 		<input type="password" name="password" size="32" maxlength="<?php echo auth_get_password_max_size(); ?>" />
 	</td>
 </tr>
+<?php
+	if( ON == config_get( 'allow_permanent_cookie' ) ) {
+?>
 <tr class="row-1">
 	<td class="category">
 		<?php echo lang_get( 'save_login' ) ?>
@@ -147,7 +150,11 @@
 	<input type="checkbox" name="perm_login" <?php echo ( $f_perm_login ? 'checked="checked" ' : '' ) ?>/>
 	</td>
 </tr>
-<?php if ( $t_session_validation ) { ?>
+<?php
+	}
+
+	if ( $t_session_validation ) {
+?>
 <tr class="row-2">
 	<td class="category">
 		<?php echo lang_get( 'secure_session' ) ?>
-- 
1.7.5.4

Relationships

related to 0015721 closedgrangeway Functionality to consider porting to master-2.0.x 
has duplicate 0010874 closedvboctor configurations don't work in config_inc.php 
has duplicate 0014069 closedatrol Turn off save login option (g_allow_save_login) 

Activities

jlatour

jlatour

2004-09-02 14:45

reporter   ~0007405

I think this problem also exists for installations that are more widely used. Would just adding a warning help?

hugopedersen

hugopedersen

2004-09-02 23:59

reporter   ~0007417

I have done some modification on my own setup that allows me to set a config option to disable this option. I have less than no experiance in PHP but I managed to get it working.

It may be a solution to warn the user when he/she has selected the option to save the password.

ape

ape

2006-12-14 18:24

reporter   ~0013830

Yup, we could definitely make use of that feature here. We have a large user-base and use the LDAP authentication feature, and we've love to just tighten up the login security just that little bit more.

I'll see if I can knock together a patch to make use of a '$g_allow_save_login' config option or something like that.

ape

ape

2007-03-27 12:26

reporter   ~0014254

I've attached the modified version of login_page.php rev 1.52.4.1.6.2 (from Mantis version 1.0.6) and the corresponding patch, which I've been using successfully here for a few months now.

Very simple fix; all it needs is something like the following in your config_inc file.

--- save login ----------------

Setting to disable the 'save login' feature.

$g_allow_save_login = OFF;

Please give it a try and let me know if it works for you.

dregad

dregad

2012-03-23 10:42

developer   ~0031515

Please test attached patch.

atrol

atrol

2012-03-23 11:55

developer   ~0031516

@dregad, if you want to introduce this option you should add this also to $g_global_settings in config_defaults_inc.php.

dregad

dregad

2012-03-27 08:54

developer   ~0031545

Since it's a global setting relating to security, I guess your remark makes sense. I'll add it.

atrol

atrol

2012-03-27 10:53

developer   ~0031548

@dregad, have also a look at function config_is_private in config_api.php
This seems to be needed because our access checks for SOAP are not well implemented, see 0012328

dregad

dregad

2012-03-27 11:28

developer   ~0031550

I am not sure. Does it really make sense to deny the SOAP API access to the contents of this new variable? It does not in itself contain any sensitive information, just the fact that users are allowed to use the "permanent" cookie. You can easily obtain the same information just by displaying the login page.

atrol

atrol

2012-03-28 05:13

developer   ~0031554

I am not sure. Does it really make sense to deny the SOAP API access to the contents of this new variable?
You can easily obtain the same information just by displaying the login page.
You are right, I am also not sure.
Nearly the same can be said for session_validation which is included in function config_is_private.
There is no real rule for this decision.

Another topic:
The patch does not disallow the option to set the permanent cookie. It suppresses just the display of the check box.
By manipulation the page in browser you would still be able to save the login.
If you really want to be sure that no permanent cookie can be set if $g_allow_permanent_cookie = OFF; you have to add another check in function auth_set_cookies.

dregad

dregad

2012-03-29 07:37

developer   ~0031558

You are right, I am also not sure.

I guess we should bounce that off grangeway (as original author of config_is_private function) and rombert. I'll write to the mailing list later as time allows.

Another topic

Good catch, thanks ! Fixed in https://github.com/dregad/mantisbt/tree/fix-4465

dregad

dregad

2012-03-30 17:19

developer   ~0031577

Feedback from grangeway on config_is_private:

22:43.33 <Paul24> I think we'd concluded stuff that is a security issue
22:43.35 <Paul24> e.g. passwords
22:43.45 <Paul24> i.e. stuff you shouldn't display in web interface
22:44.33 <Paul24> i'm still not sure I like the fact we got new features going into 1.2, 1.3 and 'next' and my 2.0 branch
22:46.01 <Paul24> so yea private was basically to hide a) paths b) usernames c) passwords d) host/ip's of other servers
22:46.12 <Paul24> so not what your proposing doing

Which confirms what I was thinking. So I'll just take the opportunity to better document the function in the code (PHPdoc) and leave it at that unless someone objects.

dregad

dregad

2012-03-30 20:47

developer   ~0031582

Changes pushed to github.

grangeway

grangeway

2013-04-05 17:57

reporter   ~0036295

Marking as 'acknowledged' not resolved/closed to track that change gets ported to master-2.0.x branch

Related Changesets

MantisBT: master 766a7d38

2012-03-23 03:32

dregad


Details Diff
Fix 0004465: Add config to disable 'save login' feature

To increase security, the administrator may want to prevent users from
using a 'permanent' cookie, thus forcing them to authenticate each time
they start a new session.

The new config option 'g_allow_permanent_cookie' enables this.

Porting to master branch of the following 1.2.x commits:
- 56986173ea3a2da12345acecb11afc7ab374696a
- 6a9f3a810dca575cc230808b780a80946acdcb73
- 568ee14305c4c6935b18137032d13de097841286
Affected Issues
0004465
mod - config_defaults_inc.php Diff File
mod - login.php Diff File
mod - login_page.php Diff File

MantisBT: master-1.2.x 56986173

2012-03-23 03:32

dregad


Details Diff
Fix 0004465: Add config to disable 'save login' feature

To increase security, the administrator may want to prevent users from
using a 'permanent' cookie, thus forcing them to authenticate each time
they start a new session.

The new config option 'g_allow_permanent_cookie' enables this.
Affected Issues
0004465
mod - config_defaults_inc.php Diff File
mod - login_page.php Diff File

MantisBT: master-1.2.x 6a9f3a81

2012-03-27 01:44

dregad


Details Diff
Add 'allow_permanent_cookie' to g_global_settings

Also reflowed the array definition to avoid long lines

Fixes 0004465
Affected Issues
0004465
mod - config_defaults_inc.php Diff File

MantisBT: master-1.2.x 568ee143

2012-03-28 23:24

dregad


Details Diff
Prevent setting permanent cookie using hand-crafted login.php

This commit prevents hand-crafted calls to login.php from setting a
permanent cookie when its use is disabled via allow_permanent_cookie
setting.

Thanks to Roland Becker for catching this.

Fixes 0004465
Affected Issues
0004465
mod - login.php Diff File