From 8ff349761d25dc0ed7f768926fd7039b9bc16cf5 Mon Sep 17 00:00:00 2001 From: smarcet Date: Wed, 12 Aug 2020 05:42:45 -0300 Subject: [PATCH] New OAUTH2 endpoints to update user info (ME) PUT /api/v1/users/me payload 'first_name' => 'sometimes|string', 'last_name' => 'sometimes|string', 'email' => 'sometimes|email', 'identifier' => 'sometimes|string', 'bio' => 'nullable|string', 'address1' => 'nullable|string', 'address2' => 'nullable|string', 'city' => 'nullable|string', 'state' => 'nullable|string', 'post_code' => 'nullable|string', 'country_iso_code' => 'nullable|country_iso_alpha2_code', 'second_email' => 'nullable|email', 'third_email' => 'nullable|email', 'gender' => 'nullable|string', 'gender_specify' => 'nullable|string', 'statement_of_interest' => 'nullable|string', 'irc' => 'nullable|string', 'linked_in_profile' => 'nullable|string', 'github_user' => 'nullable|string', 'wechat_user' => 'nullable|string', 'twitter_name' => 'nullable|string', 'language' => 'nullable|string', 'birthday' => 'nullable|date_format:U', 'password' => 'sometimes|string|min:8|confirmed', 'phone_number' => 'nullable|string', 'company' => 'nullable|string', required scopes me/write PUT /api/v1/users/me/pic multiform encoding pic: file (png, jpg, jpeg) required scopes me/write Change-Id: I31a1edd9eb1fcdee228a8f5ba1b44d324116edd9 Signed-off-by: smarcet --- app/Exceptions/Handler.php | 24 ++++- .../Api/OAuth2/OAuth2UserApiController.php | 102 ++++++++++++++++-- .../Controllers/Api/UserApiController.php | 63 +---------- .../Factories/UserValidationRulesFactory.php | 96 +++++++++++++++++ app/Http/routes.php | 13 ++- app/Providers/AppServiceProvider.php | 2 +- app/Services/OpenId/UserService.php | 11 +- app/libs/OAuth2/IUserScopes.php | 3 +- database/migrations/Version20200811151509.php | 83 ++++++++++++++ database/seeds/ApiEndpointSeeder.php | 20 +++- database/seeds/ApiScopeSeeder.php | 16 +++ database/seeds/TestSeeder.php | 51 +++++++-- tests/OAuth2UserServiceApiTest.php | 40 ++++++- 13 files changed, 438 insertions(+), 86 deletions(-) create mode 100644 app/Http/Controllers/Factories/UserValidationRulesFactory.php create mode 100644 database/migrations/Version20200811151509.php diff --git a/app/Exceptions/Handler.php b/app/Exceptions/Handler.php index 53617ef4..2fe58c6c 100644 --- a/app/Exceptions/Handler.php +++ b/app/Exceptions/Handler.php @@ -1,14 +1,27 @@ -user_service = $user_service; $this->client_repository = $client_repository; $this->id_token_builder = $id_token_builder; + $this->openid_user_service = $openid_user_service; } /** @@ -131,6 +139,88 @@ final class OAuth2UserApiController extends OAuth2ProtectedController } } + protected function curateUpdatePayload(array $payload): array + { + // remove possible fields that an user can not update + // from this endpoint + if(isset($payload['groups'])) + unset($payload['groups']); + + if(isset($payload['email_verified'])) + unset($payload['email_verified']); + + if(isset($payload['active'])) + unset($payload['active']); + + return HTMLCleaner::cleanData($payload, [ + 'bio', 'statement_of_interest' + ]); + } + + public function UpdateMe(){ + try { + if(!Request::isJson()) return $this->error400(); + if(!$this->resource_server_context->getCurrentUserId()){ + return $this->error403(); + } + $payload = Input::json()->all(); + // Creates a Validator instance and validates the data. + + $validation = Validator::make($payload, UserValidationRulesFactory::build($payload, true)); + if ($validation->fails()) { + $ex = new ValidationException(); + throw $ex->setMessages($validation->messages()->toArray()); + } + + $user = $this->openid_user_service->update($this->resource_server_context->getCurrentUserId(), $this->curateUpdatePayload($payload)); + + return $this->updated(SerializerRegistry::getInstance()->getSerializer($user, SerializerRegistry::SerializerType_Private)->serialize()); + } + catch (ValidationException $ex1) + { + Log::warning($ex1); + return $this->error412($ex1->getMessages()); + } + catch (EntityNotFoundException $ex2) + { + Log::warning($ex2); + return $this->error404(['message' => $ex2->getMessage()]); + } + catch (Exception $ex) { + Log::error($ex); + return $this->error500($ex); + } + } + + public function UpdateMyPic(){ + try { + if (!$this->resource_server_context->getCurrentUserId()) { + return $this->error403(); + } + + $file = request()->file('pic'); + + if (!is_null($file)) { + $user = $this->openid_user_service->updateProfilePhoto($this->resource_server_context->getCurrentUserId(), $file); + } + return $this->updated(SerializerRegistry::getInstance()->getSerializer($user, SerializerRegistry::SerializerType_Private)->serialize()); + } + catch (ValidationException $ex1) + { + Log::warning($ex1); + return $this->error412($ex1->getMessages()); + } + catch (EntityNotFoundException $ex2) + { + Log::warning($ex2); + return $this->error404(['message' => $ex2->getMessage()]); + } + catch (Exception $ex) { + Log::error($ex); + return $this->error500($ex); + } + } + public function userInfo() { try { diff --git a/app/Http/Controllers/Api/UserApiController.php b/app/Http/Controllers/Api/UserApiController.php index 0737cee6..ee5ad304 100644 --- a/app/Http/Controllers/Api/UserApiController.php +++ b/app/Http/Controllers/Api/UserApiController.php @@ -13,6 +13,7 @@ **/ use App\Http\Controllers\APICRUDController; +use App\Http\Controllers\UserValidationRulesFactory; use App\Http\Utils\HTMLCleaner; use App\ModelSerializers\SerializerRegistry; use Auth\Repositories\IUserRepository; @@ -173,36 +174,7 @@ final class UserApiController extends APICRUDController */ protected function getUpdatePayloadValidationRules(): array { - return [ - 'first_name' => 'required|string', - 'last_name' => 'required|string', - 'email' => 'required|email', - 'identifier' => 'sometimes|string', - 'bio' => 'nullable|string', - 'address1' => 'nullable|string', - 'address2' => 'nullable|string', - 'city' => 'nullable|string', - 'state' => 'nullable|string', - 'post_code' => 'nullable|string', - 'country_iso_code' => 'nullable|country_iso_alpha2_code', - 'second_email' => 'nullable|email', - 'third_email' => 'nullable|email', - 'gender' => 'nullable|string', - 'gender_specify' => 'nullable|string', - 'statement_of_interest' => 'nullable|string', - 'irc' => 'nullable|string', - 'linked_in_profile' => 'nullable|string', - 'github_user' => 'nullable|string', - 'wechat_user' => 'nullable|string', - 'twitter_name' => 'nullable|string', - 'language' => 'nullable|string', - 'birthday' => 'nullable|date_format:U', - 'password' => 'sometimes|string|min:8|confirmed', - 'email_verified' => 'nullable|boolean', - 'active' => 'nullable|boolean', - 'phone_number' => 'nullable|string', - 'company' => 'nullable|string', - ]; + return UserValidationRulesFactory::build([], true); } protected function curateUpdatePayload(array $payload): array @@ -224,38 +196,9 @@ final class UserApiController extends APICRUDController */ protected function getCreatePayloadValidationRules(): array { - return [ - 'first_name' => 'required|string', - 'last_name' => 'required|string', - 'email' => 'required|email', - 'identifier' => 'sometimes|string', - 'bio' => 'nullable|string', - 'address1' => 'nullable|string', - 'address2' => 'nullable|string', - 'city' => 'nullable|string', - 'state' => 'nullable|string', - 'post_code' => 'nullable|string', - 'country_iso_code' => 'nullable|country_iso_alpha2_code', - 'second_email' => 'nullable|email', - 'third_email' => 'nullable|email', - 'gender' => 'nullable|string', - 'statement_of_interest' => 'nullable|string', - 'irc' => 'nullable|string', - 'linked_in_profile' => 'nullable|string', - 'github_user' => 'nullable|string', - 'wechat_user' => 'nullable|string', - 'twitter_name' => 'nullable|string', - 'language' => 'nullable|string', - 'birthday' => 'nullable|date_format:U', - 'password' => 'sometimes|string|min:8|confirmed', - 'email_verified' => 'nullable|boolean', - 'active' => 'nullable|boolean', - 'phone_number' => 'nullable|string', - 'company' => 'nullable|string', - ]; + return UserValidationRulesFactory::build([], false); } - /** * @param LaravelRequest $request * @return \Illuminate\Http\JsonResponse|mixed diff --git a/app/Http/Controllers/Factories/UserValidationRulesFactory.php b/app/Http/Controllers/Factories/UserValidationRulesFactory.php new file mode 100644 index 00000000..7585067b --- /dev/null +++ b/app/Http/Controllers/Factories/UserValidationRulesFactory.php @@ -0,0 +1,96 @@ + 'sometimes|string', + 'last_name' => 'sometimes|string', + 'email' => 'sometimes|email', + 'identifier' => 'sometimes|string', + 'bio' => 'nullable|string', + 'address1' => 'nullable|string', + 'address2' => 'nullable|string', + 'city' => 'nullable|string', + 'state' => 'nullable|string', + 'post_code' => 'nullable|string', + 'country_iso_code' => 'nullable|country_iso_alpha2_code', + 'second_email' => 'nullable|email', + 'third_email' => 'nullable|email', + 'gender' => 'nullable|string', + 'gender_specify' => 'nullable|string', + 'statement_of_interest' => 'nullable|string', + 'irc' => 'nullable|string', + 'linked_in_profile' => 'nullable|string', + 'github_user' => 'nullable|string', + 'wechat_user' => 'nullable|string', + 'twitter_name' => 'nullable|string', + 'language' => 'nullable|string', + 'birthday' => 'nullable|date_format:U', + 'password' => 'sometimes|string|min:8|confirmed', + 'phone_number' => 'nullable|string', + 'company' => 'nullable|string', + // admin fields + 'email_verified' => 'nullable|boolean', + 'active' => 'nullable|boolean', + 'groups' => 'sometimes|int_array', + ]; + } + + return [ + 'first_name' => 'required|string', + 'last_name' => 'required|string', + 'email' => 'required|email', + 'identifier' => 'sometimes|string', + 'bio' => 'nullable|string', + 'address1' => 'nullable|string', + 'address2' => 'nullable|string', + 'city' => 'nullable|string', + 'state' => 'nullable|string', + 'post_code' => 'nullable|string', + 'country_iso_code' => 'nullable|country_iso_alpha2_code', + 'second_email' => 'nullable|email', + 'third_email' => 'nullable|email', + 'gender' => 'nullable|string', + 'statement_of_interest' => 'nullable|string', + 'irc' => 'nullable|string', + 'linked_in_profile' => 'nullable|string', + 'github_user' => 'nullable|string', + 'wechat_user' => 'nullable|string', + 'twitter_name' => 'nullable|string', + 'language' => 'nullable|string', + 'birthday' => 'nullable|date_format:U', + 'password' => 'sometimes|string|min:8|confirmed', + 'phone_number' => 'nullable|string', + 'company' => 'nullable|string', + // admin fields + 'email_verified' => 'nullable|boolean', + 'active' => 'nullable|boolean', + 'groups' => 'sometimes|int_array', + ]; + } +} \ No newline at end of file diff --git a/app/Http/routes.php b/app/Http/routes.php index ce88d263..5490f9c1 100644 --- a/app/Http/routes.php +++ b/app/Http/routes.php @@ -168,16 +168,19 @@ Route::group(['namespace' => 'App\Http\Controllers', 'middleware' => 'web' ], fu 'middleware' => ['ssl', 'auth']], function () { Route::group(['prefix' => 'users'], function () { + Route::delete('/me/tokens/{value}',"UserApiController@revokeMyToken"); Route::get('' , "UserApiController@getAll"); Route::post('', ['middleware' => ['openstackid.currentuser.serveradmin.json'], 'uses' => "UserApiController@create"]); Route::put('me', "UserApiController@updateMe"); + Route::group(['prefix' => '{id}'], function(){ Route::group(['prefix' => 'locked'], function(){ Route::put('', ['middleware' => ['openstackid.currentuser.serveradmin.json'], 'uses' => 'UserApiController@unlock']); Route::delete('', ['middleware' => ['openstackid.currentuser.serveradmin.json'], 'uses' => 'UserApiController@lock']); }); + Route::get('', ['middleware' => ['openstackid.currentuser.serveradmin.json'], 'uses' => "UserApiController@get"]); Route::delete('', ['middleware' => ['openstackid.currentuser.serveradmin.json'], 'uses' =>"UserApiController@delete"]); Route::put('', ['middleware' => ['openstackid.currentuser.serveradmin.json'], 'uses' =>"UserApiController@update"]); @@ -376,7 +379,15 @@ Route::group( Route::group(['prefix' => 'users'], function () { Route::get('', 'OAuth2UserApiController@getAll'); Route::get('/{id}', 'OAuth2UserApiController@get'); - Route::get('/me', 'OAuth2UserApiController@me'); + + Route::group(['prefix' => 'me'], function () { + Route::get('', 'OAuth2UserApiController@me'); + Route::put('','OAuth2UserApiController@UpdateMe'); + Route::group(['prefix' => 'pic'], function () { + Route::put('','OAuth2UserApiController@UpdateMyPic'); + }); + }); + Route::get('/info', 'OAuth2UserApiController@userInfo'); Route::post('/info', 'OAuth2UserApiController@userInfo'); }); diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index cbe27da3..92c39f9f 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -90,7 +90,7 @@ class AppServiceProvider extends ServiceProvider if(!is_array($value)) return false; foreach($value as $element) { - if(!is_int($element)) return false; + if(!is_integer($element)) return false; } return true; }); diff --git a/app/Services/OpenId/UserService.php b/app/Services/OpenId/UserService.php index 644e7550..c3c0e017 100644 --- a/app/Services/OpenId/UserService.php +++ b/app/Services/OpenId/UserService.php @@ -12,6 +12,7 @@ * limitations under the License. **/ use App\Events\UserEmailUpdated; +use App\Events\UserPasswordResetSuccessful; use App\Jobs\PublishUserDeleted; use App\Jobs\PublishUserUpdated; use App\libs\Auth\Factories\UserFactory; @@ -229,11 +230,14 @@ final class UserService extends AbstractService implements IUserService public function update(int $id, array $payload): IEntity { return $this->tx_service->transaction(function() use($id, $payload){ + $user = $this->repository->getById($id); if(is_null($user) || !$user instanceof User) throw new EntityNotFoundException("user not found"); $former_email = $user->getEmail(); + $former_password = $user->getPassword(); + if(isset($payload["email"])){ $former_user = $this->repository->getByEmailOrName(trim($payload["email"])); if(!is_null($former_user) && $former_user->getId() != $id) @@ -259,11 +263,16 @@ final class UserService extends AbstractService implements IUserService } if($former_email != $user->getEmail()){ - Log::debug(sprintf("UserService::update use id %s - email changed old %s - email new %s", $id, $former_email , $user->getEmail())); + Log::warning(sprintf("UserService::update use id %s - email changed old %s - email new %s", $id, $former_email , $user->getEmail())); $user->clearEmailVerification(); Event::fire(new UserEmailUpdated($user->getId())); } + if($former_password != $user->getPassword()){ + Log::warning(sprintf("UserService::update use id %s - password changed", $id)); + Event::fire(new UserPasswordResetSuccessful($user->getId())); + } + try { if(Config::get("queue.enable_message_broker", false) == true) PublishUserUpdated::dispatch($user)->onConnection('message_broker'); diff --git a/app/libs/OAuth2/IUserScopes.php b/app/libs/OAuth2/IUserScopes.php index f862e107..55ccb0e2 100644 --- a/app/libs/OAuth2/IUserScopes.php +++ b/app/libs/OAuth2/IUserScopes.php @@ -26,5 +26,6 @@ interface IUserScopes const Registration = 'user-registration'; const ReadAll = 'users-read-all'; const SSO = 'sso'; - + const MeRead = 'me/read'; + const MeWrite = 'me/write'; } \ No newline at end of file diff --git a/database/migrations/Version20200811151509.php b/database/migrations/Version20200811151509.php new file mode 100644 index 00000000..936b723f --- /dev/null +++ b/database/migrations/Version20200811151509.php @@ -0,0 +1,83 @@ + IUserScopes::MeRead, + 'short_description' => 'Allows access to read your Profile', + 'description' => 'Allows access to read your Profile', + 'system' => false, + 'default' => false, + 'groups' => false, + ], + + ], 'users'); + + SeedUtils::seedScopes([ + [ + 'name' => IUserScopes::MeWrite, + 'short_description' => 'Allows access to write your Profile', + 'description' => 'Allows access to write your Profile', + 'system' => false, + 'default' => false, + 'groups' => false, + ], + + ], 'users'); + + SeedUtils::seedApiEndpoints('users', [ + [ + 'name' => 'update-my-user', + 'active' => true, + 'route' => '/api/v1/users/me', + 'http_method' => 'PUT', + 'scopes' => [ + \App\libs\OAuth2\IUserScopes::MeWrite + ], + ], + [ + 'name' => 'update-my-user-pic', + 'active' => true, + 'route' => '/api/v1/users/me/pic', + 'http_method' => 'PUT', + 'scopes' => [ + \App\libs\OAuth2\IUserScopes::MeWrite + ], + ], + ]); + } + + /** + * @param Schema $schema + */ + public function down(Schema $schema) + { + + } +} diff --git a/database/seeds/ApiEndpointSeeder.php b/database/seeds/ApiEndpointSeeder.php index 4064f286..73942772 100644 --- a/database/seeds/ApiEndpointSeeder.php +++ b/database/seeds/ApiEndpointSeeder.php @@ -83,7 +83,25 @@ class ApiEndpointSeeder extends Seeder 'scopes' => [ \App\libs\OAuth2\IUserScopes::ReadAll ], - ] + ], + [ + 'name' => 'update-my-user', + 'active' => true, + 'route' => '/api/v1/users/me', + 'http_method' => 'PUT', + 'scopes' => [ + \App\libs\OAuth2\IUserScopes::MeWrite + ], + ], + [ + 'name' => 'update-my-user-pic', + 'active' => true, + 'route' => '/api/v1/users/me/pic', + 'http_method' => 'PUT', + 'scopes' => [ + \App\libs\OAuth2\IUserScopes::MeWrite + ], + ], ] ); } diff --git a/database/seeds/ApiScopeSeeder.php b/database/seeds/ApiScopeSeeder.php index a662fc89..ca8ee20b 100644 --- a/database/seeds/ApiScopeSeeder.php +++ b/database/seeds/ApiScopeSeeder.php @@ -65,6 +65,22 @@ class ApiScopeSeeder extends Seeder { 'system' => false, 'default' => false, 'groups' => true, + ], + [ + 'name' => IUserScopes::MeRead, + 'short_description' => 'Allows access to read your Profile', + 'description' => 'Allows access to read your Profile', + 'system' => false, + 'default' => false, + 'groups' => false, + ], + [ + 'name' => IUserScopes::MeWrite, + 'short_description' => 'Allows access to write your Profile', + 'description' => 'Allows access to write your Profile', + 'system' => false, + 'default' => false, + 'groups' => false, ] ], 'users'); diff --git a/database/seeds/TestSeeder.php b/database/seeds/TestSeeder.php index a35a7996..7ccd2563 100644 --- a/database/seeds/TestSeeder.php +++ b/database/seeds/TestSeeder.php @@ -1373,14 +1373,32 @@ PPK; 'system' => false, 'active' => true, ), - array( + [ 'name' => 'address', 'short_description' => 'This scope value requests access to the address Claim.', 'description' => 'This scope value requests access to the address Claim.', 'api' => $api, 'system' => false, 'active' => true, - ) + ], + [ + 'name' => IUserScopes::MeRead, + 'short_description' => 'Allows access to read your Profile', + 'description' => 'Allows access to read your Profile', + 'api' => $api, + 'system' => false, + 'default' => false, + 'active' => true, + ], + [ + 'name' => IUserScopes::MeWrite, + 'short_description' => 'Allows access to write your Profile', + 'description' => 'Allows access to write your Profile', + 'api' => $api, + 'system' => false, + 'default' => false, + 'active' => true, + ] ]; foreach($api_scope_payloads as $payload) { @@ -1645,28 +1663,41 @@ PPK; $users = $api_repository->findOneBy(['name' => 'users']); $api_scope_payloads = [ - array( + [ 'name' => 'get-user-info', 'active' => true, 'api' => $users, 'route' => '/api/v1/users/me', 'http_method' => 'GET' - ), - array( + ], + [ 'name' => 'get-user-claims-get', 'active' => true, 'api' => $users, 'route' => '/api/v1/users/info', 'http_method' => 'GET' - ), - array( + ], + [ 'name' => 'get-user-claims-post', 'active' => true, 'api' => $users, 'route' => '/api/v1/users/info', 'http_method' => 'POST' - ) - + ], + [ + 'name' => 'update-my-user', + 'active' => true, + 'route' => '/api/v1/users/me', + 'api' => $users, + 'http_method' => 'PUT', + ], + [ + 'name' => 'update-my-user-pic', + 'active' => true, + 'route' => '/api/v1/users/me/pic', + 'api' => $users, + 'http_method' => 'PUT', + ], ]; foreach($api_scope_payloads as $payload) { @@ -1678,12 +1709,14 @@ PPK; $profile_scope = $api_scope_repository->findOneBy(['name' => 'profile']); $email_scope = $api_scope_repository->findOneBy(['name' => 'email']); $address_scope = $api_scope_repository->findOneBy(['name' => 'address']); + $me_write = $api_scope_repository->findOneBy(['name' => IUserScopes::MeWrite]); foreach($api_scope_payloads as $payload) { $endpoint = $endpoint_repository->findOneBy(['name' => $payload['name']]); $endpoint->addScope($address_scope); $endpoint->addScope($email_scope); $endpoint->addScope($profile_scope); + $endpoint->addScope($me_write); EntityManager::persist($endpoint); } diff --git a/tests/OAuth2UserServiceApiTest.php b/tests/OAuth2UserServiceApiTest.php index f4b290f6..6757c909 100644 --- a/tests/OAuth2UserServiceApiTest.php +++ b/tests/OAuth2UserServiceApiTest.php @@ -11,13 +11,50 @@ * See the License for the specific language governing permissions and * limitations under the License. **/ +use App\libs\OAuth2\IUserScopes; use OAuth2\ResourceServer\IUserService; /** * Class OAuth2UserServiceApiTest */ final class OAuth2UserServiceApiTest extends OAuth2ProtectedApiTest { + /** + * @covers OAuth2UserApiController::get() + */ + public function testUpdateMe(){ + $first_name_val = 'test_'. str_random(16); + $data = [ + 'first_name' => $first_name_val, + ]; + + $params = [ + ]; + + $headers = [ + "HTTP_Authorization" => " Bearer " . $this->access_token, + "CONTENT_TYPE" => "application/json" + ]; + + $response = $this->action + ( + "PUT", + "Api\\OAuth2\\OAuth2UserApiController@UpdateMe", + $params, + [], + [], + [], + $headers, + json_encode($data) + ); + + $this->assertResponseStatus(201); + $content = $response->getContent(); + $user = json_decode($content); + + $this->assertTrue($user->first_name == $first_name_val); + + } /** * @covers OAuth2UserApiController::get() @@ -58,7 +95,8 @@ final class OAuth2UserServiceApiTest extends OAuth2ProtectedApiTest { $scope = array( IUserService::UserProfileScope_Address, IUserService::UserProfileScope_Email, - IUserService::UserProfileScope_Profile + IUserService::UserProfileScope_Profile, + IUserScopes::MeWrite, ); return $scope;