From 61e5f98312e8ee0fab7c1e897a7c3c196c66426a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patrick=20Bu=C3=9Fmann?= Date: Wed, 25 Aug 2021 21:47:57 +0200 Subject: [PATCH] Moved to GitHub Actions and increased test coverage --- .github/workflows/continuous-integration.yml | 80 ++++++++++++++++++++ .travis.yml | 67 ---------------- CHANGELOG.md | 11 +++ composer.json | 1 + src/Provider/Apple.php | 2 +- test/src/Provider/AppleTest.php | 59 ++++++++++++++- test/src/Token/AppleAccessTokenTest.php | 53 +++++++++---- 7 files changed, 191 insertions(+), 82 deletions(-) create mode 100644 .github/workflows/continuous-integration.yml delete mode 100644 .travis.yml diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml new file mode 100644 index 0000000..26b9de0 --- /dev/null +++ b/.github/workflows/continuous-integration.yml @@ -0,0 +1,80 @@ +name: 'CI' + +on: + pull_request: + push: + branches: + - 'main' + +env: + COMPOSER_ROOT_VERSION: '1.99.99' + +jobs: + lint: + name: 'Lint' + runs-on: 'ubuntu-latest' + steps: + - uses: 'actions/checkout@v2' + - uses: 'shivammathur/setup-php@v2' + with: + php-version: '7.4' + coverage: 'none' + ini-values: 'memory_limit=-1' + tools: 'composer:v2' + - uses: 'ramsey/composer-install@v1' + - name: 'Lint the PHP source code' + run: './vendor/bin/parallel-lint src test' + + coding-standards: + name: 'Coding Standards' + runs-on: 'ubuntu-latest' + steps: + - uses: 'actions/checkout@v2' + - uses: 'shivammathur/setup-php@v2' + with: + php-version: '7.4' + coverage: 'none' + ini-values: 'memory_limit=-1' + tools: 'composer:v2' + - uses: 'ramsey/composer-install@v1' + - name: 'Check coding standards' + run: './vendor/bin/phpcs src --standard=psr2 -sp --colors' + + unit-tests: + name: 'Unit Tests' + runs-on: 'ubuntu-latest' + continue-on-error: ${{ matrix.experimental }} + strategy: + fail-fast: false + matrix: + php-version: + - '5.6' + - '7.0' + - '7.1' + - '7.2' + - '7.3' + - '7.4' + - '8.0' + experimental: + - false + include: + - php-version: '8.1' + experimental: true + composer-options: '--ignore-platform-reqs' + steps: + - uses: 'actions/checkout@v2' + - uses: 'shivammathur/setup-php@v2' + with: + php-version: '${{ matrix.php-version }}' + coverage: 'pcov' + ini-values: 'memory_limit=-1' + tools: 'composer:v2' + - name: 'Prepare for tests' + run: 'mkdir -p build/logs' + - uses: 'ramsey/composer-install@v1' + with: + composer-options: '${{ matrix.composer-options }}' + - name: 'Run unit tests' + run: './vendor/bin/phpunit --colors=always --coverage-clover build/logs/clover.xml' + - name: 'Publish coverage report to Codecov' + uses: 'codecov/codecov-action@v1' diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index afe6e0a..0000000 --- a/.travis.yml +++ /dev/null @@ -1,67 +0,0 @@ -language: php - -matrix: - include: - - php: 5.6 - env: LCOBUCCI_JWT_VERSION=^3.4 - - php: 7.0 - env: LCOBUCCI_JWT_VERSION=^3.4 - - php: 7.1 - env: LCOBUCCI_JWT_VERSION=^3.4 - - php: 7.2 - env: LCOBUCCI_JWT_VERSION=^3.4 - - php: 7.3 - env: LCOBUCCI_JWT_VERSION=^3.4 - - php: 7.4 - env: LCOBUCCI_JWT_VERSION=^3.4 - - php: 7.4 - env: LCOBUCCI_JWT_VERSION=^4.0 - - php: 8.0 - env: LCOBUCCI_JWT_VERSION=^4.0 - - php: nightly - env: LCOBUCCI_JWT_VERSION=^4.0 - - php: hhvm-3.6 - sudo: required - dist: trusty - group: edge - - php: hhvm-3.9 - sudo: required - dist: trusty - group: edge - - php: hhvm-3.12 - sudo: required - dist: trusty - group: edge - - php: hhvm-3.15 - sudo: required - dist: trusty - group: edge - - php: hhvm-nightly - sudo: required - dist: trusty - group: edge - fast_finish: true - allow_failures: - - php: nightly - - php: hhvm-3.6 - - php: hhvm-3.9 - - php: hhvm-3.12 - - php: hhvm-3.15 - - php: hhvm-nightly - -before_script: - - travis_retry composer self-update - - travis_retry composer require lcobucci/jwt $LCOBUCCI_JWT_VERSION - - travis_retry composer install --no-interaction --prefer-source --dev - - travis_retry phpenv rehash - -script: - - ./vendor/bin/phpcs --standard=psr2 src/ - - ./vendor/bin/phpunit --coverage-text --coverage-clover=coverage.clover - -after_script: - - if [ "$TRAVIS_PHP_VERSION" == "7.4" ]; then wget https://scrutinizer-ci.com/ocular.phar; fi - - if [ "$TRAVIS_PHP_VERSION" == "7.4" ]; then php ocular.phar code-coverage:upload --format=php-clover coverage.clover; fi - -after_success: - - bash <(curl -s https://codecov.io/bash) diff --git a/CHANGELOG.md b/CHANGELOG.md index f83c6fc..45515f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,17 @@ All Notable changes to `oauth2-apple` will be documented in this file ### Security - Nothing +## 0.2.6 - 2021-08-25 + +### Added +- GitHub Actions CI + +### Removed +- Travis CI + +### Fixed +- Fixed bug with serialization of AppleAccessToken [#29](https://github.com/patrickbussmann/oauth2-apple/pull/29) (thanks to [tjveldhuizen](https://github.com/tjveldhuizen)) + ## 0.2.5 - 2021-03-10 ### Fixed diff --git a/composer.json b/composer.json index b825476..732841c 100644 --- a/composer.json +++ b/composer.json @@ -27,6 +27,7 @@ "require-dev": { "phpunit/phpunit": "^5.7 || ^6.0 || ^9.3", "mockery/mockery": "^1.3", + "php-parallel-lint/php-parallel-lint": "^1.3", "squizlabs/php_codesniffer": "^2.3 || ^3.0", "ext-json": "*" }, diff --git a/src/Provider/Apple.php b/src/Provider/Apple.php index 5fef56b..be5529a 100644 --- a/src/Provider/Apple.php +++ b/src/Provider/Apple.php @@ -88,7 +88,7 @@ private function getAppleKeys() { $response = $this->httpClient->request('GET', 'https://appleid.apple.com/auth/keys'); - if ($response) { + if ($response && $response->getStatusCode() === 200) { return JWK::parseKeySet(json_decode($response->getBody()->getContents(), true)); } diff --git a/test/src/Provider/AppleTest.php b/test/src/Provider/AppleTest.php index 1984e8b..f49fe90 100644 --- a/test/src/Provider/AppleTest.php +++ b/test/src/Provider/AppleTest.php @@ -9,6 +9,7 @@ use League\OAuth2\Client\Provider\Apple; use League\OAuth2\Client\Provider\AppleResourceOwner; use League\OAuth2\Client\Token\AccessToken; +use League\OAuth2\Client\Token\AppleAccessToken; use League\OAuth2\Client\Tool\QueryBuilderTrait; use PHPUnit\Framework\TestCase; use Mockery as m; @@ -164,6 +165,40 @@ public function testGetAccessToken() ]); } + public function testGetAccessTokenFailedBecauseAppleHasError() + { + $this->expectException('Exception'); + $this->expectExceptionMessage('Got no data within "id_token"!'); + + $provider = new TestApple([ + 'clientId' => 'mock.example', + 'teamId' => 'mock.team.id', + 'keyFileId' => 'mock.file.id', + 'keyFilePath' => __DIR__ . '/../../resources/p256-private-key.p8', + 'redirectUri' => 'none' + ]); + $provider = m::mock($provider); + + $client = m::mock(ClientInterface::class); + $client->shouldReceive('request') + ->times(1) + ->andReturn(new Response(500, [], 'Internal Server Error')); + $client->shouldReceive('send') + ->times(1) + ->andReturn(new Response(200, [], json_encode([ + 'access_token' => 'aad897dee58fe4f66bf220c181adaf82b.0.mrwxq.hmiE0djj1vJqoNisKmF-pA', + 'token_type' => 'Bearer', + 'expires_in' => 3600, + 'refresh_token' => 'r4a6e8b9c50104b78bc86b0d2649353fa.0.mrwxq.54joUj40j0cpuMANRtRjfg', + 'id_token' => 'abc' + ]))); + $provider->setHttpClient($client); + + $provider->getAccessToken('authorization_code', [ + 'code' => 'hello-world' + ]); + } + public function testFetchingOwnerDetails() { $provider = $this->getProvider(); @@ -206,6 +241,7 @@ public function testNotImplementedGetResourceOwnerDetailsUrl() public function testCheckResponse() { $this->expectException('\League\OAuth2\Client\Provider\Exception\AppleAccessDeniedException'); + $this->expectExceptionMessage('invalid_client'); $provider = $this->getProvider(); $class = new \ReflectionClass($provider); $method = $class->getMethod('checkResponse'); @@ -217,7 +253,7 @@ public function testCheckResponse() ]]); } - public function testCreationOfResourceOwner() + public function testCreationOfResourceOwnerWithName() { $provider = $this->getProvider(); $class = new \ReflectionClass($provider); @@ -247,6 +283,27 @@ public function testCreationOfResourceOwner() $this->assertArrayHasKey('name', $data->toArray()); } + public function testCreationOfResourceOwnerWithoutName() + { + $provider = $this->getProvider(); + $class = new \ReflectionClass($provider); + $method = $class->getMethod('createResourceOwner'); + $method->setAccessible(true); + + /** @var AppleResourceOwner $data */ + $data = $method->invokeArgs($provider, [ + [], + new AccessToken([ + 'access_token' => 'hello', + 'email' => 'john@doe.de', + 'resource_owner_id' => '123.4.567' + ]) + ]); + $this->assertEquals('john@doe.de', $data->getEmail()); + $this->assertNull($data->getLastName()); + $this->assertNull($data->getFirstName()); + } + public function testGetConfiguration() { $provider = m::mock(Apple::class)->makePartial(); diff --git a/test/src/Token/AppleAccessTokenTest.php b/test/src/Token/AppleAccessTokenTest.php index 8229d86..4e2f1ad 100644 --- a/test/src/Token/AppleAccessTokenTest.php +++ b/test/src/Token/AppleAccessTokenTest.php @@ -2,7 +2,6 @@ namespace League\OAuth2\Client\Test\Token; -use GuzzleHttp\Psr7\Response; use League\OAuth2\Client\Token\AppleAccessToken; use PHPUnit\Framework\TestCase; use Mockery as m; @@ -20,7 +19,10 @@ public function testCreatingAccessToken() ->with('something', 'examplekey', ['RS256']) ->once() ->andReturn([ - 'sub' => '123.abc.123' + 'sub' => '123.abc.123', + 'email_verified' => true, + 'email' => 'john@doe.com', + 'is_private_email' => true ]); $accessToken = new AppleAccessToken(['examplekey'], [ @@ -33,6 +35,21 @@ public function testCreatingAccessToken() $this->assertEquals('something', $accessToken->getIdToken()); $this->assertEquals('123.abc.123', $accessToken->getResourceOwnerId()); $this->assertEquals('access_token', $accessToken->getToken()); + $this->assertEquals('john@doe.com', $accessToken->getEmail()); + $this->assertTrue($accessToken->isPrivateEmail()); + } + + public function testCreateFailsBecauseNoIdTokenIsSet() + { + $this->expectException('\InvalidArgumentException'); + $this->expectExceptionMessage('Required option not passed: "id_token"'); + + new AppleAccessToken(['examplekey'], [ + 'access_token' => 'access_token', + 'token_type' => 'Bearer', + 'expires_in' => 3600, + 'refresh_token' => 'abc.0.def' + ]); } public function testCreatingRefreshToken() @@ -45,17 +62,27 @@ public function testCreatingRefreshToken() $this->assertEquals('access_token', $refreshToken->getToken()); } - private function getClient($times) + /** + * @runInSeparateProcess + * @preserveGlobalState disabled + */ + public function testCreatingAccessTokenFailsBecauseNoDecodingIsPossible() { - $client = m::mock('GuzzleHttp\ClientInterface'); - if ($times > 0) { - $client->shouldReceive('request') - ->times($times) - ->withArgs(['GET', 'https://appleid.apple.com/auth/keys']) - ->andReturn(new Response()) - ; - } - - return $client; + $this->expectException('\Exception'); + $this->expectExceptionMessage('Got no data within "id_token"!'); + + $externalJWTMock = m::mock('overload:Firebase\JWT\JWT'); + $externalJWTMock->shouldReceive('decode') + ->with('something', 'examplekey', ['RS256']) + ->once() + ->andReturnNull(); + + new AppleAccessToken(['examplekey'], [ + 'access_token' => 'access_token', + 'token_type' => 'Bearer', + 'expires_in' => 3600, + 'refresh_token' => 'abc.0.def', + 'id_token' => 'something' + ]); } }