View Issue Details

IDProjectCategoryView StatusLast Update
0011351mantisbtadministrationpublic2014-12-22 08:24
Reporterwgodin Assigned Todhx  
PrioritynormalSeveritymajorReproducibilityalways
Status closedResolutionfixed 
Product Versiongit trunk 
Target Version1.2.4Fixed in Version1.2.4 
Summary0011351: User Real Name and E-Mail values deleted
Description

Having set authentication to LDAP, updating any user which "Real Name" and "Email" values have been pulled from LDAP server deletes those values.

Steps To Reproduce

Set LDAP login method in config_inc.php : $g_login_method = LDAP;
Select "Manage Users" ;
Select a user which "Real Name" or "Email" values have been pulled from LDAP server and set to some non blank values ;
Change any of "access level" or "Enabled" or "Protected" field values.
Click "Update User".

Previously set "Real Name" and "Email" fields are lost.

Tagspatch
Attached Files
Fix-11351.patch (4,545 bytes)   
From ff0dc92f1810612680db88aa2156a3b5922e2958 Mon Sep 17 00:00:00 2001
From: Damien Regad <damien.regad@merckserono.net>
Date: Wed, 20 Oct 2010 18:07:45 +0200
Subject: [PATCH] Fix #11351: Do not delete email or realname when editing user with LDAP

When connecting to Mantis with LDAP and either use_ldap_email or
use_ldap_realname = ON, that field is set to blank when the user edits
their profile (account_page.php). The same happens when either or both
of the above options are ON and the administrator updates a user from
manage_user_edit_page.php.
---
 account_page.php          |   53 +++++++++++++++++++++++---------------------
 manage_user_edit_page.php |   26 ++++++++++++++++------
 2 files changed, 47 insertions(+), 32 deletions(-)

diff --git a/account_page.php b/account_page.php
index 2c9790e..bc9058b 100644
--- a/account_page.php
+++ b/account_page.php
@@ -176,20 +176,6 @@
 <?php
 } // End LDAP conditional
 
-if ( $t_ldap && ON == config_get( 'use_ldap_email' ) ) { ?> <!-- With LDAP Email-->
-
-	<!-- Email -->
-	<tr <?php echo helper_alternate_class() ?>>
-		<td class="category">
-			<?php echo lang_get( 'email' ) ?>
-		</td>
-		<td>
-			<?php echo $u_email ?>
-		</td>
-	</tr>
-
-<?php } else { ?> <!-- Without LDAP Email -->
-
 	<!-- Email -->
 	<tr <?php echo helper_alternate_class() ?>>
 		<td class="category">
@@ -197,13 +183,22 @@ if ( $t_ldap && ON == config_get( 'use_ldap_email' ) ) { ?> <!-- With LDAP Email
 		</td>
 		<td>
 			<?php
-				$t_show_update_button = true;
-				print_email_input( 'email', $u_email );
+				// With LDAP
+				if ( $t_ldap && ON == config_get( 'use_ldap_email' ) ) { 
+					echo $u_email
+			?>	
+					<input type="hidden" name="email" value="<?php echo string_attribute( $u_email ) ?>" />
+			<?php 
+				} 
+				// Without LDAP
+				else { 
+					$t_show_update_button = true;
+					print_email_input( 'email', $u_email );
+				}	
 			?>
 		</td>
 	</tr>
 
-<?php } ?> <!-- End LDAP Email conditional -->
 
 	<!-- Realname -->
 	<tr <?php echo helper_alternate_class() ?> valign="top">
@@ -211,14 +206,22 @@ if ( $t_ldap && ON == config_get( 'use_ldap_email' ) ) { ?> <!-- With LDAP Email
 			<?php echo lang_get( 'realname' ) ?>
 		</td>
 		<td>
-<?php
-if ( $t_ldap && ON == config_get( 'use_ldap_realname' ) ) {
-	echo string_display( ldap_realname_from_username( $u_username ) );
-} else {
-	$t_show_update_button = true;
-?>
-			<input type="text" size="32" maxlength="<?php echo REALLEN;?>" name="realname" value="<?php echo string_attribute( $u_realname ) ?>" />
-<?php } ?>
+			<?php
+				// With LDAP
+				if ( $t_ldap && ON == config_get( 'use_ldap_realname' ) ) {
+					echo string_display( ldap_realname_from_username( $u_username ) );
+			?>
+					<input type="hidden" name="realname" value="<?php echo string_attribute( ldap_realname_from_username( $u_username ) ) ?>" />
+			<?php
+				} 
+				// Without LDAP
+				else {
+					$t_show_update_button = true;
+			?>
+					<input type="text" size="32" maxlength="<?php echo REALLEN;?>" name="realname" value="<?php echo string_attribute( $u_realname ) ?>" />
+			<?php 
+				}
+			?>
 		</td>
 	</tr>
 
diff --git a/manage_user_edit_page.php b/manage_user_edit_page.php
index 4ac18d4..2c66b72 100644
--- a/manage_user_edit_page.php
+++ b/manage_user_edit_page.php
@@ -88,12 +88,18 @@
 	</td>
 	<td width="70%">
 		<?php
-			if ( !$t_ldap || config_get( 'use_ldap_realname' ) == OFF ) {
+			// With LDAP
+			if ( $t_ldap && ON == config_get( 'use_ldap_realname' ) ) {
+				echo string_display( user_get_realname( $f_user_id ) );
+		?>
+				<input type="hidden" name="realname" value="<?php echo string_attribute( user_get_realname( $f_user_id ) ) ?>" />
+		<?php
+			} 
+			// Without LDAP
+			else {
 		?>
 				<input type="text" size="16" maxlength="<?php echo REALLEN;?>" name="realname" value="<?php echo string_attribute( $t_user['realname'] ) ?>" />
 		<?php
-			} else {
-				echo string_display( user_get_realname( $f_user_id ) );
 			}
 		?>
 	</td>
@@ -106,11 +112,17 @@
 	</td>
 	<td>
 		<?php
-			if ( !$t_ldap || config_get( 'use_ldap_email' ) == OFF ) {
-				print_email_input( 'email', $t_user['email'] );
-			} else {
+			// With LDAP
+			if ( $t_ldap && ON == config_get( 'use_ldap_email' ) ) { 
 				echo string_display( user_get_email( $f_user_id ) );
-			}
+		?>
+				<input type="hidden" name="email" value="<?php echo user_get_email( $f_user_id ) ?>" />
+		<?php 
+			} 
+			// Without LDAP
+			else { 
+				print_email_input( 'email', $t_user['email'] );
+			}	
 		?>
 	</td>
 </tr>
-- 
1.7.1

Fix-11351.patch (4,545 bytes)   

Relationships

has duplicate 0012567 closedatrol Updating LDAP user overwrites email address and real name until next login 
has duplicate 0012950 closeddhx DB values are deleted when using LDAP 

Activities

gordon1986

gordon1986

2010-06-09 16:14

reporter   ~0025789

Last edited: 2010-06-09 16:15

Following patch maintains email and realname fields if LDAP authentication is used:
--- mantisbt-1.2.1/manage_user_edit_page.php 2010-04-23 14:28:34.000000000 -0400
+++ gmr/manage_user_edit_page.php 2010-06-09 16:11:37.000000000 -0400
@@ -93,6 +93,9 @@
<input type="text" size="16" maxlength="<?php echo REALLEN;?>" name="realname" value="<?php echo string_attribute( $t_user['realname'] ) ?>" />
<?php
} else {

  • ?>
  • <input type="hidden" name="realname" value="<?php echo string_attribute( user_get_realname( $f_user_id ) ) ?>" />
  • <?php
    echo string_display( user_get_realname( $f_user_id ) );
    }
    ?>
    @@ -109,12 +112,15 @@
    if ( !$t_ldap || config_get( 'use_ldap_email' ) == OFF ) {
    print_email_input( 'email', $t_user['email'] );
    } else {
  • ?>
  • <input type="hidden" name="email" value="<?php echo string_attribute( user_get_email( $f_user_id ) ) ?>" />
  • <?php
    echo string_display( user_get_email( $f_user_id ) );
    }
    ?>
    </td>
    </tr>
  • <!-- Access Level -->
    <tr <?php echo helper_alternate_class() ?>>
    <td class="category">

dregad

dregad

2010-09-27 07:19

developer   ~0026868

Last edited: 2010-09-27 08:03

As part of my testing for 0012402, I noticed similar behavior with the user's "My Account" page.

To reproduce:

  • set $g_use_ldap_email = OFF; in config_inc.php
  • login and navigate to account_page.php
  • Make no changes, just click "Update User"

==> Mantis displays:
Real name successfully updated
Operation successful.
[ Proceed ]

The realname is now blank, until the user's next login (when it is reset to its LDAP value)

This is with 1.2.1.

Following gordon1986's logic in the previous note, I added hidden fields for LDAP-maintained values in account_page.php, which seem to resolve the problem for me.

--- ../1.2.1/mantisbt/account_page.php 2010-08-25 18:28:22.000000000 +0200
+++ account_page.php 2010-09-27 13:56:01.000000000 +0200
@@ -185,6 +185,7 @@
</td>
<td>
<?php echo $u_email ?>

  • <input type="hidden" name="email" value="<?php echo string_attribute( $u_email ) ?>" />
    </td>
    </tr>

@@ -214,6 +215,9 @@
<?php
if ( $t_ldap && ON == config_get( 'use_ldap_realname' ) ) {
echo string_display( ldap_realname_from_username( $u_username ) );
+?>

  • <input type="hidden" name="realname" value="<?php echo string_attribute( ldap_realname_from_username( $u_username ) ) ?>" />
    +<?php
    } else {
    $t_show_update_button = true;
    ?>
dregad

dregad

2010-10-13 10:16

developer   ~0027019

Related to 0005175

dregad

dregad

2010-10-22 06:31

developer   ~0027122

Yaaay !! My first "official" code contribution to Mantis :-)

Thanks David

dhx

dhx

2010-10-22 07:36

reporter   ~0027123

Thanks Damien, good work with the patch! It's great to have patches from new contributors.

I have supplied some feedback on your patch and the changes I've made to the developer mailing list. Hopefully there is some assistance there to help you with any future patches you contribute to MantisBT.

I haven't tested these commits with LDAP enabled (only without it enabled). I would therefore appreciate it if someone could run some tests to see if things work as expected with LDAP enabled.

Just reopen this bug if the problem isn't fixed.

Thanks!

dregad

dregad

2010-11-01 15:34

developer   ~0027210

I confirm successful completion of following tests, with $g_login_method = LDAP :

  1. $g_use_ldap_email = ON; $g_use_ldap_realname = OFF;
    ==> realname is not deleted in manage_user_edit_page (PASS)
    ==> realname is not deleted in account_page (PASS)
  2. $g_use_ldap_email = OFF; $g_use_ldap_realname = ON;
    ==> email is not deleted in manage_user_edit_page (PASS)
    ==> email is not deleted in account_page (PASS)
  3. $g_use_ldap_email = ON; $g_use_ldap_realname = ON;
    ==> email and realname are not deleted in manage_user_edit_page (PASS)
    ==> N/A for account_page (no update button is available)

Damien

Related Changesets

MantisBT: master 1aa11780

2010-10-20 12:07

Damien Regad

Committer: dhx


Details Diff
Fix 0011351: Do not delete email or realname when editing user with LDAP

When connecting to Mantis with LDAP and either use_ldap_email or
use_ldap_realname = ON, that field is set to blank when the user edits
their profile (account_page.php). The same happens when either or both
of the above options are ON and the administrator updates a user from
manage_user_edit_page.php.

The original patch from Damien was updated to fix a few minor bugs and
more importantly, to resolve a number of potential XSS vulnerabilities.

Co-contributed-by: David Hicks <hickseydr@optusnet.com.au>
Signed-off-by: David Hicks <hickseydr@optusnet.com.au>
Affected Issues
0011351
mod - core/print_api.php Diff File
mod - account_page.php Diff File
mod - manage_user_edit_page.php Diff File

MantisBT: master-1.2.x 99e7eedc

2010-10-20 12:07

Damien Regad

Committer: dhx


Details Diff
Fix 0011351: Do not delete email or realname when editing user with LDAP

When connecting to Mantis with LDAP and either use_ldap_email or
use_ldap_realname = ON, that field is set to blank when the user edits
their profile (account_page.php). The same happens when either or both
of the above options are ON and the administrator updates a user from
manage_user_edit_page.php.

The original patch from Damien was updated to fix a few minor bugs and
more importantly, to resolve a number of potential XSS vulnerabilities.

Co-contributed-by: David Hicks <hickseydr@optusnet.com.au>
Signed-off-by: David Hicks <hickseydr@optusnet.com.au>
Affected Issues
0011351
mod - manage_user_edit_page.php Diff File
mod - core/print_api.php Diff File
mod - account_page.php Diff File

MantisBT: master-1.2.x 7672ca3d

2010-10-22 06:59

dhx


Details Diff
Fix 0011351: Real name and email should not be updated via GPC (LDAP)

When LDAP is being used for retrieving the user real name and/or email
address we should not provide any way for MantisBT forms to update these
fields manually via GPC parameters to account_update and
manage_user_update. Thus we don't need (and never wanted to) send hidden
fields with forms containing the current LDAP details. A user could
maliciously change these values by crafting their own HTTP POST queries
to the server.
Affected Issues
0011351
mod - manage_user_update.php Diff File
mod - account_page.php Diff File
mod - manage_user_edit_page.php Diff File
mod - account_update.php Diff File

MantisBT: master 71ad8c6f

2010-10-22 06:59

dhx


Details Diff
Fix 0011351: Real name and email should not be updated via GPC (LDAP)

When LDAP is being used for retrieving the user real name and/or email
address we should not provide any way for MantisBT forms to update these
fields manually via GPC parameters to account_update and
manage_user_update. Thus we don't need (and never wanted to) send hidden
fields with forms containing the current LDAP details. A user could
maliciously change these values by crafting their own HTTP POST queries
to the server.
Affected Issues
0011351
mod - manage_user_edit_page.php Diff File
mod - account_page.php Diff File
mod - account_update.php Diff File
mod - manage_user_update.php Diff File

MantisBT: master-1.2.x 5f24068e

2010-10-22 07:19

dhx


Details Diff
Issue 0011351: Fix variable names for $t_email

Fixes commit 7672ca3d7f00 whereby some old variable names were not
updated correctly. We're now using $t_email for comparisons instead of
the direct user-supplied $f_email.
Affected Issues
0011351
mod - manage_user_update.php Diff File