View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0013445 | mantisbt | api soap | public | 2011-10-27 03:54 | 2013-04-06 08:12 |
Reporter | vboctor | Assigned To | vboctor | ||
Priority | normal | Severity | feature | Reproducibility | have not tried |
Status | closed | Resolution | fixed | ||
Product Version | 1.2.8 | ||||
Target Version | 1.2.12 | Fixed in Version | 1.2.12 | ||
Summary | 0013445: Add mc_login() for login and to return user data | ||||
Description | Currently there is no way to get user data from a username. The user data can only be retrieved as part of issue or not information. Such data includes id, username, real name and email. The user information should include the same information, but can be extended to include other more detailed information as needed. For example, global access level, accessible projects, access for each of these projects, etc. The access level information wil be not as important, as we implement our access check apis which profiles a list of allowed actions on an issue or a project for a specific user. If the user is an administrator, it should be possible to retrieve information for other users. Otherwise, we can limit this API to only retrieve information about the current logged in user. UserData mc_user_get( $p_username, $p_password, $p_username ); UserData
| ||||
Tags | No tags attached. | ||||
Attached Files | mc_login.diff (5,734 bytes)
diff --git a/api/soap/mantisconnect.php b/api/soap/mantisconnect.php index 819dced..adea6a7 100644 --- a/api/soap/mantisconnect.php +++ b/api/soap/mantisconnect.php @@ -94,6 +94,20 @@ $l_oServer->wsdl->addComplexType( ) ); +### UserData +$l_oServer->wsdl->addComplexType( + 'UserData', + 'complexType', + 'struct', + 'all', + '', + array( + 'account_data' => array( 'name' => 'account_data', 'type' => 'tns:AccountData', 'minOccurs' => '0'), + 'access_level' => array( 'name' => 'global_access_level', 'type' => 'xsd:integer', 'minOccurs' => '0'), + 'timezone' => array( 'name' => 'timezone', 'type' => 'xsd:string', 'minOccurs' => '0'), + ) +); + ### AccountDataArray $l_oServer->wsdl->addComplexType( 'AccountDataArray', @@ -664,6 +678,20 @@ $l_oServer->register( 'mc_version', ### PUBLIC METHODS (defined in mc_enum_api.php) ### +### mc_login +$l_oServer->register( 'mc_login', + array( + 'username' => 'xsd:string', + 'password' => 'xsd:string' + ), + array( + 'return' => 'tns:UserData' + ), + $t_namespace, + false, false, false, + 'Log the user in and get their information.' +); + ### mc_enum_status $l_oServer->register( 'mc_enum_status', array( diff --git a/api/soap/mc_api.php b/api/soap/mc_api.php index 6777811..9ec61a7 100644 --- a/api/soap/mc_api.php +++ b/api/soap/mc_api.php @@ -16,7 +16,37 @@ function mc_version() { return MANTIS_VERSION; } -# Checks if MantisBT installation is marked as offline by the administrator. +/** + * Attempts to login the user. + * If logged in successfully, return user information. + * If failed to login in, then throw a fault. + */ +function mc_login( $p_username, $p_password ) { + $t_user_id = mci_check_login( $p_username, $p_password ); + if ( $t_user_id === false ) { + return mci_soap_fault_login_failed(); + } + + return mci_user_get( $p_username, $p_password, $t_user_id ); +} + +/** + * Given an id, this method returns the user. + * When calling this method make sure that the caller has the right to retrieve + * information about the target user. + */ +function mci_user_get( $p_username, $p_password, $p_user_id ) { + $t_user_data = array(); + + // if user doesn't exist, then mci_account_get_array_by_id() will throw. + $t_user_data['account_data'] = mci_account_get_array_by_id( $p_user_id ); + $t_user_data['access_level'] = access_get_global_level( $p_user_id ); + $t_user_data['timezone'] = user_pref_get_pref( $p_user_id, 'timezone' ); + + return $t_user_data; +} + +# access_ if MantisBT installation is marked as offline by the administrator. # true: offline, false: online function mci_is_mantis_offline() { $t_offline_file = dirname( dirname( __FILE__ ) ) . DIRECTORY_SEPARATOR . 'mantis_offline.php'; diff --git a/tests/soap/LoginTest.php b/tests/soap/LoginTest.php index d8b17b0..f2fed26 100644 --- a/tests/soap/LoginTest.php +++ b/tests/soap/LoginTest.php @@ -31,6 +31,25 @@ class LoginTest extends SoapBase { private $dummyUser = 'no'; private $dummyPassword = 'user'; + public function testLoginFailed() { + try { + $this->client->mc_login( $this->dummyUser , $this->dummyPassword ); + $this->fail( "Should have failed." ); + } catch ( SoapFault $e) { + $this->assertIsLoginFailure( $e ); + } + } + + public function testLoginSuccessfully() { + $t_user_data = $this->client->mc_login( $this->userName, $this->password ); + + $this->assertEquals( $this->userName, $t_user_data->account_data->name, 'name' ); + $this->assertEquals( $this->userId, $t_user_data->account_data->id, 'id' ); + $this->assertEquals( $this->email, $t_user_data->account_data->email, 'email' ); + $this->assertEquals( false, empty( $t_user_data->timezone ), 'timezone' ); + $this->assertEquals( 90, (integer)$t_user_data->access_level, 'access_level' ); + } + public function testGetIssueGetLoginFailed() { try { $this->client->mc_issue_get( $this->dummyUser , $this->dummyPassword, 1 ); @@ -77,29 +96,29 @@ class LoginTest extends SoapBase { } public function testLoginWithNullPasswordIsRejected() { - try { - $this->client->mc_enum_status( $this->userName, null); - $this->fail( "Should have failed." ); - } catch ( SoapFault $e) { - $this->assertIsLoginFailure( $e ); + try { + $this->client->mc_enum_status( $this->userName, null); + $this->fail( "Should have failed." ); + } catch ( SoapFault $e) { + $this->assertIsLoginFailure( $e ); } } public function testLoginWithEmptyPasswordIsRejected() { - try { - $this->client->mc_enum_status( $this->userName, ''); - $this->fail( "Should have failed." ); - } catch ( SoapFault $e) { - $this->assertIsLoginFailure( $e ); + try { + $this->client->mc_enum_status( $this->userName, ''); + $this->fail( "Should have failed." ); + } catch ( SoapFault $e) { + $this->assertIsLoginFailure( $e ); } } public function testLoginWithIncorrectPasswordIsRejected() { - try { - $this->client->mc_enum_status( $this->userName, "This really should be incorrect"); - $this->fail( "Should have failed." ); - } catch ( SoapFault $e) { - $this->assertIsLoginFailure( $e ); + try { + $this->client->mc_enum_status( $this->userName, "This really should be incorrect"); + $this->fail( "Should have failed." ); + } catch ( SoapFault $e) { + $this->assertIsLoginFailure( $e ); } } diff --git a/tests/soap/SoapBase.php b/tests/soap/SoapBase.php index 39365fb..dea42d4 100644 --- a/tests/soap/SoapBase.php +++ b/tests/soap/SoapBase.php @@ -32,6 +32,7 @@ class SoapBase extends PHPUnit_Framework_TestCase { protected $client; protected $userName = 'administrator'; protected $password = 'root'; + protected $email = 'root@localhost'; protected $userId = '1'; protected $mantisPath; | ||||
Sounds good to me. Except that I would make the retrieval by id rather than by username, for consistency reasons. |
|
That means that we need to have another method to retrieve the AccountData for the logged in user. The scenario I\\\\'m talking about is where the client application needs to retrieve information for the user who just logged in. We could argue that this can be achieved via: We can also have a model where we can pass in to mc_user_get() an ObjectRef that can have id or username set. I believe we do this model when reporting or updating issues. |
|
(In reply to comment 0013445:0030100)
Right, that is clearer to me.
I think we should do both, even though the implementation will likely be shared. Performing a mc_login opens the door to returning access tokens, to eliminate the need for sending a username/password on each query, and instead using soap headers ( for instance ). |
|
As suggested in https://sourceforge.net/mailarchive/message.php?msg_id=28368791 , we should return the user's timezone as part of the UserData. |
|
@rombert, I've attached a patch that implements mc_login() which returns the UserData as a result. I've decided to skip mc_user_get() since I don't think it is as critical and it requires thinking through who can get information or what subset of information about other users. I've implemented the corresponding tests and verified that all tests pass. Let me know your thoughts. |
|
The patch looks good to me. I only have a couple of notes/questions
try {
|
|
Relating to the extra diff, I checked and there is no extra spaces or change in the content. I originally thought it is related to removal of white spaces, but it is not the case. I also changed my editor setting to not automatically remove extra spaces. Not sure why it is showing in the diff, but given that I changed editor settings, I hope it won't show up again. As for the per project access levels, we can always add this later. I would like to consider it after thinking through the work for 0013443 |
|
That means that we need to have another method to retrieve the AccountData for the logged in user. The scenario I\\\\'m talking about is where the client application needs to retrieve information for the user who just logged in. (rombert: removed unrelated links) |
|
Marking as 'acknowledged' not resolved/closed to track that change gets ported to master-2.0.x branch |
|
MantisBT: master-1.2.x 657c81f9 2012-06-06 21:33 Details Diff |
Fixes 0013445: Add mc_login() for login and to return user data. |
Affected Issues 0013445 |
|
mod - api/soap/mantisconnect.php | Diff File | ||
mod - api/soap/mc_api.php | Diff File | ||
mod - tests/soap/LoginTest.php | Diff File | ||
mod - tests/soap/SoapBase.php | Diff File | ||
MantisBT: master ae27eaeb 2012-06-06 21:33 Details Diff |
Fixes 0013445: Add mc_login() for login and to return user data. |
Affected Issues 0013445 |
|
mod - api/soap/mantisconnect.php | Diff File | ||
mod - api/soap/mc_api.php | Diff File | ||
mod - tests/soap/LoginTest.php | Diff File | ||
mod - tests/soap/SoapBase.php | Diff File |