From 86dc386519f73f75beb251785a82bbd78e5de4e3 Mon Sep 17 00:00:00 2001 From: Lutt Date: Wed, 9 Jun 2021 22:50:28 +0200 Subject: [PATCH 1/4] Upgrade lcobucci/jwt to ^4.0 Added editorconfig to enforce code formatting Added gitignore Reference for upgrade: https://github.com/thephpleague/oauth2-server/blob/master/src/AuthorizationValidators/BearerTokenValidator.php --- .editorconfig | 15 ++++++++ .gitignore | 2 + composer.json | 2 +- src/AmoclientController.php | 63 ++++++++++++++++++++++---------- src/AmoclientServiceProvider.php | 4 +- src/routes.php | 2 +- 6 files changed, 64 insertions(+), 24 deletions(-) create mode 100644 .editorconfig create mode 100644 .gitignore diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 0000000..6537ca4 --- /dev/null +++ b/.editorconfig @@ -0,0 +1,15 @@ +root = true + +[*] +charset = utf-8 +end_of_line = lf +insert_final_newline = true +indent_style = space +indent_size = 4 +trim_trailing_whitespace = true + +[*.md] +trim_trailing_whitespace = false + +[*.{yml,yaml}] +indent_size = 2 diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..d1502b0 --- /dev/null +++ b/.gitignore @@ -0,0 +1,2 @@ +vendor/ +composer.lock diff --git a/composer.json b/composer.json index 2706158..464df9e 100644 --- a/composer.json +++ b/composer.json @@ -11,7 +11,7 @@ ], "minimum-stability": "stable", "require": { - "lcobucci/jwt": "3.3.3", + "lcobucci/jwt": "^4.0", "guzzlehttp/guzzle": "^7.0" }, "autoload": { diff --git a/src/AmoclientController.php b/src/AmoclientController.php index a443871..edbb986 100644 --- a/src/AmoclientController.php +++ b/src/AmoclientController.php @@ -2,24 +2,43 @@ namespace StudioKaa\Amoclient; -use Illuminate\Http\Request; -use App\Http\Requests; +use App\Models\User; use App\Http\Controllers\Controller; -use Illuminate\Support\Facades\Auth; -use Lcobucci\JWT\Parser; +use DateTimeZone; +use Illuminate\Http\Request; +use Illuminate\Support\Facades\Auth; +use Lcobucci\JWT\Configuration; use Lcobucci\JWT\Signer\Hmac\Sha256; -use App\Models\User; +use Lcobucci\JWT\Signer\Key\InMemory; +use Lcobucci\Clock\SystemClock; +use Lcobucci\JWT\Validation\Constraint\ValidAt; +use Lcobucci\JWT\Validation\Constraint\SignedWith; +use Lcobucci\JWT\Validation\RequiredConstraintsViolated; class AmoclientController extends Controller { + private Configuration $config; + + public function __construct() + { + $this->config = Configuration::forSymmetricSigner( + new Sha256(), + InMemory::plainText('') + ); + + $this->config->setValidationConstraints( + new ValidAt(new SystemClock(new DateTimeZone(\date_default_timezone_get()))), + new SignedWith(new Sha256(), InMemory::plainText(config('amoclient.client_secret'))) + ); + } public function redirect() { $client_id = config('amoclient.client_id'); if($client_id == null) { - dd('Please set AMO_CLIENT_ID and AMO_CLIENT_SECRET in .env file'); + abort(500, 'Please set AMO_CLIENT_ID and AMO_CLIENT_SECRET in .env file.'); } return redirect('https://login.curio.codes/oauth/authorize?client_id=' . $client_id . '&redirect_id=' . url('amoclient/callback') . '&response_type=code'); @@ -41,25 +60,29 @@ public function callback(Request $request) ] ]); - //Get id_token from the reponse - $tokens = json_decode( (string) $response->getBody() ); - $id_token = (new Parser())->parse((string) $tokens->id_token); + $tokens = json_decode((string) $response->getBody()); - //Verify id_token - if(!$id_token->verify(new Sha256(), config('amoclient.client_secret'))) - { - dd("The id_token cannot be verified."); - } + try { + $token = $this->config->parser()->parse($tokens->id_token); + } catch (\Lcobucci\JWT\Exception $exception) { + abort(400, 'Access token could not be parsed!'); + } + + try { + $constraints = $this->config->validationConstraints(); + $this->config->validator()->assert($token, ...$constraints); + } catch (RequiredConstraintsViolated $exception) { + abort(400, 'Access token could not be verified!'); + } - //Get 'user' claim - $id_token->getClaims(); - $token_user = $id_token->getClaim('user'); + $claims = $token->claims(); + $token_user = $claims->get('user'); $token_user = json_decode($token_user); //Check if user may login if(config('amoclient.app_for') == 'teachers' && $token_user->type != 'teacher') { - dd('Oops: this app is only availble to teacher-accounts'); + abort(403, 'Oops: This app is only available to teacher-accounts!'); } //Create new user if not exists @@ -78,7 +101,7 @@ public function callback(Request $request) Auth::login($user); //Store access- and refresh-token in session - $access_token = (new Parser())->parse((string) $tokens->access_token); + $access_token = $this->config->parser()->parse((string) $tokens->access_token); $request->session()->put('access_token', $access_token); $request->session()->put('refresh_token', $tokens->refresh_token); @@ -86,7 +109,7 @@ public function callback(Request $request) return redirect('/amoclient/ready'); } catch (\GuzzleHttp\Exception\BadResponseException $e) { - dd("Unable to retrieve access token: " . $e->getResponse()->getBody()); + abort(500, 'Unable to retrieve access token: '. $e->getResponse()->getBody()); } } diff --git a/src/AmoclientServiceProvider.php b/src/AmoclientServiceProvider.php index faa38f1..a3fd2cb 100644 --- a/src/AmoclientServiceProvider.php +++ b/src/AmoclientServiceProvider.php @@ -16,7 +16,7 @@ public function boot() if(config('amoclient.use_migration') == 'yes') { $this->loadMigrationsFrom(__DIR__.'/migrations'); - } + } } /** * Register the application services. @@ -33,5 +33,5 @@ public function register() $this->app->singleton('StudioKaa\AmoAPI', function () { return new AmoAPI(); }); - } + } } diff --git a/src/routes.php b/src/routes.php index 3144bb3..bd23860 100644 --- a/src/routes.php +++ b/src/routes.php @@ -4,4 +4,4 @@ Route::get('amoclient/redirect', 'StudioKaa\Amoclient\AmoclientController@redirect'); Route::get('amoclient/callback', 'StudioKaa\Amoclient\AmoclientController@callback'); Route::get('amoclient/logout', 'StudioKaa\Amoclient\AmoclientController@logout'); -}); \ No newline at end of file +}); From 5337c37fb077624b9f235e83cd90a381f1b52bb6 Mon Sep 17 00:00:00 2001 From: Luttje Date: Thu, 10 Jun 2021 11:50:45 +0200 Subject: [PATCH 2/4] Add ordered list to readme --- README.md | 39 ++++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 367abf4..09357ac 100644 --- a/README.md +++ b/README.md @@ -7,24 +7,28 @@ Now using curio.codes! __!!__ Please make sure your app is using _https_, to prevent unwanted exposure of token, secrets, etc. -In your laravel project run: `composer require studiokaa/amoclient` +To use amoclient in your project: + +1. In your laravel project run: `composer require studiokaa/amoclient` +2. Set these keys in your .env file: + * AMO_CLIENT_ID + * AMO_CLIENT_SECRET + * AMO_API_LOG (optional) + * Default: no + * Set to 'yes' to make Amoclient log all usage of access_tokens and refresh_tokens to the default log-channel. + * AMO_APP_FOR (optional) + * Default: teachers + * This key determines if students can login to your application. + * May be one of: + * _all_: everyone can login, you may restrict access using guards or middleware. + * _teachers_: a student will be completely blocked and no user will be created when they try to login. + * AMO_USE_MIGRATION (optional) + * Default: yes + * Set to no if you want to use your own migration instead of the users migration this package provides +3. Alter your User model and add the line: `public $incrementing = false;` +4. (Recommended) Remove any default users-migration from your app, because Amoclient will conflict with it. Do _not_ remove the user-model. If you want to keep using your own migration, in your .env file set: `AMO_USE_MIGRATION=no` +5. Lastly, run `php artisan migrate`. -Now set these keys in your .env file: -* AMO_CLIENT_ID -* AMO_CLIENT_SECRET -* AMO_API_LOG - * Set to 'yes' to make Amoclient log all usage of access_tokens and refresh_tokens to the default log-channel. -* AMO_APP_FOR - * This key determines if students can login to your application. - * May be one of: - * _all_: everyone can login, you may restrict access using guards or middleware. - * _teachers_: a student will be completely blocked and no user will be created when they try to login. - -Alter your User model by adding the line: `public $incrementing = false;` - -You should remove any default users-migration from your app, because Amoclient will conflict with it. Do _not_ remove the user-model. If you want to keep using your own migration, in your .env file set: `AMO_USE_MIGRATION=no` - -Lastly, run `php artisan migrate`. ## Usage @@ -59,6 +63,7 @@ _Please note:_ a real logout cannot be accomplished at this time. If you log-out ### Laravel's `make:auth` Don't use this in combination with Amoclient. + ## AmoAPI Apart from being the central login-server, _login.amo.rocks_ also exposes an api. Please note this api is currently undocumented, although there are options to explore the api: * Refer to _amologin_'s [routes/api.php](https://github.com/StudioKaa/amologin/blob/master/routes/api.php) file. From 19bef0446d2a6e253c166ed63c13b5a9d8dd3c38 Mon Sep 17 00:00:00 2001 From: Luttje Date: Thu, 10 Jun 2021 12:04:17 +0200 Subject: [PATCH 3/4] Add contribution notes --- README.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/README.md b/README.md index 09357ac..20be26f 100644 --- a/README.md +++ b/README.md @@ -90,3 +90,23 @@ class MyController extends Controller * Performs an HTTP-request like `GET https://api.amo.rocks/$endpoint`. * This method relies on a user being authenticated through the amoclient first. Please do call this method only from routes and/or controllers protected by the _auth_ middlware. * Returns a Laravel-collection + + +## Contributing + +1. Clone this repository to your device +2. Inside the root of this repository run `composer install` +3. Create a test project in which you will use this package (Follow [Usage](#usage) instructions above) +4. Add the package locally using the following additions to your composer.json: + ```json + "repositories": [ + { + "type": "path", + "url": "../amoclient" + } + ], + ``` + * **Note:** `../amoclient` should point to where you cloned this package +5. Run `composer require "vendorname/packagename @dev"` inside the test project + +You can now test and modify this package. Changes will immediately be reflected in the test project. \ No newline at end of file From c59933ad9262361dbf9ac1eca5969639fec51b2f Mon Sep 17 00:00:00 2001 From: Luttje Date: Thu, 10 Jun 2021 12:44:39 +0200 Subject: [PATCH 4/4] Have AmoAPI work --- README.md | 5 +++++ src/AmoAPI.php | 20 +++++++++++-------- src/AmoclientController.php | 35 +++++--------------------------- src/AmoclientHelper.php | 40 +++++++++++++++++++++++++++++++++++++ 4 files changed, 62 insertions(+), 38 deletions(-) create mode 100644 src/AmoclientHelper.php diff --git a/README.md b/README.md index 20be26f..8e9cde3 100644 --- a/README.md +++ b/README.md @@ -65,10 +65,13 @@ Don't use this in combination with Amoclient. ## AmoAPI + Apart from being the central login-server, _login.amo.rocks_ also exposes an api. Please note this api is currently undocumented, although there are options to explore the api: * Refer to _amologin_'s [routes/api.php](https://github.com/StudioKaa/amologin/blob/master/routes/api.php) file. * Play around at [apitest.amo.rocks](https://apitest.amo.rocks/). +### Amoclient API Interface + An example of calling the api through Amoclient; ``` namespace App\Http\Controllers; @@ -86,6 +89,8 @@ class MyController extends Controller ``` +**Known 'bug':** Currently the AmoAPI class doesn't check if the token expired but just refreshes it anytime you use it. + ### `AmoAPI::get($endpoint)` * Performs an HTTP-request like `GET https://api.amo.rocks/$endpoint`. * This method relies on a user being authenticated through the amoclient first. Please do call this method only from routes and/or controllers protected by the _auth_ middlware. diff --git a/src/AmoAPI.php b/src/AmoAPI.php index 6570e5d..c6c5e31 100644 --- a/src/AmoAPI.php +++ b/src/AmoAPI.php @@ -1,7 +1,7 @@ log('START using access_token'); - if($access_token->isExpired()) + // TODO: Don't needlesly refresh the token + //if($access_token->isExpired()) { $this->log('access_token expired, trying to refresh'); $access_token = $this->refresh(session('refresh_token')); } - else - { - $this->log('Succesfully using current access_token'); - } + // else + // { + // $this->log('Succesfully using current access_token'); + // } $response = $this->client->request($method, 'https://api.curio.codes' . $endpoint, [ 'headers' => [ @@ -59,6 +61,8 @@ private function call($endpoint = 'user', $method = 'GET') private function refresh($refresh_token) { + $config = AmoclientHelper::getTokenConfig(); + try { $response = $this->client->post('https://login.curio.codes/oauth/token', [ @@ -73,8 +77,8 @@ private function refresh($refresh_token) $this->log('new access_token acquired'); $tokens = json_decode( (string) $response->getBody() ); - $access_token = (new Parser())->parse((string) $tokens->access_token); - session()->put('access_token', $access_token); + $access_token = $config->parser()->parse((string) $tokens->access_token)->toString(); + session()->put('access_token', $tokens->access_token); session()->put('refresh_token', $tokens->refresh_token); return $access_token; diff --git a/src/AmoclientController.php b/src/AmoclientController.php index edbb986..e92eb09 100644 --- a/src/AmoclientController.php +++ b/src/AmoclientController.php @@ -5,34 +5,12 @@ use App\Models\User; use App\Http\Controllers\Controller; -use DateTimeZone; use Illuminate\Http\Request; use Illuminate\Support\Facades\Auth; -use Lcobucci\JWT\Configuration; -use Lcobucci\JWT\Signer\Hmac\Sha256; -use Lcobucci\JWT\Signer\Key\InMemory; -use Lcobucci\Clock\SystemClock; -use Lcobucci\JWT\Validation\Constraint\ValidAt; -use Lcobucci\JWT\Validation\Constraint\SignedWith; use Lcobucci\JWT\Validation\RequiredConstraintsViolated; class AmoclientController extends Controller { - private Configuration $config; - - public function __construct() - { - $this->config = Configuration::forSymmetricSigner( - new Sha256(), - InMemory::plainText('') - ); - - $this->config->setValidationConstraints( - new ValidAt(new SystemClock(new DateTimeZone(\date_default_timezone_get()))), - new SignedWith(new Sha256(), InMemory::plainText(config('amoclient.client_secret'))) - ); - } - public function redirect() { $client_id = config('amoclient.client_id'); @@ -60,17 +38,18 @@ public function callback(Request $request) ] ]); + $config = AmoclientHelper::getTokenConfig(); $tokens = json_decode((string) $response->getBody()); try { - $token = $this->config->parser()->parse($tokens->id_token); + $token = $config->parser()->parse($tokens->id_token); } catch (\Lcobucci\JWT\Exception $exception) { abort(400, 'Access token could not be parsed!'); } try { - $constraints = $this->config->validationConstraints(); - $this->config->validator()->assert($token, ...$constraints); + $constraints = $config->validationConstraints(); + $config->validator()->assert($token, ...$constraints); } catch (RequiredConstraintsViolated $exception) { abort(400, 'Access token could not be verified!'); } @@ -97,17 +76,13 @@ public function callback(Request $request) $user->save(); } - //Login Auth::login($user); //Store access- and refresh-token in session - $access_token = $this->config->parser()->parse((string) $tokens->access_token); - $request->session()->put('access_token', $access_token); + $request->session()->put('access_token', $tokens->access_token); $request->session()->put('refresh_token', $tokens->refresh_token); - //Redirect return redirect('/amoclient/ready'); - } catch (\GuzzleHttp\Exception\BadResponseException $e) { abort(500, 'Unable to retrieve access token: '. $e->getResponse()->getBody()); } diff --git a/src/AmoclientHelper.php b/src/AmoclientHelper.php new file mode 100644 index 0000000..762c1eb --- /dev/null +++ b/src/AmoclientHelper.php @@ -0,0 +1,40 @@ +setValidationConstraints( + new ValidAt(new SystemClock(new DateTimeZone(\date_default_timezone_get()))), + new SignedWith(new Sha256(), InMemory::plainText($client_id)) + ); + + return self::$cachedConfig; + } +}