Skip to content

Commit

Permalink
feature #1036 refs #955: deprecate Client::AUTH_* constants and repla…
Browse files Browse the repository at this point in the history
…ce them with AuthMethod::AUTH_* const (ipalo)

This PR was squashed before being merged into the 3.4.x-dev branch.

Discussion
----------

Contribution for #955

Commits
-------

11c2d9f refs #955: deprecate Client::AUTH_* constants and replace them with AuthMethod::AUTH_* const
f4774d0 refs #955: revert the Client::AUTH_* deletion (BC)
e7f1ab9 refs #955: fix CR issues
c9cf54e refs #955: add upgrade to v4.0 notes
ec6656c refs #955: set public all constants
  • Loading branch information
ipalo authored Nov 2, 2021
1 parent 0349838 commit de2f278
Show file tree
Hide file tree
Showing 10 changed files with 79 additions and 35 deletions.
6 changes: 6 additions & 0 deletions UPGRADE-4.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,9 @@
### ResultPager

* `\Github\ResultPagerInterface::postFetch` is deprecated, and the method will be removed from the ResultPager interface/class.

### Authentication methods

* `Github\Client::AUTH_CLIENT_ID` is deprecated, use `Github\AuthMethod::CLIENT_ID` instead.
* `Github\Client::AUTH_ACCESS_TOKEN` is deprecated, use `Github\AuthMethod::ACCESS_TOKEN` instead.
* `Github\Client::AUTH_JWT` is deprecated, use `Github\AuthMethod::JWT` instead.
6 changes: 3 additions & 3 deletions doc/currentuser/repositories.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ There are three values that can be passed into the `repositories` method: `type`
| sort | `full_name` | `created`, `updated`, `pushed`, `full_name`
| direction | `asc` | `asc`, `desc`

> See https://developer.github.com/v3/repos/#list-your-repositories for possible values and additional information
> See https://developer.github.com/v3/repos/#list-your-repositories for possible values and additional information
#### Code Example:

```php
$client = new \Github\Client();
$client->authenticate($github_token, null, \Github\Client::AUTH_ACCESS_TOKEN);
$client = new \Github\Client();
$client->authenticate($github_token, null, \Github\AuthMethod::ACCESS_TOKEN);
$client->currentUser()->repositories();
```
6 changes: 3 additions & 3 deletions doc/graphql.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ $rateLimits = $client->api('graphql')->execute($query);
To use [GitHub v4 API (GraphQL API)](http://developer.github.com/v4/) requests must [authenticated]((../security.md)).

```php
$client->authenticate($token, null, Github\Client::AUTH_ACCESS_TOKEN);
$client->authenticate($token, null, Github\AuthMethod::ACCESS_TOKEN);

$result = $client->api('graphql')->execute($query);
```
Expand All @@ -28,7 +28,7 @@ To use [GitHub v4 API (GraphQL API)](http://developer.github.com/v4/) with diffe
```php
$result = $client->api('graphql')->execute($query, [], 'application/vnd.github.starfox-preview+json')
```
> default accept header is `application/vnd.github.v4+json`
> default accept header is `application/vnd.github.v4+json`


Expand All @@ -51,7 +51,7 @@ $variables = [
'organizationLogin' => 'KnpLabs'
];

$client->authenticate('<your-token>', null, Github\Client::AUTH_ACCESS_TOKEN);
$client->authenticate('<your-token>', null, Github\AuthMethod::ACCESS_TOKEN);

$orgInfo = $client->api('graphql')->execute($query, $variables);
```
18 changes: 9 additions & 9 deletions doc/security.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,23 @@ $client->authenticate($usernameOrToken, $password, $method);
and guess what should contain `$password`. The `$method` can contain one of the three allowed values:

#### Supported methods
* `Github\Client::AUTH_CLIENT_ID` - https://developer.github.com/v3/#oauth2-keysecret
* `Github\Client::AUTH_ACCESS_TOKEN` - https://developer.github.com/v3/#oauth2-token-sent-in-a-header
* `Github\Client::AUTH_JWT` - https://developer.github.com/apps/building-github-apps/authenticating-with-github-apps/#authenticating-as-a-github-app
* `Github\AuthMethod::CLIENT_ID` - https://developer.github.com/v3/#oauth2-keysecret
* `Github\AuthMethod::ACCESS_TOKEN` - https://developer.github.com/v3/#oauth2-token-sent-in-a-header
* `Github\AuthMethod::JWT` - https://developer.github.com/apps/building-github-apps/authenticating-with-github-apps/#authenticating-as-a-github-app

The required value of `$password` depends on the chosen `$method`. For `Github\Client::AUTH_ACCESS_TOKEN`, `Github\Client::AUTH_ACCESS_TOKEN` and
`Github\Client::JWT` methods you should provide the API token in `$usernameOrToken` variable (`$password` is omitted in
The required value of `$password` depends on the chosen `$method`. For `Github\AuthMethod::CLIENT_ID`, `Github\AuthMethod::ACCESS_TOKEN` and
`Github\AuthMethod::JWT` methods you should provide the API token in `$usernameOrToken` variable (`$password` is omitted in
this particular case).

The `Github\Client::AUTH_JWT` authentication method sends the specified JSON Web Token in an Authorization header.
The `Github\AuthMethod::JWT` authentication method sends the specified JSON Web Token in an Authorization header.

After executing the `$client->authenticate($usernameOrToken, $secret, $method);` method using correct credentials, all
further requests are done as the given user.

### Authenticating as an Integration

To authenticate as an integration you need to supply a JSON Web Token with `Github\Client::AUTH_JWT` to request
and installation access token which is then usable with `Github\Client::AUTH_ACCESS_TOKEN`. [Github´s integration
To authenticate as an integration you need to supply a JSON Web Token with `Github\AuthMethod::JWT` to request
and installation access token which is then usable with `Github\AuthMethod::ACCESS_TOKEN`. [Github´s integration
authentication docs](https://developer.github.com/apps/building-github-apps/authentication-options-for-github-apps/#authenticating-as-a-github-app) describe the flow in detail.
It´s important for integration requests to use the custom Accept header `application/vnd.github.machine-man-preview`.

Expand Down Expand Up @@ -64,7 +64,7 @@ $jwt = $config->builder(ChainedFormatter::withUnixTimestampDates())
->getToken($config->signer(), $config->signingKey())
;

$github->authenticate($jwt->toString(), null, Github\Client::AUTH_JWT)
$github->authenticate($jwt->toString(), null, Github\AuthMethod::JWT)
```

The `$integrationId` you can find in the about section of your github app.
Expand Down
30 changes: 30 additions & 0 deletions lib/Github/AuthMethod.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php

namespace Github;

final class AuthMethod
{
/**
* Authenticate using a client_id/client_secret combination.
*
* @var string
*/
public const CLIENT_ID = 'client_id_header';

/**
* Authenticate using a GitHub access token.
*
* @var string
*/
public const ACCESS_TOKEN = 'access_token_header';

/**
* Constant for authentication method.
*
* Indicates JSON Web Token authentication required for GitHub apps access
* to the API.
*
* @var string
*/
public const JWT = 'jwt';
}
14 changes: 10 additions & 4 deletions lib/Github/Client.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,19 @@ class Client
* Authenticate using a client_id/client_secret combination.
*
* @var string
*
* @deprecated Use the AuthMethod const
*/
const AUTH_CLIENT_ID = 'client_id_header';
const AUTH_CLIENT_ID = AuthMethod::CLIENT_ID;

/**
* Authenticate using a GitHub access token.
*
* @var string
*
* @deprecated Use the AuthMethod const
*/
const AUTH_ACCESS_TOKEN = 'access_token_header';
const AUTH_ACCESS_TOKEN = AuthMethod::ACCESS_TOKEN;

/**
* Constant for authentication method.
Expand All @@ -90,8 +94,10 @@ class Client
* to the API.
*
* @var string
*
* @deprecated Use the AuthMethod const
*/
const AUTH_JWT = 'jwt';
const AUTH_JWT = AuthMethod::JWT;

/**
* @var string
Expand Down Expand Up @@ -313,7 +319,7 @@ public function api($name): AbstractApi
*/
public function authenticate($tokenOrLogin, $password = null, $authMethod = null): void
{
if (null === $authMethod && (self::AUTH_JWT === $password || self::AUTH_ACCESS_TOKEN === $password)) {
if (null === $authMethod && (AuthMethod::JWT === $password || AuthMethod::ACCESS_TOKEN === $password)) {
$authMethod = $password;
$password = null;
}
Expand Down
8 changes: 4 additions & 4 deletions lib/Github/HttpClient/Plugin/Authentication.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

namespace Github\HttpClient\Plugin;

use Github\Client;
use Github\AuthMethod;
use Github\Exception\RuntimeException;
use Http\Client\Common\Plugin;
use Http\Promise\Promise;
Expand Down Expand Up @@ -58,11 +58,11 @@ public function handleRequest(RequestInterface $request, callable $next, callabl
private function getAuthorizationHeader(): string
{
switch ($this->method) {
case Client::AUTH_CLIENT_ID:
case AuthMethod::CLIENT_ID:
return sprintf('Basic %s', base64_encode($this->tokenOrLogin.':'.$this->password));
case Client::AUTH_ACCESS_TOKEN:
case AuthMethod::ACCESS_TOKEN:
return sprintf('token %s', $this->tokenOrLogin);
case Client::AUTH_JWT:
case AuthMethod::JWT:
return sprintf('Bearer %s', $this->tokenOrLogin);
default:
throw new RuntimeException(sprintf('%s not yet implemented', $this->method));
Expand Down
11 changes: 6 additions & 5 deletions test/Github/Tests/ClientTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace Github\Tests;

use Github\Api;
use Github\AuthMethod;
use Github\Client;
use Github\Exception\BadMethodCallException;
use Github\Exception\InvalidArgumentException;
Expand Down Expand Up @@ -68,9 +69,9 @@ public function shouldAuthenticateUsingAllGivenParameters($login, $password, $me
public function getAuthenticationFullData()
{
return [
['token', null, Client::AUTH_ACCESS_TOKEN],
['client_id', 'client_secret', Client::AUTH_CLIENT_ID],
['token', null, Client::AUTH_JWT],
['token', null, AuthMethod::ACCESS_TOKEN],
['client_id', 'client_secret', AuthMethod::CLIENT_ID],
['token', null, AuthMethod::JWT],
];
}

Expand All @@ -84,7 +85,7 @@ public function shouldAuthenticateUsingGivenParameters()
->getMock();
$builder->expects($this->once())
->method('addPlugin')
->with($this->equalTo(new Authentication('token', null, Client::AUTH_ACCESS_TOKEN)));
->with($this->equalTo(new Authentication('token', null, AuthMethod::ACCESS_TOKEN)));

$builder->expects($this->once())
->method('removePlugin')
Expand All @@ -98,7 +99,7 @@ public function shouldAuthenticateUsingGivenParameters()
->method('getHttpClientBuilder')
->willReturn($builder);

$client->authenticate('token', Client::AUTH_ACCESS_TOKEN);
$client->authenticate('token', AuthMethod::ACCESS_TOKEN);
}

/**
Expand Down
7 changes: 4 additions & 3 deletions test/Github/Tests/Functional/CacheTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Github\Tests\Functional;

use Github\AuthMethod;
use Github\Client;
use GuzzleHttp\Psr7\Response;
use Symfony\Component\Cache\Adapter\ArrayAdapter;
Expand All @@ -25,7 +26,7 @@ public function shouldServeCachedResponse()
$github = Client::createWithHttpClient($mockClient);
$github->addCache(new ArrayAdapter(), ['default_ttl'=>600]);

$github->authenticate('fake_token_aaa', Client::AUTH_ACCESS_TOKEN);
$github->authenticate('fake_token_aaa', AuthMethod::ACCESS_TOKEN);
$userA = $github->currentUser()->show();
$this->assertEquals('nyholm', $userA['login']);

Expand All @@ -45,11 +46,11 @@ public function shouldVaryOnAuthorization()
$github = Client::createWithHttpClient($mockClient);
$github->addCache(new ArrayAdapter(), ['default_ttl'=>600]);

$github->authenticate('fake_token_aaa', Client::AUTH_ACCESS_TOKEN);
$github->authenticate('fake_token_aaa', AuthMethod::ACCESS_TOKEN);
$userA = $github->currentUser()->show();
$this->assertEquals('nyholm', $userA['login']);

$github->authenticate('fake_token_bbb', Client::AUTH_ACCESS_TOKEN);
$github->authenticate('fake_token_bbb', AuthMethod::ACCESS_TOKEN);
$userB = $github->currentUser()->show();
$this->assertEquals('octocat', $userB['login'], 'We must vary on the Authorization header.');
}
Expand Down
8 changes: 4 additions & 4 deletions test/Github/Tests/HttpClient/Plugin/AuthenticationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

namespace Github\Tests\HttpClient\Plugin;

use Github\Client;
use Github\AuthMethod;
use Github\HttpClient\Plugin\Authentication;
use GuzzleHttp\Psr7\Request;
use Http\Promise\FulfilledPromise;
Expand Down Expand Up @@ -41,9 +41,9 @@ public function testAuthenticationMethods($tokenOrLogin, $password, $method, $ex
public function getAuthenticationData()
{
return [
['access_token', null, Client::AUTH_ACCESS_TOKEN, 'token access_token'],
['client_id', 'client_secret', Client::AUTH_CLIENT_ID, sprintf('Basic %s', base64_encode('client_id'.':'.'client_secret'))],
['jwt_token', null, Client::AUTH_JWT, 'Bearer jwt_token'],
['access_token', null, AuthMethod::ACCESS_TOKEN, 'token access_token'],
['client_id', 'client_secret', AuthMethod::CLIENT_ID, sprintf('Basic %s', base64_encode('client_id'.':'.'client_secret'))],
['jwt_token', null, AuthMethod::JWT, 'Bearer jwt_token'],
];
}
}

0 comments on commit de2f278

Please sign in to comment.