diff --git a/htdocs/admin/oauth.php b/htdocs/admin/oauth.php index 121663e75bb..78f68401bb8 100644 --- a/htdocs/admin/oauth.php +++ b/htdocs/admin/oauth.php @@ -101,7 +101,9 @@ if ($action == 'update') { } } if (GETPOSTISSET($constvalue.'_URL')) { - if (!dolibarr_set_const($db, $newconstvalue.'_URL', GETPOST($constvalue.'_URL'), 'chaine', 0, '', $conf->entity)) { + $cleanurl = GETPOST($constvalue.'_URL'); + $cleanurl = preg_replace('/\/$/', '', $cleanurl); + if (!dolibarr_set_const($db, $newconstvalue.'_URL', $cleanurl, 'chaine', 0, '', $conf->entity)) { $error++; } } diff --git a/htdocs/admin/oauthlogintokens.php b/htdocs/admin/oauthlogintokens.php index 75b260beebe..99327e786ff 100644 --- a/htdocs/admin/oauthlogintokens.php +++ b/htdocs/admin/oauthlogintokens.php @@ -66,7 +66,6 @@ if (!$user->admin) { accessforbidden(); } - /* * Action */ diff --git a/htdocs/core/modules/oauth/generic_oauthcallback.php b/htdocs/core/modules/oauth/generic_oauthcallback.php index fd3cac893e7..832b20883b7 100644 --- a/htdocs/core/modules/oauth/generic_oauthcallback.php +++ b/htdocs/core/modules/oauth/generic_oauthcallback.php @@ -36,7 +36,6 @@ if (!defined('NOLOGIN') && $forlogin) { // Load Dolibarr environment require '../../../main.inc.php'; -require_once DOL_DOCUMENT_ROOT.'/includes/OAuth/bootstrap.php'; /** * @var Conf $conf * @var DoliDB $db @@ -45,8 +44,11 @@ require_once DOL_DOCUMENT_ROOT.'/includes/OAuth/bootstrap.php'; * * @var string $dolibarr_main_url_root */ +require_once DOL_DOCUMENT_ROOT.'/includes/OAuth/bootstrap.php'; + use OAuth\Common\Storage\DoliStorage; use OAuth\Common\Consumer\Credentials; +use OAuth\Common\Http\Uri\Uri; // Define $urlwithroot global $dolibarr_main_url_root; @@ -102,34 +104,56 @@ $statewithanticsrfonly = ''; $requestedpermissionsarray = array(); if ($state) { - // 'state' parameter is standard to store a hash value and can be used to retrieve some parameters back + // 'state' parameter is standard to store a hash value and can also be used to retrieve some parameters back $statewithscopeonly = preg_replace('/\-.*$/', '', preg_replace('/^forlogin-/', '', $state)); - $requestedpermissionsarray = explode(',', $statewithscopeonly); // Example: 'userinfo_email,userinfo_profile,openid,email,profile,cloud_print'. - $statewithanticsrfonly = preg_replace('/^.*\-/', '', $state); + if ($statewithscopeonly != 'none') { + $requestedpermissionsarray = explode(',', $statewithscopeonly); // Example: 'userinfo_email,userinfo_profile,openid,email,profile,cloud_print'. + $statewithanticsrfonly = preg_replace('/^.*\-/', '', $state); + } else { + $statewithscopeonly = ''; + } } // Add a test to check that the state parameter is provided into URL when we make the first call to ask the redirect or when we receive the callback // but not when callback was ok and we recall the page -if ($action != 'delete' && !GETPOST('afteroauthloginreturn') && (empty($statewithscopeonly) || empty($requestedpermissionsarray))) { - dol_syslog("state or statewithscopeonly and/or requestedpermissionsarray are empty"); - setEventMessages($langs->trans('ScopeUndefined'), null, 'errors'); - if (empty($backtourl)) { - $backtourl = DOL_URL_ROOT.'/'; +if ($action != 'delete' && !GETPOST('afteroauthloginreturn') && (empty($statewithscopeonly) || empty($requestedpermissionsarray)) && $state != 'none') { + if (GETPOST('error') || GETPOST('error_description')) { + setEventMessages($langs->trans("Error").' '.GETPOST('error_description'), null, 'errors'); + } else { + dol_syslog("state or statewithscopeonly and/or requestedpermissionsarray are empty"); + setEventMessages($langs->trans('ScopeUndefined'), null, 'errors'); + if (empty($backtourl)) { + $backtourl = DOL_URL_ROOT.'/'; + } + header('Location: '.$backtourl); + exit(); } - header('Location: '.$backtourl); - exit(); } -//var_dump($requestedpermissionsarray);exit; // Dolibarr storage $storage = new DoliStorage($db, $conf, $keyforprovider); -// Instantiate the Api service using the credentials, http client and storage mechanism for the token -// ucfirst(strtolower($genericstring)) must be the name of a class into OAuth/OAuth2/Services/Xxxx -$apiService = $serviceFactory->createService(ucfirst(strtolower($genericstring)), $credentials, $storage, $requestedpermissionsarray); +$keyforurl = 'OAUTH_'.$genericstring.($keyforprovider ? '-'.$keyforprovider : '').'_URL'; +if ($keyforprovider) { + $baseApiUriInt = new Uri(getDolGlobalString($keyforurl)); +} else { + print 'Error, failed to get value for constant '.$keyforurl; + exit; +} +$apiService = null; +$nameofservice = ucfirst(strtolower($genericstring)); +try { + // Instantiate the Api service using the credentials, http client and storage mechanism for the token + // ucfirst(strtolower($genericstring)) must be the name of a class into OAuth/OAuth2/Services/Xxxx + $apiService = $serviceFactory->createService($nameofservice, $credentials, $storage, $requestedpermissionsarray, $baseApiUriInt); + '@phan-var-force OAuth\OAuth2\Service\AbstractService|OAuth\OAuth1\Service\AbstractService $apiService'; // createService is only ServiceInterface +} catch (Exception $e) { + print 'Error, failed to create service for provider '.$nameofservice.($keyforprovider ? '-'.$keyforprovider : '').'. Message was: '.$e->getMessage(); + exit; +} /* var_dump($genericstring.($keyforprovider ? '-'.$keyforprovider : '')); var_dump($credentials); @@ -137,6 +161,7 @@ var_dump($storage); var_dump($requestedpermissionsarray); */ + if (empty($apiService) || !$apiService instanceof OAuth\OAuth2\Service\Generic) { print 'Error, failed to create Generic serviceFactory'; exit; @@ -181,6 +206,11 @@ if ($action == 'delete' && (!empty($user->admin) || $user->id == GETPOSTINT('use if (!GETPOST('code') && !GETPOST('error')) { dol_syslog("Page is called without the 'code' parameter defined"); + if (empty($state) || $state == 'none') { + // Generate a random state value to prevent CSRF attack. Store it into session juste after to check it when we will receive the callback from provider. + $state = bin2hex(random_bytes(16)); + } + // If we enter this page without 'code' parameter, it means we click on the link from login page ($forlogin is set) or from setup page and we want to get the redirect // to the OAuth provider login page. $_SESSION["backtourlsavedbeforeoauthjump"] = $backtourl; @@ -205,15 +235,17 @@ if (!GETPOST('code') && !GETPOST('error')) { // This may create record into oauth_state before the header redirect. // Creation of record with state, create record or just update column state of table llx_oauth_token (and create/update entry in llx_oauth_state) depending on the Provider used (see its constructor). - if ($state) { + //if ($state && $state != 'none') { $url = $apiService->getAuthorizationUri(array('client_id' => getDolGlobalString($keyforparamid), 'response_type' => 'code', 'state' => $state)); - } else { - $url = $apiService->getAuthorizationUri(array('client_id' => getDolGlobalString($keyforparamid), 'response_type' => 'code')); // Parameter state will be randomly generated - } + //} else { + // $url = $apiService->getAuthorizationUri(array('client_id' => getDolGlobalString($keyforparamid), 'response_type' => 'code')); // Parameter state will be randomly generated + //} // The redirect_uri is included into this $url // Add scopes - $url .= '&scope='.str_replace(',', '+', $statewithscopeonly); + if ($statewithscopeonly) { + $url .= '&scope='.str_replace(',', '+', $statewithscopeonly); + } // Add more param $url .= '&nonce='.bin2hex(random_bytes(64 / 8)); @@ -245,7 +277,7 @@ if (!GETPOST('code') && !GETPOST('error')) { } } - //var_dump($url);exit; + //var_dump($keyforurl, $url, $statewithscopeonly);exit; // we go on oauth provider authorization page, we will then go back on this page but into the other branch of the if (!GETPOST('code')) header('Location: '.$url); @@ -272,7 +304,6 @@ if (!GETPOST('code') && !GETPOST('error')) { // This requests the token from the received OAuth code (call of the endpoint) // Result is stored into object managed by class DoliStorage into includes/OAuth/Common/Storage/DoliStorage.php and into database table llx_oauth_token $token = $apiService->requestAccessToken(GETPOST('code'), $state); - '@phan-var-force OAuth\Common\Token\AbstractToken $token'; // The refresh token is inside the object token if the prompt was forced only. @@ -282,6 +313,13 @@ if (!GETPOST('code') && !GETPOST('error')) { // Note: The extraparams has the 'id_token' than contains a lot of information about the user. $extraparams = $token->getExtraParams(); + $scope = empty($extraparams['scope']) ? '' : $extraparams['scope']; + $tokenstring = $token->getAccessToken(); + // Update entry in llx_oauth_token to store the scope associated to the token into field "state" (field should be renamed). + // It is not stored by default by DoliStorage. + // TODO Update using $scope and $tokenstring + + $username = ''; $useremail = ''; diff --git a/htdocs/core/modules/oauth/github_oauthcallback.php b/htdocs/core/modules/oauth/github_oauthcallback.php index c8cb76c43fa..f91cdb05ede 100644 --- a/htdocs/core/modules/oauth/github_oauthcallback.php +++ b/htdocs/core/modules/oauth/github_oauthcallback.php @@ -98,9 +98,16 @@ if ($action != 'delete' && empty($requestedpermissionsarray)) { //var_dump($requestedpermissionsarray);exit; // Instantiate the Api service using the credentials, http client and storage mechanism for the token -$apiService = $serviceFactory->createService('GitHub', $credentials, $storage, $requestedpermissionsarray); -'@phan-var-force OAuth\OAuth2\Service\AbstractService|OAuth\OAuth1\Service\AbstractService $apiService'; // createService is only ServiceInterface - +$apiService = null; +$nameofservice = 'GitHub'; +try { + //$nameofservice = ucfirst(strtolower($genericstring)); + $apiService = $serviceFactory->createService($nameofservice, $credentials, $storage, $requestedpermissionsarray); + '@phan-var-force OAuth\OAuth2\Service\AbstractService|OAuth\OAuth1\Service\AbstractService $apiService'; // createService is only ServiceInterface +} catch (Exception $e) { + print 'Error, failed to create service for provider '.$nameofservice.($keyforprovider ? '-'.$keyforprovider : '').'. Message was: '.$e->getMessage(); + exit; +} // access type needed to have oauth provider refreshing token //$apiService->setAccessType('offline'); diff --git a/htdocs/core/modules/oauth/google_oauthcallback.php b/htdocs/core/modules/oauth/google_oauthcallback.php index 087fe34f654..b1f6c4ee588 100644 --- a/htdocs/core/modules/oauth/google_oauthcallback.php +++ b/htdocs/core/modules/oauth/google_oauthcallback.php @@ -133,9 +133,16 @@ $storage = new DoliStorage($db, $conf, $keyforprovider); // Instantiate the Api service using the credentials, http client and storage mechanism for the token // $requestedpermissionsarray contains list of scopes. // Conversion into URL is done by Reflection on constant with name SCOPE_scope_in_uppercase -$apiService = $serviceFactory->createService('Google', $credentials, $storage, $requestedpermissionsarray); -'@phan-var-force OAuth\OAuth2\Service\Google $apiService'; // createService is only ServiceInterface - +$apiService = null; +$nameofservice = 'Google'; +try { + //$nameofservice = ucfirst(strtolower($genericstring)); + $apiService = $serviceFactory->createService($nameofservice, $credentials, $storage, $requestedpermissionsarray); + '@phan-var-force OAuth\OAuth2\Service\Google $apiService'; // createService is only ServiceInterface +} catch (Exception $e) { + print 'Error, failed to create service for provider '.$nameofservice.($keyforprovider ? '-'.$keyforprovider : '').'. Message was: '.$e->getMessage(); + exit; +} // access type needed to have oauth provider refreshing token // also note that a refresh token is sent only after a prompt $apiService->setAccessType('offline'); diff --git a/htdocs/includes/OAuth/OAuth2/Service/Generic.php b/htdocs/includes/OAuth/OAuth2/Service/Generic.php index 4d4ab836b03..e6268abbe21 100644 --- a/htdocs/includes/OAuth/OAuth2/Service/Generic.php +++ b/htdocs/includes/OAuth/OAuth2/Service/Generic.php @@ -1,5 +1,7 @@ baseApiUri.'/oauth/request'); + $urltouse = (preg_match('/oauth2?\/?$/', $this->baseApiUri) ? $this->baseApiUri : $this->baseApiUri.'/oauth').'/request'; + + return new Uri($urltouse); } /** @@ -62,7 +66,8 @@ class Generic extends AbstractService */ public function getAuthorizationEndpoint() { - return new Uri($this->baseApiUri.'/oauth/authorize'); + $urltouse = (preg_match('/oauth2?\/?$/', $this->baseApiUri) ? $this->baseApiUri : $this->baseApiUri.'/oauth').'/authorize'; + return new Uri($urltouse); } /** @@ -70,7 +75,9 @@ class Generic extends AbstractService */ public function getAccessTokenEndpoint() { - return new Uri($this->baseApiUri.'/oauth/token'); + $urltouse = (preg_match('/oauth2?\/?$/', $this->baseApiUri) ? $this->baseApiUri : $this->baseApiUri.'/oauth').'/token'; + + return new Uri($urltouse); } /** @@ -140,7 +147,7 @@ class Generic extends AbstractService 'redirect_uri' => $this->credentials->getCallbackUrl(), 'grant_type' => 'authorization_code', 'code' => $code, - 'consumer_key' => $this->credentials->getConsumerId(), + 'consumer_key' => $this->credentials->getConsumerId() ); $responseBody = $this->httpClient->retrieveResponse( diff --git a/htdocs/install/mysql/tables/llx_oauth_token.sql b/htdocs/install/mysql/tables/llx_oauth_token.sql index b7483b1bb7a..326d33b55b2 100644 --- a/htdocs/install/mysql/tables/llx_oauth_token.sql +++ b/htdocs/install/mysql/tables/llx_oauth_token.sql @@ -20,9 +20,9 @@ CREATE TABLE llx_oauth_token ( service varchar(36), -- What king of key or token: 'Google', 'Stripe', 'auth-public-key', 'api', ... token text, -- token in serialize format, of an object StdOAuth2Token of library phpoauth2. Deprecated, use tokenstring instead. tokenstring text, -- token in json or text format. Value depends on 'service'. For example for an OAUTH service: '{"access_token": "sk_test_cccc", "refresh_token": "rt_aaa", "token_type": "bearer", ..., "scope": "read_write"} - tokenstring_refresh text NULL, -- token refresh in text format. Value depends on 'service'. + tokenstring_refresh text NULL, -- token refresh in text format. Value depends on 'service'. expire_at datetime NULL, - state text, -- the state (list of permission) the token was obtained if relevant + state text, -- the scope (list of permission) the token was obtained if relevant. Field has wrong name. We store sometimes here a value matching "scope-state" fk_soc integer, -- Id of thirdparty in llx_societe fk_user integer, -- Id of user in llx_user fk_adherent integer, -- Id of member in llx_adherent diff --git a/htdocs/langs/en_US/oauth.lang b/htdocs/langs/en_US/oauth.lang index 8365ff571da..1ffd65a7baa 100644 --- a/htdocs/langs/en_US/oauth.lang +++ b/htdocs/langs/en_US/oauth.lang @@ -40,7 +40,7 @@ URLOfOAuthServiceEndpointsExample=https://mastodon.example.com URLOfServiceForAuthorization=URL provided by OAuth service for authentication Scopes=Permissions (Scopes) ScopeUndefined=Permissions (Scopes) undefined (see previous tab) -ScopesDesc=Example: read,write with Mastodon +ScopesDesc=Example: "read", "write" with Mastodon or "none" TokenRawValue=Full Token (object) AccessToken=Access Token TokenExpired=Expired