From 07a6d7655fd77b9c33360b855a0c08d922b2f3ed Mon Sep 17 00:00:00 2001 From: Jasper Weyne Date: Wed, 1 Jul 2020 23:27:50 +0200 Subject: [PATCH 01/13] First basic OpenID Connect implementation --- .env.example.complete | 8 + app/Auth/Access/Guards/OpenIdSessionGuard.php | 39 +++ app/Auth/Access/OpenIdService.php | 235 ++++++++++++++++++ app/Config/auth.php | 4 + app/Config/openid.php | 43 ++++ app/Exceptions/OpenIdException.php | 6 + app/Http/Controllers/Auth/LoginController.php | 3 +- .../Controllers/Auth/OpenIdController.php | 70 ++++++ app/Http/Controllers/UserController.php | 4 +- app/Http/Middleware/VerifyCsrfToken.php | 3 +- app/Providers/AuthServiceProvider.php | 11 + composer.json | 3 +- composer.lock | 177 ++++++++++++- resources/lang/en/errors.php | 4 + .../views/auth/forms/login/openid.blade.php | 11 + resources/views/common/header.blade.php | 2 + resources/views/settings/index.blade.php | 2 +- resources/views/settings/roles/form.blade.php | 2 +- resources/views/users/form.blade.php | 2 +- routes/web.php | 5 + tests/Auth/AuthTest.php | 2 + 21 files changed, 626 insertions(+), 10 deletions(-) create mode 100644 app/Auth/Access/Guards/OpenIdSessionGuard.php create mode 100644 app/Auth/Access/OpenIdService.php create mode 100644 app/Config/openid.php create mode 100644 app/Exceptions/OpenIdException.php create mode 100644 app/Http/Controllers/Auth/OpenIdController.php create mode 100644 resources/views/auth/forms/login/openid.blade.php diff --git a/.env.example.complete b/.env.example.complete index 472ca051b..b211ad939 100644 --- a/.env.example.complete +++ b/.env.example.complete @@ -228,6 +228,14 @@ SAML2_USER_TO_GROUPS=false SAML2_GROUP_ATTRIBUTE=group SAML2_REMOVE_FROM_GROUPS=false +# OpenID Connect authentication configuration +OPENID_CLIENT_ID=null +OPENID_CLIENT_SECRET=null +OPENID_ISSUER=https://example.com +OPENID_PUBLIC_KEY=file:///my/public.key +OPENID_URL_AUTHORIZE=https://example.com/authorize +OPENID_URL_TOKEN=https://example.com/token + # Disable default third-party services such as Gravatar and Draw.IO # Service-specific options will override this option DISABLE_EXTERNAL_SERVICES=false diff --git a/app/Auth/Access/Guards/OpenIdSessionGuard.php b/app/Auth/Access/Guards/OpenIdSessionGuard.php new file mode 100644 index 000000000..0dfbb67a3 --- /dev/null +++ b/app/Auth/Access/Guards/OpenIdSessionGuard.php @@ -0,0 +1,39 @@ +config = config('openid'); + $this->registrationService = $registrationService; + $this->user = $user; + } + + /** + * Initiate a authorization flow. + * @throws Error + */ + public function login(): array + { + $provider = $this->getProvider(); + return [ + 'url' => $provider->getAuthorizationUrl(), + 'state' => $provider->getState(), + ]; + } + + /** + * Initiate a logout flow. + * @throws Error + */ + public function logout(): array + { + $this->actionLogout(); + $url = '/'; + $id = null; + + return ['url' => $url, 'id' => $id]; + } + + /** + * Process the Authorization response from the authorization server and + * return the matching, or new if registration active, user matched to + * the authorization server. + * Returns null if not authenticated. + * @throws Error + * @throws OpenIdException + * @throws ValidationError + * @throws JsonDebugException + * @throws UserRegistrationException + */ + public function processAuthorizeResponse(?string $authorizationCode): ?User + { + $provider = $this->getProvider(); + + // Try to exchange authorization code for access token + $accessToken = $provider->getAccessToken('authorization_code', [ + 'code' => $authorizationCode, + ]); + + return $this->processAccessTokenCallback($accessToken); + } + + /** + * Do the required actions to log a user out. + */ + protected function actionLogout() + { + auth()->logout(); + session()->invalidate(); + } + + /** + * Load the underlying Onelogin SAML2 toolkit. + * @throws Error + * @throws Exception + */ + protected function getProvider(): OpenIDConnectProvider + { + $settings = $this->config['openid']; + $overrides = $this->config['openid_overrides'] ?? []; + + if ($overrides && is_string($overrides)) { + $overrides = json_decode($overrides, true); + } + + $openIdSettings = $this->loadOpenIdDetails(); + $settings = array_replace_recursive($settings, $openIdSettings, $overrides); + + $signer = new \Lcobucci\JWT\Signer\Rsa\Sha256(); + return new OpenIDConnectProvider($settings, ['signer' => $signer]); + } + + /** + * Load dynamic service provider options required by the onelogin toolkit. + */ + protected function loadOpenIdDetails(): array + { + return [ + 'redirectUri' => url('/openid/redirect'), + ]; + } + + /** + * Calculate the display name + */ + protected function getUserDisplayName(Token $token, string $defaultValue): string + { + $displayNameAttr = $this->config['display_name_attributes']; + + $displayName = []; + foreach ($displayNameAttr as $dnAttr) { + $dnComponent = $token->getClaim($dnAttr, ''); + if ($dnComponent !== '') { + $displayName[] = $dnComponent; + } + } + + if (count($displayName) == 0) { + $displayName = $defaultValue; + } else { + $displayName = implode(' ', $displayName); + } + + return $displayName; + } + + /** + * Get the value to use as the external id saved in BookStack + * used to link the user to an existing BookStack DB user. + */ + protected function getExternalId(Token $token, string $defaultValue) + { + $userNameAttr = $this->config['external_id_attribute']; + if ($userNameAttr === null) { + return $defaultValue; + } + + return $token->getClaim($userNameAttr, $defaultValue); + } + + /** + * Extract the details of a user from a SAML response. + */ + protected function getUserDetails(Token $token): array + { + $email = null; + $emailAttr = $this->config['email_attribute']; + if ($token->hasClaim($emailAttr)) { + $email = $token->getClaim($emailAttr); + } + + return [ + 'external_id' => $token->getClaim('sub'), + 'email' => $email, + 'name' => $this->getUserDisplayName($token, $email), + ]; + } + + /** + * Get the user from the database for the specified details. + * @throws OpenIdException + * @throws UserRegistrationException + */ + protected function getOrRegisterUser(array $userDetails): ?User + { + $user = $this->user->newQuery() + ->where('external_auth_id', '=', $userDetails['external_id']) + ->first(); + + if (is_null($user)) { + $userData = [ + 'name' => $userDetails['name'], + 'email' => $userDetails['email'], + 'password' => Str::random(32), + 'external_auth_id' => $userDetails['external_id'], + ]; + + $user = $this->registrationService->registerUser($userData, null, false); + } + + return $user; + } + + /** + * Processes a received access token for a user. Login the user when + * they exist, optionally registering them automatically. + * @throws OpenIdException + * @throws JsonDebugException + * @throws UserRegistrationException + */ + public function processAccessTokenCallback(AccessToken $accessToken): User + { + $userDetails = $this->getUserDetails($accessToken->getIdToken()); + $isLoggedIn = auth()->check(); + + if ($this->config['dump_user_details']) { + throw new JsonDebugException($accessToken->jsonSerialize()); + } + + if ($userDetails['email'] === null) { + throw new OpenIdException(trans('errors.openid_no_email_address')); + } + + if ($isLoggedIn) { + throw new OpenIdException(trans('errors.openid_already_logged_in'), '/login'); + } + + $user = $this->getOrRegisterUser($userDetails); + if ($user === null) { + throw new OpenIdException(trans('errors.openid_user_not_registered', ['name' => $userDetails['external_id']]), '/login'); + } + + auth()->login($user); + return $user; + } +} diff --git a/app/Config/auth.php b/app/Config/auth.php index 51b152ff1..a1824bc78 100644 --- a/app/Config/auth.php +++ b/app/Config/auth.php @@ -40,6 +40,10 @@ return [ 'driver' => 'saml2-session', 'provider' => 'external', ], + 'openid' => [ + 'driver' => 'openid-session', + 'provider' => 'external', + ], 'api' => [ 'driver' => 'api-token', ], diff --git a/app/Config/openid.php b/app/Config/openid.php new file mode 100644 index 000000000..2232ba7b2 --- /dev/null +++ b/app/Config/openid.php @@ -0,0 +1,43 @@ + env('OPENID_NAME', 'SSO'), + + // Dump user details after a login request for debugging purposes + 'dump_user_details' => env('OPENID_DUMP_USER_DETAILS', false), + + // Attribute, within a OpenId token, to find the user's email address + 'email_attribute' => env('OPENID_EMAIL_ATTRIBUTE', 'email'), + // Attribute, within a OpenId token, to find the user's display name + 'display_name_attributes' => explode('|', env('OPENID_DISPLAY_NAME_ATTRIBUTES', 'username')), + // Attribute, within a OpenId token, to use to connect a BookStack user to the OpenId user. + 'external_id_attribute' => env('OPENID_EXTERNAL_ID_ATTRIBUTE', null), + + // Overrides, in JSON format, to the configuration passed to underlying OpenIDConnectProvider library. + 'openid_overrides' => env('OPENID_OVERRIDES', null), + + 'openid' => [ + // OAuth2/OpenId client id, as configured in your Authorization server. + 'clientId' => env('OPENID_CLIENT_ID', ''), + + // OAuth2/OpenId client secret, as configured in your Authorization server. + 'clientSecret' => env('OPENID_CLIENT_SECRET', ''), + + // OAuth2 scopes that are request, by default the OpenId-native profile and email scopes. + 'scopes' => 'profile email', + + // The issuer of the identity token (id_token) this will be compared with what is returned in the token. + 'idTokenIssuer' => env('OPENID_ISSUER', ''), + + // Public key that's used to verify the JWT token with. + 'publicKey' => env('OPENID_PUBLIC_KEY', ''), + + // OAuth2 endpoints. + 'urlAuthorize' => env('OPENID_URL_AUTHORIZE', ''), + 'urlAccessToken' => env('OPENID_URL_TOKEN', ''), + 'urlResourceOwnerDetails' => env('OPENID_URL_RESOURCE', ''), + ], + +]; diff --git a/app/Exceptions/OpenIdException.php b/app/Exceptions/OpenIdException.php new file mode 100644 index 000000000..056f95c56 --- /dev/null +++ b/app/Exceptions/OpenIdException.php @@ -0,0 +1,6 @@ +can('users-manage') && $user->can('user-roles-manage')) { - $guards = ['standard', 'ldap', 'saml2']; + $guards = ['standard', 'ldap', 'saml2', 'openid']; foreach ($guards as $guard) { auth($guard)->login($user); } @@ -186,5 +186,4 @@ class LoginController extends Controller return redirect('/login'); } - } diff --git a/app/Http/Controllers/Auth/OpenIdController.php b/app/Http/Controllers/Auth/OpenIdController.php new file mode 100644 index 000000000..8e475ffdb --- /dev/null +++ b/app/Http/Controllers/Auth/OpenIdController.php @@ -0,0 +1,70 @@ +openidService = $openidService; + $this->middleware('guard:openid'); + } + + /** + * Start the authorization login flow via OpenId Connect. + */ + public function login() + { + $loginDetails = $this->openidService->login(); + session()->flash('openid_state', $loginDetails['state']); + + return redirect($loginDetails['url']); + } + + /** + * Start the logout flow via OpenId Connect. + */ + public function logout() + { + $logoutDetails = $this->openidService->logout(); + + if ($logoutDetails['id']) { + session()->flash('saml2_logout_request_id', $logoutDetails['id']); + } + + return redirect($logoutDetails['url']); + } + + /** + * Authorization flow Redirect. + * Processes authorization response from the OpenId Connect Authorization Server. + */ + public function redirect() + { + $storedState = session()->pull('openid_state'); + $responseState = request()->query('state'); + + if ($storedState !== $responseState) { + $this->showErrorNotification(trans('errors.openid_fail_authed', ['system' => config('saml2.name')])); + return redirect('/login'); + } + + $user = $this->openidService->processAuthorizeResponse(request()->query('code')); + if ($user === null) { + $this->showErrorNotification(trans('errors.openid_fail_authed', ['system' => config('saml2.name')])); + return redirect('/login'); + } + + return redirect()->intended(); + } +} diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php index 97ec31dcc..5db3c59bd 100644 --- a/app/Http/Controllers/UserController.php +++ b/app/Http/Controllers/UserController.php @@ -76,7 +76,7 @@ class UserController extends Controller if ($authMethod === 'standard' && !$sendInvite) { $validationRules['password'] = 'required|min:6'; $validationRules['password-confirm'] = 'required|same:password'; - } elseif ($authMethod === 'ldap' || $authMethod === 'saml2') { + } elseif ($authMethod === 'ldap' || $authMethod === 'saml2' || $authMethod === 'openid') { $validationRules['external_auth_id'] = 'required'; } $this->validate($request, $validationRules); @@ -85,7 +85,7 @@ class UserController extends Controller if ($authMethod === 'standard') { $user->password = bcrypt($request->get('password', Str::random(32))); - } elseif ($authMethod === 'ldap' || $authMethod === 'saml2') { + } elseif ($authMethod === 'ldap' || $authMethod === 'saml2' || $authMethod === 'openid') { $user->external_auth_id = $request->get('external_auth_id'); } diff --git a/app/Http/Middleware/VerifyCsrfToken.php b/app/Http/Middleware/VerifyCsrfToken.php index bdeb26554..007564eb3 100644 --- a/app/Http/Middleware/VerifyCsrfToken.php +++ b/app/Http/Middleware/VerifyCsrfToken.php @@ -19,6 +19,7 @@ class VerifyCsrfToken extends Middleware * @var array */ protected $except = [ - 'saml2/*' + 'saml2/*', + 'openid/*' ]; } diff --git a/app/Providers/AuthServiceProvider.php b/app/Providers/AuthServiceProvider.php index fe52df168..1b3f169ae 100644 --- a/app/Providers/AuthServiceProvider.php +++ b/app/Providers/AuthServiceProvider.php @@ -7,6 +7,7 @@ use BookStack\Api\ApiTokenGuard; use BookStack\Auth\Access\ExternalBaseUserProvider; use BookStack\Auth\Access\Guards\LdapSessionGuard; use BookStack\Auth\Access\Guards\Saml2SessionGuard; +use BookStack\Auth\Access\Guards\OpenIdSessionGuard; use BookStack\Auth\Access\LdapService; use BookStack\Auth\Access\RegistrationService; use BookStack\Auth\UserRepo; @@ -45,6 +46,16 @@ class AuthServiceProvider extends ServiceProvider $app[RegistrationService::class] ); }); + + Auth::extend('openid-session', function ($app, $name, array $config) { + $provider = Auth::createUserProvider($config['provider']); + return new OpenIdSessionGuard( + $name, + $provider, + $this->app['session.store'], + $app[RegistrationService::class] + ); + }); } /** diff --git a/composer.json b/composer.json index 59fc909d6..7b1a3d592 100644 --- a/composer.json +++ b/composer.json @@ -32,7 +32,8 @@ "socialiteproviders/microsoft-azure": "^3.0", "socialiteproviders/okta": "^1.0", "socialiteproviders/slack": "^3.0", - "socialiteproviders/twitch": "^5.0" + "socialiteproviders/twitch": "^5.0", + "steverhoades/oauth2-openid-connect-client": "^0.3.0" }, "require-dev": { "barryvdh/laravel-debugbar": "^3.2.8", diff --git a/composer.lock b/composer.lock index a8c3b1e50..0f5e29792 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "34390536dd685e0bc49b179babaa06ec", + "content-hash": "f9d604c2456771f9b939f04492dde182", "packages": [ { "name": "aws/aws-sdk-php", @@ -1814,6 +1814,71 @@ ], "time": "2020-02-04T15:30:01+00:00" }, + { + "name": "lcobucci/jwt", + "version": "3.3.2", + "source": { + "type": "git", + "url": "https://github.com/lcobucci/jwt.git", + "reference": "56f10808089e38623345e28af2f2d5e4eb579455" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/lcobucci/jwt/zipball/56f10808089e38623345e28af2f2d5e4eb579455", + "reference": "56f10808089e38623345e28af2f2d5e4eb579455", + "shasum": "" + }, + "require": { + "ext-mbstring": "*", + "ext-openssl": "*", + "php": "^5.6 || ^7.0" + }, + "require-dev": { + "mikey179/vfsstream": "~1.5", + "phpmd/phpmd": "~2.2", + "phpunit/php-invoker": "~1.1", + "phpunit/phpunit": "^5.7 || ^7.3", + "squizlabs/php_codesniffer": "~2.3" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "3.1-dev" + } + }, + "autoload": { + "psr-4": { + "Lcobucci\\JWT\\": "src" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "BSD-3-Clause" + ], + "authors": [ + { + "name": "Luís Otávio Cobucci Oblonczyk", + "email": "lcobucci@gmail.com", + "role": "Developer" + } + ], + "description": "A simple library to work with JSON Web Token and JSON Web Signature", + "keywords": [ + "JWS", + "jwt" + ], + "funding": [ + { + "url": "https://github.com/lcobucci", + "type": "github" + }, + { + "url": "https://www.patreon.com/lcobucci", + "type": "patreon" + } + ], + "time": "2020-05-22T08:21:12+00:00" + }, { "name": "league/commonmark", "version": "1.4.3", @@ -2114,6 +2179,73 @@ ], "time": "2016-08-17T00:36:58+00:00" }, + { + "name": "league/oauth2-client", + "version": "2.4.1", + "source": { + "type": "git", + "url": "https://github.com/thephpleague/oauth2-client.git", + "reference": "cc114abc622a53af969e8664722e84ca36257530" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/thephpleague/oauth2-client/zipball/cc114abc622a53af969e8664722e84ca36257530", + "reference": "cc114abc622a53af969e8664722e84ca36257530", + "shasum": "" + }, + "require": { + "guzzlehttp/guzzle": "^6.0", + "paragonie/random_compat": "^1|^2|^9.99", + "php": "^5.6|^7.0" + }, + "require-dev": { + "eloquent/liberator": "^2.0", + "eloquent/phony-phpunit": "^1.0|^3.0", + "jakub-onderka/php-parallel-lint": "^0.9.2", + "phpunit/phpunit": "^5.7|^6.0", + "squizlabs/php_codesniffer": "^2.3|^3.0" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-2.x": "2.0.x-dev" + } + }, + "autoload": { + "psr-4": { + "League\\OAuth2\\Client\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Alex Bilbie", + "email": "hello@alexbilbie.com", + "homepage": "http://www.alexbilbie.com", + "role": "Developer" + }, + { + "name": "Woody Gilk", + "homepage": "https://github.com/shadowhand", + "role": "Contributor" + } + ], + "description": "OAuth 2.0 Client Library", + "keywords": [ + "Authentication", + "SSO", + "authorization", + "identity", + "idp", + "oauth", + "oauth2", + "single sign on" + ], + "time": "2018-11-22T18:33:57+00:00" + }, { "name": "monolog/monolog", "version": "2.1.0", @@ -3502,6 +3634,49 @@ "description": "Twitch OAuth2 Provider for Laravel Socialite", "time": "2020-05-06T22:51:30+00:00" }, + { + "name": "steverhoades/oauth2-openid-connect-client", + "version": "v0.3.0", + "source": { + "type": "git", + "url": "https://github.com/steverhoades/oauth2-openid-connect-client.git", + "reference": "0159471487540a4620b8d0b693f5f215503a8d75" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/steverhoades/oauth2-openid-connect-client/zipball/0159471487540a4620b8d0b693f5f215503a8d75", + "reference": "0159471487540a4620b8d0b693f5f215503a8d75", + "shasum": "" + }, + "require": { + "lcobucci/jwt": "^3.2", + "league/oauth2-client": "^2.0" + }, + "require-dev": { + "phpmd/phpmd": "~2.2", + "phpunit/php-invoker": "~1.1", + "phpunit/phpunit": "~4.5", + "squizlabs/php_codesniffer": "~2.3" + }, + "type": "library", + "autoload": { + "psr-4": { + "OpenIDConnectClient\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Steve Rhoades", + "email": "sedonami@gmail.com" + } + ], + "description": "OAuth2 OpenID Connect Client that utilizes the PHP Leagues OAuth2 Client", + "time": "2020-05-19T23:06:36+00:00" + }, { "name": "swiftmailer/swiftmailer", "version": "v6.2.3", diff --git a/resources/lang/en/errors.php b/resources/lang/en/errors.php index 06a5285f5..ec4ce813e 100644 --- a/resources/lang/en/errors.php +++ b/resources/lang/en/errors.php @@ -23,6 +23,10 @@ return [ 'saml_no_email_address' => 'Could not find an email address, for this user, in the data provided by the external authentication system', 'saml_invalid_response_id' => 'The request from the external authentication system is not recognised by a process started by this application. Navigating back after a login could cause this issue.', 'saml_fail_authed' => 'Login using :system failed, system did not provide successful authorization', + 'openid_already_logged_in' => 'Already logged in', + 'openid_user_not_registered' => 'The user :name is not registered and automatic registration is disabled', + 'openid_no_email_address' => 'Could not find an email address, for this user, in the data provided by the external authentication system', + 'openid_fail_authed' => 'Login using :system failed, system did not provide successful authorization', 'social_no_action_defined' => 'No action defined', 'social_login_bad_response' => "Error received during :socialAccount login: \n:error", 'social_account_in_use' => 'This :socialAccount account is already in use, Try logging in via the :socialAccount option.', diff --git a/resources/views/auth/forms/login/openid.blade.php b/resources/views/auth/forms/login/openid.blade.php new file mode 100644 index 000000000..b1ef51d13 --- /dev/null +++ b/resources/views/auth/forms/login/openid.blade.php @@ -0,0 +1,11 @@ +
+ {!! csrf_field() !!} + +
+ +
+ +
\ No newline at end of file diff --git a/resources/views/common/header.blade.php b/resources/views/common/header.blade.php index 827abcac6..996d44d27 100644 --- a/resources/views/common/header.blade.php +++ b/resources/views/common/header.blade.php @@ -66,6 +66,8 @@
  • @if(config('auth.method') === 'saml2') @icon('logout'){{ trans('auth.logout') }} + @elseif(config('auth.method') === 'openid') + @icon('logout'){{ trans('auth.logout') }} @else @icon('logout'){{ trans('auth.logout') }} @endif diff --git a/resources/views/settings/index.blade.php b/resources/views/settings/index.blade.php index 6ccb8d8f9..500db64e6 100644 --- a/resources/views/settings/index.blade.php +++ b/resources/views/settings/index.blade.php @@ -224,7 +224,7 @@ 'label' => trans('settings.reg_enable_toggle') ]) - @if(in_array(config('auth.method'), ['ldap', 'saml2'])) + @if(in_array(config('auth.method'), ['ldap', 'saml2', 'openid']))
    {{ trans('settings.reg_enable_external_warning') }}
    @endif diff --git a/resources/views/settings/roles/form.blade.php b/resources/views/settings/roles/form.blade.php index ed57ad944..b5aa96911 100644 --- a/resources/views/settings/roles/form.blade.php +++ b/resources/views/settings/roles/form.blade.php @@ -19,7 +19,7 @@ @include('form.text', ['name' => 'description']) - @if(config('auth.method') === 'ldap' || config('auth.method') === 'saml2') + @if(config('auth.method') === 'ldap' || config('auth.method') === 'saml2' || config('auth.method') === 'openid')
    @include('form.text', ['name' => 'external_auth_id']) diff --git a/resources/views/users/form.blade.php b/resources/views/users/form.blade.php index df3d06c2f..f3e8cedff 100644 --- a/resources/views/users/form.blade.php +++ b/resources/views/users/form.blade.php @@ -25,7 +25,7 @@
    -@if(($authMethod === 'ldap' || $authMethod === 'saml2') && userCan('users-manage')) +@if(($authMethod === 'ldap' || $authMethod === 'saml2' || $authMethod === 'openid') && userCan('users-manage'))
    diff --git a/routes/web.php b/routes/web.php index 6b7911825..a47080e8e 100644 --- a/routes/web.php +++ b/routes/web.php @@ -234,6 +234,11 @@ Route::get('/saml2/metadata', 'Auth\Saml2Controller@metadata'); Route::get('/saml2/sls', 'Auth\Saml2Controller@sls'); Route::post('/saml2/acs', 'Auth\Saml2Controller@acs'); +// OpenId routes +Route::post('/openid/login', 'Auth\OpenIdController@login'); +Route::get('/openid/logout', 'Auth\OpenIdController@logout'); +Route::get('/openid/redirect', 'Auth\OpenIdController@redirect'); + // User invitation routes Route::get('/register/invite/{token}', 'Auth\UserInviteController@showSetPassword'); Route::post('/register/invite/{token}', 'Auth\UserInviteController@setPassword'); diff --git a/tests/Auth/AuthTest.php b/tests/Auth/AuthTest.php index f1f476966..779d5e70f 100644 --- a/tests/Auth/AuthTest.php +++ b/tests/Auth/AuthTest.php @@ -387,6 +387,7 @@ class AuthTest extends BrowserKitTest $this->assertTrue(auth()->check()); $this->assertTrue(auth('ldap')->check()); $this->assertTrue(auth('saml2')->check()); + $this->assertTrue(auth('openid')->check()); } public function test_login_authenticates_nonadmins_on_default_guard_only() @@ -399,6 +400,7 @@ class AuthTest extends BrowserKitTest $this->assertTrue(auth()->check()); $this->assertFalse(auth('ldap')->check()); $this->assertFalse(auth('saml2')->check()); + $this->assertFalse(auth('openid')->check()); } /** From 25144a13c7150c75a023cb039972a3f784bee8cf Mon Sep 17 00:00:00 2001 From: Jasper Weyne Date: Mon, 6 Jul 2020 18:14:43 +0200 Subject: [PATCH 02/13] Deduplicated getOrRegisterUser method --- app/Auth/Access/ExternalAuthService.php | 37 +++++++++++++++++++++++++ app/Auth/Access/OpenIdService.php | 32 ++------------------- app/Auth/Access/Saml2Service.php | 32 ++------------------- 3 files changed, 41 insertions(+), 60 deletions(-) diff --git a/app/Auth/Access/ExternalAuthService.php b/app/Auth/Access/ExternalAuthService.php index db8bd2dfb..7f15307ae 100644 --- a/app/Auth/Access/ExternalAuthService.php +++ b/app/Auth/Access/ExternalAuthService.php @@ -3,9 +3,46 @@ use BookStack\Auth\Role; use BookStack\Auth\User; use Illuminate\Database\Eloquent\Builder; +use Illuminate\Support\Str; class ExternalAuthService { + protected $registrationService; + protected $user; + + /** + * ExternalAuthService base constructor. + */ + public function __construct(RegistrationService $registrationService, User $user) + { + $this->registrationService = $registrationService; + $this->user = $user; + } + + /** + * Get the user from the database for the specified details. + * @throws UserRegistrationException + */ + protected function getOrRegisterUser(array $userDetails): ?User + { + $user = $this->user->newQuery() + ->where('external_auth_id', '=', $userDetails['external_id']) + ->first(); + + if (is_null($user)) { + $userData = [ + 'name' => $userDetails['name'], + 'email' => $userDetails['email'], + 'password' => Str::random(32), + 'external_auth_id' => $userDetails['external_id'], + ]; + + $user = $this->registrationService->registerUser($userData, null, false); + } + + return $user; + } + /** * Check a role against an array of group names to see if it matches. * Checked against role 'external_auth_id' if set otherwise the name of the role. diff --git a/app/Auth/Access/OpenIdService.php b/app/Auth/Access/OpenIdService.php index 870299a57..084adfb13 100644 --- a/app/Auth/Access/OpenIdService.php +++ b/app/Auth/Access/OpenIdService.php @@ -5,7 +5,6 @@ use BookStack\Exceptions\JsonDebugException; use BookStack\Exceptions\OpenIdException; use BookStack\Exceptions\UserRegistrationException; use Exception; -use Illuminate\Support\Str; use Lcobucci\JWT\Token; use OpenIDConnectClient\AccessToken; use OpenIDConnectClient\OpenIDConnectProvider; @@ -17,17 +16,15 @@ use OpenIDConnectClient\OpenIDConnectProvider; class OpenIdService extends ExternalAuthService { protected $config; - protected $registrationService; - protected $user; /** * OpenIdService constructor. */ public function __construct(RegistrationService $registrationService, User $user) { + parent::__construct($registrationService, $user); + $this->config = config('openid'); - $this->registrationService = $registrationService; - $this->user = $user; } /** @@ -175,31 +172,6 @@ class OpenIdService extends ExternalAuthService ]; } - /** - * Get the user from the database for the specified details. - * @throws OpenIdException - * @throws UserRegistrationException - */ - protected function getOrRegisterUser(array $userDetails): ?User - { - $user = $this->user->newQuery() - ->where('external_auth_id', '=', $userDetails['external_id']) - ->first(); - - if (is_null($user)) { - $userData = [ - 'name' => $userDetails['name'], - 'email' => $userDetails['email'], - 'password' => Str::random(32), - 'external_auth_id' => $userDetails['external_id'], - ]; - - $user = $this->registrationService->registerUser($userData, null, false); - } - - return $user; - } - /** * Processes a received access token for a user. Login the user when * they exist, optionally registering them automatically. diff --git a/app/Auth/Access/Saml2Service.php b/app/Auth/Access/Saml2Service.php index 8f9a24cde..4c1fce864 100644 --- a/app/Auth/Access/Saml2Service.php +++ b/app/Auth/Access/Saml2Service.php @@ -5,7 +5,6 @@ use BookStack\Exceptions\JsonDebugException; use BookStack\Exceptions\SamlException; use BookStack\Exceptions\UserRegistrationException; use Exception; -use Illuminate\Support\Str; use OneLogin\Saml2\Auth; use OneLogin\Saml2\Error; use OneLogin\Saml2\IdPMetadataParser; @@ -18,17 +17,15 @@ use OneLogin\Saml2\ValidationError; class Saml2Service extends ExternalAuthService { protected $config; - protected $registrationService; - protected $user; /** * Saml2Service constructor. */ public function __construct(RegistrationService $registrationService, User $user) { + parent::__construct($registrationService, $user); + $this->config = config('saml2'); - $this->registrationService = $registrationService; - $this->user = $user; } /** @@ -309,31 +306,6 @@ class Saml2Service extends ExternalAuthService return $defaultValue; } - /** - * Get the user from the database for the specified details. - * @throws SamlException - * @throws UserRegistrationException - */ - protected function getOrRegisterUser(array $userDetails): ?User - { - $user = $this->user->newQuery() - ->where('external_auth_id', '=', $userDetails['external_id']) - ->first(); - - if (is_null($user)) { - $userData = [ - 'name' => $userDetails['name'], - 'email' => $userDetails['email'], - 'password' => Str::random(32), - 'external_auth_id' => $userDetails['external_id'], - ]; - - $user = $this->registrationService->registerUser($userData, null, false); - } - - return $user; - } - /** * Process the SAML response for a user. Login the user when * they exist, optionally registering them automatically. From 10c890947f9ea5661729f88e9e85464522498dd7 Mon Sep 17 00:00:00 2001 From: Jasper Weyne Date: Tue, 7 Jul 2020 02:26:00 +0200 Subject: [PATCH 03/13] Token expiration and refreshing using the refresh_token flow --- app/Auth/Access/Guards/OpenIdSessionGuard.php | 40 ++++++++++++++++ app/Auth/Access/OpenIdService.php | 46 ++++++++++++++++++- 2 files changed, 84 insertions(+), 2 deletions(-) diff --git a/app/Auth/Access/Guards/OpenIdSessionGuard.php b/app/Auth/Access/Guards/OpenIdSessionGuard.php index 0dfbb67a3..634464493 100644 --- a/app/Auth/Access/Guards/OpenIdSessionGuard.php +++ b/app/Auth/Access/Guards/OpenIdSessionGuard.php @@ -2,6 +2,11 @@ namespace BookStack\Auth\Access\Guards; +use BookStack\Auth\Access\OpenIdService; +use BookStack\Auth\Access\RegistrationService; +use Illuminate\Contracts\Auth\UserProvider; +use Illuminate\Contracts\Session\Session; + /** * OpenId Session Guard * @@ -14,6 +19,41 @@ namespace BookStack\Auth\Access\Guards; */ class OpenIdSessionGuard extends ExternalBaseSessionGuard { + + protected $openidService; + + /** + * OpenIdSessionGuard constructor. + */ + public function __construct( + $name, + UserProvider $provider, + Session $session, + OpenIdService $openidService, + RegistrationService $registrationService + ) { + $this->openidService = $openidService; + parent::__construct($name, $provider, $session, $registrationService); + } + + /** + * Get the currently authenticated user. + * + * @return \Illuminate\Contracts\Auth\Authenticatable|null + */ + public function user() + { + // retrieve the current user + $user = parent::user(); + + // refresh the current user + if ($user && !$this->openidService->refresh()) { + $this->user = null; + } + + return $this->user; + } + /** * Validate a user's credentials. * diff --git a/app/Auth/Access/OpenIdService.php b/app/Auth/Access/OpenIdService.php index 084adfb13..377925d61 100644 --- a/app/Auth/Access/OpenIdService.php +++ b/app/Auth/Access/OpenIdService.php @@ -6,6 +6,7 @@ use BookStack\Exceptions\OpenIdException; use BookStack\Exceptions\UserRegistrationException; use Exception; use Lcobucci\JWT\Token; +use League\OAuth2\Client\Provider\Exception\IdentityProviderException; use OpenIDConnectClient\AccessToken; use OpenIDConnectClient\OpenIDConnectProvider; @@ -53,6 +54,46 @@ class OpenIdService extends ExternalAuthService return ['url' => $url, 'id' => $id]; } + /** + * Refresh the currently logged in user. + * @throws Error + */ + public function refresh(): bool + { + // Retrieve access token for current session + $json = session()->get('openid_token'); + $accessToken = new AccessToken(json_decode($json, true)); + + // Check whether the access token or ID token is expired + if (!$accessToken->getIdToken()->isExpired() && !$accessToken->hasExpired()) { + return true; + } + + // If no refresh token available, logout + if ($accessToken->getRefreshToken() === null) { + $this->actionLogout(); + return false; + } + + // ID token or access token is expired, we refresh it using the refresh token + try { + $provider = $this->getProvider(); + + $accessToken = $provider->getAccessToken('refresh_token', [ + 'refresh_token' => $accessToken->getRefreshToken(), + ]); + } catch (IdentityProviderException $e) { + // Refreshing failed, logout + $this->actionLogout(); + return false; + } + + // A valid token was obtained, we update the access token + session()->put('openid_token', json_encode($accessToken)); + + return true; + } + /** * Process the Authorization response from the authorization server and * return the matching, or new if registration active, user matched to @@ -86,7 +127,7 @@ class OpenIdService extends ExternalAuthService } /** - * Load the underlying Onelogin SAML2 toolkit. + * Load the underlying OpenID Connect Provider. * @throws Error * @throws Exception */ @@ -155,7 +196,7 @@ class OpenIdService extends ExternalAuthService } /** - * Extract the details of a user from a SAML response. + * Extract the details of a user from an ID token. */ protected function getUserDetails(Token $token): array { @@ -202,6 +243,7 @@ class OpenIdService extends ExternalAuthService } auth()->login($user); + session()->put('openid_token', json_encode($accessToken)); return $user; } } From 5df7db510524a156a0a1f0d659a06a02dd5d3644 Mon Sep 17 00:00:00 2001 From: Jasper Weyne Date: Tue, 7 Jul 2020 02:51:33 +0200 Subject: [PATCH 04/13] Ignore ID token expiry if unavailable --- app/Auth/Access/OpenIdService.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/app/Auth/Access/OpenIdService.php b/app/Auth/Access/OpenIdService.php index 377925d61..7b651c3de 100644 --- a/app/Auth/Access/OpenIdService.php +++ b/app/Auth/Access/OpenIdService.php @@ -8,6 +8,7 @@ use Exception; use Lcobucci\JWT\Token; use League\OAuth2\Client\Provider\Exception\IdentityProviderException; use OpenIDConnectClient\AccessToken; +use OpenIDConnectClient\Exception\InvalidTokenException; use OpenIDConnectClient\OpenIDConnectProvider; /** @@ -64,8 +65,9 @@ class OpenIdService extends ExternalAuthService $json = session()->get('openid_token'); $accessToken = new AccessToken(json_decode($json, true)); - // Check whether the access token or ID token is expired - if (!$accessToken->getIdToken()->isExpired() && !$accessToken->hasExpired()) { + // Check if both the access token and the ID token (if present) are unexpired + $idToken = $accessToken->getIdToken(); + if (!$accessToken->hasExpired() && (!$idToken || !$idToken->isExpired())) { return true; } @@ -86,6 +88,9 @@ class OpenIdService extends ExternalAuthService // Refreshing failed, logout $this->actionLogout(); return false; + } catch (InvalidTokenException $e) { + // A refresh token doesn't necessarily contain + // an ID token, ignore this exception } // A valid token was obtained, we update the access token From 97cde9c56a3268da179c2701d209a9a1224bac85 Mon Sep 17 00:00:00 2001 From: Jasper Weyne Date: Wed, 8 Jul 2020 17:02:52 +0200 Subject: [PATCH 05/13] Generalize refresh failure handling --- app/Auth/Access/OpenIdService.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/Auth/Access/OpenIdService.php b/app/Auth/Access/OpenIdService.php index 7b651c3de..14b6ac9a5 100644 --- a/app/Auth/Access/OpenIdService.php +++ b/app/Auth/Access/OpenIdService.php @@ -88,9 +88,10 @@ class OpenIdService extends ExternalAuthService // Refreshing failed, logout $this->actionLogout(); return false; - } catch (InvalidTokenException $e) { - // A refresh token doesn't necessarily contain - // an ID token, ignore this exception + } catch (\Exception $e) { + // Unknown error, logout and throw + $this->actionLogout(); + throw $e; } // A valid token was obtained, we update the access token From 13d0260cc97c5cce9399f44afa65b70857499da6 Mon Sep 17 00:00:00 2001 From: Jasper Weyne Date: Thu, 9 Jul 2020 16:27:45 +0200 Subject: [PATCH 06/13] Configurable OpenID Connect services --- app/Auth/Access/OpenIdService.php | 22 +++++++++++++++++++--- app/Config/openid.php | 3 +++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/app/Auth/Access/OpenIdService.php b/app/Auth/Access/OpenIdService.php index 14b6ac9a5..fc0c00298 100644 --- a/app/Auth/Access/OpenIdService.php +++ b/app/Auth/Access/OpenIdService.php @@ -139,6 +139,7 @@ class OpenIdService extends ExternalAuthService */ protected function getProvider(): OpenIDConnectProvider { + // Setup settings $settings = $this->config['openid']; $overrides = $this->config['openid_overrides'] ?? []; @@ -149,12 +150,27 @@ class OpenIdService extends ExternalAuthService $openIdSettings = $this->loadOpenIdDetails(); $settings = array_replace_recursive($settings, $openIdSettings, $overrides); - $signer = new \Lcobucci\JWT\Signer\Rsa\Sha256(); - return new OpenIDConnectProvider($settings, ['signer' => $signer]); + // Setup services + $services = $this->loadOpenIdServices(); + $overrides = $this->config['openid_services'] ?? []; + + $services = array_replace_recursive($services, $overrides); + + return new OpenIDConnectProvider($settings, $services); } /** - * Load dynamic service provider options required by the onelogin toolkit. + * Load services utilized by the OpenID Connect provider. + */ + protected function loadOpenIdServices(): array + { + return [ + 'signer' => new \Lcobucci\JWT\Signer\Rsa\Sha256(), + ]; + } + + /** + * Load dynamic service provider options required by the OpenID Connect provider. */ protected function loadOpenIdDetails(): array { diff --git a/app/Config/openid.php b/app/Config/openid.php index 2232ba7b2..20089518b 100644 --- a/app/Config/openid.php +++ b/app/Config/openid.php @@ -18,6 +18,9 @@ return [ // Overrides, in JSON format, to the configuration passed to underlying OpenIDConnectProvider library. 'openid_overrides' => env('OPENID_OVERRIDES', null), + // Custom service instances, used by the underlying OpenIDConnectProvider library + 'openid_services' => [], + 'openid' => [ // OAuth2/OpenId client id, as configured in your Authorization server. 'clientId' => env('OPENID_CLIENT_ID', ''), From 75b4a05200ebf6b107b4448915f811f247bcba69 Mon Sep 17 00:00:00 2001 From: Jasper Weyne Date: Thu, 9 Jul 2020 18:00:16 +0200 Subject: [PATCH 07/13] Add OpenIdService to OpenIdSessionGuard constructor call --- app/Providers/AuthServiceProvider.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/Providers/AuthServiceProvider.php b/app/Providers/AuthServiceProvider.php index 1b3f169ae..653a29248 100644 --- a/app/Providers/AuthServiceProvider.php +++ b/app/Providers/AuthServiceProvider.php @@ -9,6 +9,7 @@ use BookStack\Auth\Access\Guards\LdapSessionGuard; use BookStack\Auth\Access\Guards\Saml2SessionGuard; use BookStack\Auth\Access\Guards\OpenIdSessionGuard; use BookStack\Auth\Access\LdapService; +use BookStack\Auth\Access\OpenIdService; use BookStack\Auth\Access\RegistrationService; use BookStack\Auth\UserRepo; use Illuminate\Support\ServiceProvider; @@ -53,6 +54,7 @@ class AuthServiceProvider extends ServiceProvider $name, $provider, $this->app['session.store'], + $app[OpenIdService::class], $app[RegistrationService::class] ); }); From 46388a591b7cff9364dff2502419ffdafab0137c Mon Sep 17 00:00:00 2001 From: Jasper Weyne Date: Thu, 9 Jul 2020 18:29:44 +0200 Subject: [PATCH 08/13] AccessToken empty array parameter on null --- app/Auth/Access/OpenIdService.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Auth/Access/OpenIdService.php b/app/Auth/Access/OpenIdService.php index fc0c00298..3d8818fa5 100644 --- a/app/Auth/Access/OpenIdService.php +++ b/app/Auth/Access/OpenIdService.php @@ -63,7 +63,7 @@ class OpenIdService extends ExternalAuthService { // Retrieve access token for current session $json = session()->get('openid_token'); - $accessToken = new AccessToken(json_decode($json, true)); + $accessToken = new AccessToken(json_decode($json, true) ?? []); // Check if both the access token and the ID token (if present) are unexpired $idToken = $accessToken->getIdToken(); From 6feaf25c902d8cf1315ca0612e3f54387dbb55f4 Mon Sep 17 00:00:00 2001 From: Jasper Weyne Date: Tue, 4 Aug 2020 21:29:11 +0200 Subject: [PATCH 09/13] Increase robustness of the refresh method --- app/Auth/Access/OpenIdService.php | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/app/Auth/Access/OpenIdService.php b/app/Auth/Access/OpenIdService.php index 3d8818fa5..2b536d492 100644 --- a/app/Auth/Access/OpenIdService.php +++ b/app/Auth/Access/OpenIdService.php @@ -8,7 +8,6 @@ use Exception; use Lcobucci\JWT\Token; use League\OAuth2\Client\Provider\Exception\IdentityProviderException; use OpenIDConnectClient\AccessToken; -use OpenIDConnectClient\Exception\InvalidTokenException; use OpenIDConnectClient\OpenIDConnectProvider; /** @@ -63,11 +62,20 @@ class OpenIdService extends ExternalAuthService { // Retrieve access token for current session $json = session()->get('openid_token'); + + // If no access token was found, reject the refresh + if (!$json) { + $this->actionLogout(); + return false; + } + $accessToken = new AccessToken(json_decode($json, true) ?? []); // Check if both the access token and the ID token (if present) are unexpired $idToken = $accessToken->getIdToken(); - if (!$accessToken->hasExpired() && (!$idToken || !$idToken->isExpired())) { + $accessTokenUnexpired = $accessToken->getExpires() && !$accessToken->hasExpired(); + $idTokenUnexpired = !$idToken || !$idToken->isExpired(); + if ($accessTokenUnexpired && $idTokenUnexpired) { return true; } From 23402ae81287bdfd0539d20a3a81c38d9efce1e5 Mon Sep 17 00:00:00 2001 From: Jasper Weyne Date: Tue, 4 Aug 2020 21:30:17 +0200 Subject: [PATCH 10/13] Initial unit tests for OpenID --- tests/Auth/OpenIdTest.php | 112 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 tests/Auth/OpenIdTest.php diff --git a/tests/Auth/OpenIdTest.php b/tests/Auth/OpenIdTest.php new file mode 100644 index 000000000..9ad90fb5b --- /dev/null +++ b/tests/Auth/OpenIdTest.php @@ -0,0 +1,112 @@ +set([ + 'auth.method' => 'openid', + 'auth.defaults.guard' => 'openid', + 'openid.name' => 'SingleSignOn-Testing', + 'openid.email_attribute' => 'email', + 'openid.display_name_attributes' => ['given_name', 'family_name'], + 'openid.external_id_attribute' => 'uid', + 'openid.openid_overrides' => null, + 'openid.openid.clientId' => 'testapp', + 'openid.openid.clientSecret' => 'testpass', + 'openid.openid.publicKey' => $this->testCert, + 'openid.openid.idTokenIssuer' => 'https://openid.local', + 'openid.openid.urlAuthorize' => 'https://openid.local/auth', + 'openid.openid.urlAccessToken' => 'https://openid.local/token', + ]); + } + + public function test_openid_overrides_functions_as_expected() + { + $json = '{"urlAuthorize": "https://openid.local/custom"}'; + config()->set(['openid.openid_overrides' => $json]); + + $req = $this->get('/openid/login'); + $redirect = $req->headers->get('location'); + $this->assertStringStartsWith('https://openid.local/custom', $redirect, 'Login redirects to SSO location'); + } + + public function test_login_option_shows_on_login_page() + { + $req = $this->get('/login'); + $req->assertSeeText('SingleSignOn-Testing'); + $req->assertElementExists('form[action$="/openid/login"][method=POST] button'); + } + + public function test_login() + { + $req = $this->post('/openid/login'); + $redirect = $req->headers->get('location'); + + $this->assertStringStartsWith('https://openid.local/auth', $redirect, 'Login redirects to SSO location'); + $this->assertFalse($this->isAuthenticated()); + } + + public function test_openid_routes_are_only_active_if_openid_enabled() + { + config()->set(['auth.method' => 'standard']); + $getRoutes = ['/logout', '/metadata', '/sls']; + foreach ($getRoutes as $route) { + $req = $this->get('/openid' . $route); + $this->assertPermissionError($req); + } + + $postRoutes = ['/login', '/acs']; + foreach ($postRoutes as $route) { + $req = $this->post('/openid' . $route); + $this->assertPermissionError($req); + } + } + + public function test_forgot_password_routes_inaccessible() + { + $resp = $this->get('/password/email'); + $this->assertPermissionError($resp); + + $resp = $this->post('/password/email'); + $this->assertPermissionError($resp); + + $resp = $this->get('/password/reset/abc123'); + $this->assertPermissionError($resp); + + $resp = $this->post('/password/reset'); + $this->assertPermissionError($resp); + } + + public function test_standard_login_routes_inaccessible() + { + $resp = $this->post('/login'); + $this->assertPermissionError($resp); + + $resp = $this->get('/logout'); + $this->assertPermissionError($resp); + } + + public function test_user_invite_routes_inaccessible() + { + $resp = $this->get('/register/invite/abc123'); + $this->assertPermissionError($resp); + + $resp = $this->post('/register/invite/abc123'); + $this->assertPermissionError($resp); + } + + public function test_user_register_routes_inaccessible() + { + $resp = $this->get('/register'); + $this->assertPermissionError($resp); + + $resp = $this->post('/register'); + $this->assertPermissionError($resp); + } +} From f2d320825a34b425457954b832bccd3a6ed56cfd Mon Sep 17 00:00:00 2001 From: Jasper Weyne Date: Tue, 4 Aug 2020 22:09:53 +0200 Subject: [PATCH 11/13] Simplify refresh method --- app/Auth/Access/OpenIdService.php | 65 +++++++++++++++++++------------ 1 file changed, 40 insertions(+), 25 deletions(-) diff --git a/app/Auth/Access/OpenIdService.php b/app/Auth/Access/OpenIdService.php index 2b536d492..4eea3c252 100644 --- a/app/Auth/Access/OpenIdService.php +++ b/app/Auth/Access/OpenIdService.php @@ -71,41 +71,56 @@ class OpenIdService extends ExternalAuthService $accessToken = new AccessToken(json_decode($json, true) ?? []); - // Check if both the access token and the ID token (if present) are unexpired - $idToken = $accessToken->getIdToken(); - $accessTokenUnexpired = $accessToken->getExpires() && !$accessToken->hasExpired(); - $idTokenUnexpired = !$idToken || !$idToken->isExpired(); - if ($accessTokenUnexpired && $idTokenUnexpired) { + // If the token is not expired, refreshing isn't necessary + if ($this->isUnexpired($accessToken)) { return true; } - // If no refresh token available, logout - if ($accessToken->getRefreshToken() === null) { - $this->actionLogout(); - return false; - } - - // ID token or access token is expired, we refresh it using the refresh token + // Try to obtain refreshed access token try { - $provider = $this->getProvider(); - - $accessToken = $provider->getAccessToken('refresh_token', [ - 'refresh_token' => $accessToken->getRefreshToken(), - ]); - } catch (IdentityProviderException $e) { - // Refreshing failed, logout - $this->actionLogout(); - return false; + $newAccessToken = $this->refreshAccessToken($accessToken); } catch (\Exception $e) { - // Unknown error, logout and throw + // Log out if an unknown problem arises $this->actionLogout(); throw $e; } - // A valid token was obtained, we update the access token - session()->put('openid_token', json_encode($accessToken)); + // If a token was obtained, update the access token, otherwise log out + if ($newAccessToken !== null) { + session()->put('openid_token', json_encode($newAccessToken)); + return true; + } else { + $this->actionLogout(); + return false; + } + } - return true; + protected function isUnexpired(AccessToken $accessToken): bool + { + $idToken = $accessToken->getIdToken(); + + $accessTokenUnexpired = $accessToken->getExpires() && !$accessToken->hasExpired(); + $idTokenUnexpired = !$idToken || !$idToken->isExpired(); + + return $accessTokenUnexpired && $idTokenUnexpired; + } + + protected function refreshAccessToken(AccessToken $accessToken): ?AccessToken + { + // If no refresh token available, abort + if ($accessToken->getRefreshToken() === null) { + return null; + } + + // ID token or access token is expired, we refresh it using the refresh token + try { + return $this->getProvider()->getAccessToken('refresh_token', [ + 'refresh_token' => $accessToken->getRefreshToken(), + ]); + } catch (IdentityProviderException $e) { + // Refreshing failed + return null; + } } /** From 35c48b94163d8a17bcdd9a9fb360a3f43f1cd2b5 Mon Sep 17 00:00:00 2001 From: Jasper Weyne Date: Wed, 5 Aug 2020 00:18:43 +0200 Subject: [PATCH 12/13] Method descriptions --- app/Auth/Access/OpenIdService.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/app/Auth/Access/OpenIdService.php b/app/Auth/Access/OpenIdService.php index 4eea3c252..70df963f5 100644 --- a/app/Auth/Access/OpenIdService.php +++ b/app/Auth/Access/OpenIdService.php @@ -95,6 +95,9 @@ class OpenIdService extends ExternalAuthService } } + /** + * Check whether an access token or OpenID token isn't expired. + */ protected function isUnexpired(AccessToken $accessToken): bool { $idToken = $accessToken->getIdToken(); @@ -105,6 +108,10 @@ class OpenIdService extends ExternalAuthService return $accessTokenUnexpired && $idTokenUnexpired; } + /** + * Generate an updated access token, through the associated refresh token. + * @throws Error + */ protected function refreshAccessToken(AccessToken $accessToken): ?AccessToken { // If no refresh token available, abort From 69a47319d51efb3d39bb36eff4df9ca2c31165ae Mon Sep 17 00:00:00 2001 From: Jasper Weyne Date: Wed, 5 Aug 2020 13:14:46 +0200 Subject: [PATCH 13/13] Default OpenID display name set to standard value --- app/Config/openid.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Config/openid.php b/app/Config/openid.php index 20089518b..90eb6d5db 100644 --- a/app/Config/openid.php +++ b/app/Config/openid.php @@ -11,7 +11,7 @@ return [ // Attribute, within a OpenId token, to find the user's email address 'email_attribute' => env('OPENID_EMAIL_ATTRIBUTE', 'email'), // Attribute, within a OpenId token, to find the user's display name - 'display_name_attributes' => explode('|', env('OPENID_DISPLAY_NAME_ATTRIBUTES', 'username')), + 'display_name_attributes' => explode('|', env('OPENID_DISPLAY_NAME_ATTRIBUTES', 'name')), // Attribute, within a OpenId token, to use to connect a BookStack user to the OpenId user. 'external_id_attribute' => env('OPENID_EXTERNAL_ID_ATTRIBUTE', null),