Skip to content
This repository has been archived by the owner on Jan 16, 2023. It is now read-only.

Commit

Permalink
Fail gracefully if no user auth is available for GH
Browse files Browse the repository at this point in the history
The way we get project metadata from GitHub requires an authenticated
user. We get the required token if a user adds a project to LibreCores
through some, but not all mechanisms. Fail more gracefully if we don't
have the required information available.

This, however, doesn't yet give us the ability to get project metadata
without a user access token. This work is tracked in #446.
  • Loading branch information
imphil committed Apr 28, 2020
1 parent 15a8e09 commit d52f89a
Show file tree
Hide file tree
Showing 2 changed files with 101 additions and 50 deletions.
19 changes: 15 additions & 4 deletions site/src/Consumer/UpdateGitHubMetadataConsumer.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,18 @@ protected function processProject(Project $project) : bool
{
try {
$this->logger->info("Enriching project: {$project->getFqname()} with GitHub metadata");
$client = $this->getGithubClient($project);

// GraphQL (GitHub APIv4) queries must use an authenticated client,
// we cannot authenticate as application.
// TODO: Implement fallback with GitHub v3 REST API.
// https://github.com/librecores/librecores-web/issues/446
$client = $this->getAuthenticatedGithubClient($project);
if ($client === null) {
$this->logger->warning("Unable to get authenticated GitHub client. Unable to ".
"update GitHub meta data for {$project->getFqname()}.");
return true;

}

$repoUrl = $project->getSourceRepo()->getUrl();

Expand Down Expand Up @@ -179,10 +190,10 @@ private function fetchGithubMetadata(
*
* @param Project $project
*
* @return Client
*
* @return Client|null An authenticated Client, or null if authentication
* failed.
*/
private function getGithubClient(Project $project): Client
private function getAuthenticatedGithubClient(Project $project): ?Client
{

$user = $project->getParentUser();
Expand Down
132 changes: 86 additions & 46 deletions site/src/Service/GitHub/GitHubApiService.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,17 @@
use Github\Client;
use Symfony\Component\Cache\Adapter\AdapterInterface;
use Symfony\Component\Routing\RouterInterface;
use Psr\Log\LoggerInterface;

/**
* Wrap the KnpLabs/php-github-api GitHub API as Symfony service
*
* This service wrapper gives access to the GitHub API client as documented
* at https://github.com/KnpLabs/php-github-api/tree/master/doc. Instead of
* constructing the client manually, the getClient() and
* getAuthenticatedClient() methods of this class initialize the object for our
* use case, including authenticating with a user's GitHub OAuth access token.
* constructing the client manually, the getClientForUser() and
* getAuthenticatedClientForUser() methods of this class initialize the object
* for our use case, including authenticating with a user's GitHub OAuth access
* token.
*
* All requests are cached if a cache pool is given.
*/
Expand All @@ -41,18 +43,9 @@ class GitHubApiService
protected $router;

/**
* GitHub client API wrapper
*
* @var Client
* @var LoggerInterface
*/
private $client = null;

/**
* Is $client authenticated?
*
* @var bool
*/
private $clientIsAuthenticated = false;
protected $logger;


/**
Expand All @@ -67,13 +60,16 @@ class GitHubApiService
*
* @param AdapterInterface $cachePool the cache pool to use for all requests
* @param RouterInterface $router
* @param LoggerInterface $logger
*/
public function __construct(
AdapterInterface $cachePool,
RouterInterface $router
RouterInterface $router,
LoggerInterface $logger
) {
$this->cachePool = $cachePool;
$this->router = $router;
$this->logger = $logger;
}

/**
Expand Down Expand Up @@ -106,26 +102,6 @@ public static function parseGitHubRepoUrl(string $url): array
}
}

/**
* Get a GitHub API client object
*
* If a valid user is available, an authenticated client object is returned.
* See getAuthenticatedClient() for details what this means. Otherwise
* an unauthenticated client is returned, allowing the application to make
* anonymous API calls.
*
* @return Client a GitHub client
*/
public function getClient()
{
if (!$this->client) {
$this->client = $this->createClient();
$this->client->addCache($this->cachePool);
}

return $this->client;
}

/**
* Populate a Project with data obtained from the GitHub API
*
Expand Down Expand Up @@ -161,6 +137,36 @@ public function populateProject(
$project->setSourceRepo($sourceRepo);
}

/**
* Get a GitHub API client object
*
* If a valid user is available, an authenticated client object is returned.
* See getAuthenticatedClientForUser() for details what this means.
* Otherwise an unauthenticated client is returned, allowing the application
* to make anonymous API calls.
*
* @return Client a GitHub client
*/
public function getClientForUser(User $user) : Client
{
$client = $this->createClient();
$client->addCache($this->cachePool);

// Try to authenticate as user, but ignore failures.
if (!$this->authenticateClientAsUser($user, $client)) {
$this->logger->debug("Unable to authenticate to GitHub API as user.");

if (!$this->authenticateClientAsApplication($client)) {
$this->logger->error("Unable to authenticate to GitHub API as application.");
// Failure isn't fatal, but only decreases our rate limit. But
// failing still indicates something is wrong with the OAuth
// tokens, which should be diagnosed and fixed.
}
}

return $client;
}

/**
* Get an authenticated GitHub API client for a user
*
Expand All @@ -175,20 +181,13 @@ public function populateProject(
*
* @return Client|NULL an authenticated GitHub client, or NULL
*/
public function getAuthenticatedClientForUser(User $user)
public function getAuthenticatedClientForUser(User $user) : ?Client
{
$this->logger->debug("Trying to get authenticated GitHub client for ".
"user \"{$user->getUsername()}\".");
$client = $this->createClient();
$client->addCache($this->cachePool);

// try to authenticate as user with its access token
if (null !== $user && $user->isConnectedToOAuthService('github')) {
$oauthAccessToken = $user->getGithubOAuthAccessToken();
$client->authenticate(
$oauthAccessToken,
null,
Client::AUTH_URL_TOKEN
);

if ($this->authenticateClientAsUser($user, $client)) {
return $client;
}

Expand Down Expand Up @@ -233,4 +232,45 @@ private function createClient(): Client
{
return new GithubClient();
}

/**
* Try to authenticate as user with its access token
*
* @return bool success?
*/
private function authenticateClientAsUser(User $user, Client $client) : bool
{
if ($user === null || !$user->isConnectedToOAuthService('github')) {
return false;
}

$oauthAccessToken = $user->getGithubOAuthAccessToken();
$client->authenticate(
$oauthAccessToken,
null,
Client::AUTH_HTTP_TOKEN
);

return true;
}

/**
* Authenticate to GitHub as application
*
* This authentication doesn't make the GitHub client "authenticated", but
* increases the rate limit.
*
* See https://developer.github.com/v3/#increasing-the-unauthenticated-rate-limit-for-oauth-applications
* for details.
*
* @param Client $client the client to authenticate
* @return bool successful?
*/
private function authenticateClientAsApplication(Client $client) : bool
{
// TODO: Implement this. Use github_site_id/github_site_secret from
// configuration.
// https://github.com/librecores/librecores-web/issues/447
return true;
}
}

0 comments on commit d52f89a

Please sign in to comment.