diff --git a/install/empty_data.php b/install/empty_data.php index cc0b2f3e89c..8084c0703f9 100644 --- a/install/empty_data.php +++ b/install/empty_data.php @@ -9522,6 +9522,17 @@ public function getEmptyData(): array 'is_recursive' => 1, 'is_dynamic' => 0, ]; + + $tables['glpi_oauthclients'][] = [ + 'name' => 'Test E2E OAuth Client', + 'redirect_uri' => json_encode(["/api.php/oauth2/redirection"]), + 'grants' => json_encode(['authorization_code']), + 'scopes' => json_encode(['api', 'user']), + 'is_active' => 1, + 'is_confidential' => 1, + 'identifier' => '9246d35072ff62193330003a8106d947fafe5ac036d11a51ebc7ca11b9bc135e', + 'secret' => (new GLPIKey())->encrypt('d2c4f3b8a0e1f7b5c6a9d1e4f3b8a0e1f7b5c6a9d1e4f3b8a0e1f7b5c6a9d1') + ]; } return $tables; diff --git a/phpunit/functional/Glpi/Api/HL/Controller/CoreControllerTest.php b/phpunit/functional/Glpi/Api/HL/Controller/CoreControllerTest.php index 5a2c3d5dcbd..d95b0a57598 100644 --- a/phpunit/functional/Glpi/Api/HL/Controller/CoreControllerTest.php +++ b/phpunit/functional/Glpi/Api/HL/Controller/CoreControllerTest.php @@ -320,55 +320,6 @@ public function testOAuthPasswordGrantHeader() }); } - public function testOAuthAuthCodeGrant() - { - // Not a complete end to end test. Not sure how that could be done. Should probably be using Cypress. - global $DB; - - // Create an OAuth client - $client = new \OAuthClient(); - $client_id = $client->add([ - 'name' => __FUNCTION__, - 'is_active' => 1, - 'is_confidential' => 1, - ]); - $this->assertGreaterThan(0, $client_id); - - $client->update([ - 'id' => $client_id, - 'grants' => ['authorization_code'], - 'redirect_uri' => ["/api.php/oauth2/redirection"], - ]); - - // get client ID and secret - $it = $DB->request([ - 'SELECT' => ['identifier', 'secret', 'redirect_uri'], - 'FROM' => \OAuthClient::getTable(), - 'WHERE' => ['id' => $client_id], - ]); - $this->assertCount(1, $it); - $client_data = $it->current(); - - // Test authorize endpoint - $request = new Request('GET', '/Authorize', [], null); - $request = $request->withQueryParams([ - 'response_type' => 'code', - 'client_id' => $client_data['identifier'], - 'scope' => '', - 'redirect_uri' => json_decode($client_data['redirect_uri'])[0], - ]); - - $this->api->call($request, function ($call) { - /** @var \HLAPICallAsserter $call */ - $call->response - ->status(fn ($status) => $this->assertEquals(302, $status)) - ->headers(function ($headers) { - global $CFG_GLPI; - $this->assertMatchesRegularExpression('/^' . preg_quote($CFG_GLPI['url_base'], '/') . '\/\?redirect=/', $headers['Location']); - }); - }); - } - public function testOAuthClientCredentialsGrant(): void { global $DB; diff --git a/src/Glpi/Api/HL/Middleware/CookieAuthMiddleware.php b/src/Glpi/Api/HL/Middleware/CookieAuthMiddleware.php index 36145648241..5dafccca590 100644 --- a/src/Glpi/Api/HL/Middleware/CookieAuthMiddleware.php +++ b/src/Glpi/Api/HL/Middleware/CookieAuthMiddleware.php @@ -41,17 +41,18 @@ class CookieAuthMiddleware extends AbstractMiddleware implements AuthMiddlewareI { public function process(MiddlewareInput $input, callable $next): void { - $auth = new \Auth(); - if ($auth->getAlternateAuthSystemsUserLogin(\Auth::COOKIE)) { - // User could be authenticated by a cookie - // Need to use cookies for session and start it manually - ini_set('session.use_cookies', '1'); - Session::start(); + // User could be authenticated by a cookie + // Need to use cookies for session and start it manually + ini_set('session.use_cookies', '1'); + Session::setPath(); + Session::start(); + + if (($user_id = Session::getLoginUserID()) !== false) { // unset the response to indicate a successful auth $input->response = null; $input->client = [ 'client_id' => 'internal', // Internal just means the user was authenticated internally either by cookie or an already existing session. - 'users_id' => Session::getLoginUserID(), + 'users_id' => $user_id, 'scopes' => [] ]; } else { diff --git a/src/Glpi/Api/HL/Middleware/IPRestrictionRequestMiddleware.php b/src/Glpi/Api/HL/Middleware/IPRestrictionRequestMiddleware.php index f5400b5e0f2..17632b4f30c 100644 --- a/src/Glpi/Api/HL/Middleware/IPRestrictionRequestMiddleware.php +++ b/src/Glpi/Api/HL/Middleware/IPRestrictionRequestMiddleware.php @@ -53,13 +53,14 @@ public function process(MiddlewareInput $input, callable $next): void $request_ip = $_SERVER['REMOTE_ADDR']; - $allowed_ips = $DB->request([ + $result = $DB->request([ 'SELECT' => ['allowed_ips'], 'FROM' => 'glpi_oauthclients', 'WHERE' => [ 'identifier' => $client['client_id'] ] - ])->current()['allowed_ips']; + ])->current(); + $allowed_ips = $result['allowed_ips'] ?? []; if (empty($allowed_ips)) { // No IP restriction diff --git a/src/Glpi/Application/Environment.php b/src/Glpi/Application/Environment.php index 4ec3c25703c..9e459c6ccb7 100644 --- a/src/Glpi/Application/Environment.php +++ b/src/Glpi/Application/Environment.php @@ -178,7 +178,7 @@ public function shouldExpectRessourcesToChange(): bool // In others environments, we must match for changes. return match ($this) { default => false, - self::DEVELOPMENT => true, + self::DEVELOPMENT, self::TESTING => true, }; } diff --git a/tests/cypress/e2e/oauth_authcode.cy.js b/tests/cypress/e2e/oauth_authcode.cy.js new file mode 100644 index 00000000000..d4399c863a1 --- /dev/null +++ b/tests/cypress/e2e/oauth_authcode.cy.js @@ -0,0 +1,107 @@ +/** + * --------------------------------------------------------------------- + * + * GLPI - Gestionnaire Libre de Parc Informatique + * + * http://glpi-project.org + * + * @copyright 2015-2025 Teclib' and contributors. + * @licence https://www.gnu.org/licenses/gpl-3.0.html + * + * --------------------------------------------------------------------- + * + * LICENSE + * + * This file is part of GLPI. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + * --------------------------------------------------------------------- + */ + +describe('OAuth - Authorization Code Grant', () => { + const oauthclient_id = '9246d35072ff62193330003a8106d947fafe5ac036d11a51ebc7ca11b9bc135e'; + const oauthclient_secret = 'd2c4f3b8a0e1f7b5c6a9d1e4f3b8a0e1f7b5c6a9d1e4f3b8a0e1f7b5c6a9d1'; + + function doAuthCodeGrant(expect_already_logged_in = false, remember_me = false) { + function doGLPILogin() { + cy.findByRole('textbox', {'name': "Login"}).type('e2e_tests'); + cy.findByLabelText("Password").type('glpi'); + if (remember_me) { + cy.findByRole('checkbox', {name: "Remember me"}).check(); + } else { + cy.findByRole('checkbox', {name: "Remember me"}).uncheck(); + } + cy.getDropdownByLabelText("Login source").selectDropdownValue('GLPI internal database'); + cy.findByRole('button', {name: "Sign in"}).click(); + } + + function doAuthorization() { + // Should be on a page asking the user to approve or reject the authorization request + cy.findByRole('heading', {name: 'Test E2E OAuth Client wants to access your GLPI account'}).should('be.visible'); + cy.findByText('Access to the API').should('be.visible'); + cy.findByText('Access to the user\'s information').should('be.visible'); + cy.findByRole('button', {name: 'Deny'}).should('be.visible'); + // Clicking the Accept button would go to a 401 error page, because we didn't give a real redirect URL, which Cypress will have issues with because it isn't an HTML page + // We only care that the redirect URL includes the code parameter + cy.url().then((url) => { + cy.request({ + url: `${url}&accept=1`, + failOnStatusCode: false + }).then((response) => { + expect(response.status).to.eq(401); + expect(response.redirects[0]).to.include('code='); + // extract the code from the URL + const code = response.redirects[0].split('code=')[1]; + // Request the token now + cy.request({ + method: 'POST', + url: '/api.php/token', + body: { + grant_type: 'authorization_code', + client_id: oauthclient_id, + client_secret: oauthclient_secret, + code: code, + redirect_uri: '/api.php/oauth2/redirection' + }, + headers: { + 'Content-Type': 'application/json' + } + }).then((response) => { + expect(response.status).to.eq(200); + expect(response.body.access_token).to.exist; + expect(response.body.refresh_token).to.exist; + }); + }); + }); + } + + cy.visit(`/api.php/Authorize?response_type=code&client_id=${oauthclient_id}&scope=api user&redirect_uri=/api.php/oauth2/redirection`); + if (!expect_already_logged_in) { + doGLPILogin(); + } + doAuthorization(); + } + + it('Should authorize without cookie - no remember me', () => { + doAuthCodeGrant(); + }); + it('Should authorize without cookie - remember me', () => { + doAuthCodeGrant(false, true); + }); + it('Should authorize with cookie', () => { + cy.login(); + doAuthCodeGrant(true); + }); +}); diff --git a/tests/src/Command/TestUpdatedDataCommand.php b/tests/src/Command/TestUpdatedDataCommand.php index adc6295aeba..e07e536db5c 100644 --- a/tests/src/Command/TestUpdatedDataCommand.php +++ b/tests/src/Command/TestUpdatedDataCommand.php @@ -171,6 +171,11 @@ public function __construct($dbhost, $dbuser, $dbpassword, $dbdefault) continue; } + // Ignore e2e oauth client + if ($table_name === 'glpi_oauthclients' && $row_data['name'] === 'Test E2E OAuth Client') { + continue; + } + foreach ($row_data as $key => $value) { if (in_array($key, $excluded_fields)) { continue; // Ignore fields that would be subject to legitimate changes