Add second api token for each user, with only safe api access

Fixes #1458

To migrate DB, run:

    bin/console doctrine:schema:update --force --complete

then to populate the new tokens in the DB:

    bin/console packagist:tokens:generate

css/app.scss

@@ -683,16 +683,15 @@ input:focus:invalid:focus, textarea:focus:invalid:focus, select:focus:invalid:fo
 
 .api-token-group {
  display: block;
- min-height: 45px;
+ margin-bottom: 5px;
 }
-.api-token-group #api-token {
+.api-token-group .api-token {
  width: 85%;
- position: absolute;
  right: 0;
  text-align: center;
 }
 .api-token-group .btn-show-api-token {
- width: 145px;
+ width: 185px;
  position: absolute;
  z-index: 10;
 }

js/app.js

@@ -38,21 +38,23 @@ import 'bootstrap';
  /**
  * API Token visibility toggling
  */
- var token = $('#api-token');
- token.val('');
+ $('.api-token').val();
 
- $('.btn-show-api-token,#api-token').each(function() {
+ $('.btn-show-api-token, .api-token').each(function() {
  $(this).click(function (e) {
+ const parent = $(this).closest('.api-token-group');
+ const token = parent.find('.api-token');
  token.val(token.data('api-token'));
  token.select();
 
- $('.btn-show-api-token').text('Your API token');
+ const button = parent.find('.btn-show-api-token').first();
+ button.text(button.text().replace('Show', 'Your'));
 
  e.preventDefault();
  });
  });
  $('.btn-rotate-api-token').click(function (e) {
- if (!window.confirm('Are you sure? This will revoke your current API token and generate a new one.')) {
+ if (!window.confirm('Are you sure? This will revoke your current API tokens and generate new ones.')) {
  e.preventDefault();
  }
  });

src/Command/GenerateTokensCommand.php

@@ -51,6 +51,16 @@ protected function execute(InputInterface $input, OutputInterface $output): int
  $user->initializeApiToken();
  }
  $this->doctrine->getManager()->flush();
+ $this->doctrine->getManager()->clear();
+
+ do {
+ $users = $userRepo->findUsersMissingSafeApiToken();
+ foreach ($users as $user) {
+ $user->initializeSafeApiToken();
+ }
+ $this->doctrine->getManager()->flush();
+ $this->doctrine->getManager()->clear();
+ } while (\count($users) > 0);
 
  return 0;
  }

src/Controller/ApiController.php

@@ -38,11 +38,23 @@
 use Symfony\Component\Validator\Validator\ValidatorInterface;
 use Symfony\Contracts\HttpClient\HttpClientInterface;
 
+enum ApiType {
+ case Safe;
+ case Unsafe;
+}
+
 /**
  * @author Jordi Boggiano <[email protected]>
  */
 class ApiController extends Controller
 {
+ private const REGEXES = [
+ 'gitlab' => '{^(?:ssh://git@|https?://|git://|git@)?(?P<host>[a-z0-9.-]+)(?::[0-9]+/|[:/])(?P<path>[\w.-]+(?:/[\w.-]+?)+)(?:\.git|/)?$}i',
+ 'any' => '{^(?:ssh://git@|https?://|git://|git@)?(?P<host>[a-z0-9.-]+)(?::[0-9]+/|[:/])(?P<path>[\w.-]+(?:/[\w.-]+?)*)(?:\.git|/)?$}i',
+ 'bitbucket_push' => '{^(?:https?://|git://|git@)?(?:api\.)?(?P<host>bitbucket\.org)[/:](?P<path>[\w.-]+/[\w.-]+?)(?:\.git)?/?$}i',
+ 'bitbucket_hook' => '{^(?:https?://|git://|git@)?(?P<host>bitbucket\.org)[/:](?P<path>[\w.-]+/[\w.-]+?)(?:\.git)?/?$}i',
+ ];
+
  public function __construct(
  private Scheduler $scheduler,
  private LoggerInterface $logger,
@@ -72,20 +84,27 @@ public function packagesAction(string $webDir): Response
  #[Route(path: '/api/create-package', name: 'generic_create', defaults: ['_format' => 'json'], methods: ['POST'])]
  public function createPackageAction(Request $request, ProviderManager $providerManager, GitHubUserMigrationWorker $githubUserMigrationWorker, RouterInterface $router, ValidatorInterface $validator): JsonResponse
  {
- $payload = json_decode($request->getContent(), true);
+ $payload = json_decode((string) $request->request->get('payload'), true);
+ if (!$payload && $request->headers->get('Content-Type') === 'application/json') {
+ $payload = json_decode($request->getContent(), true);
+ }
+
  if (!$payload || !is_array($payload)) {
  return new JsonResponse(['status' => 'error', 'message' => 'Missing payload parameter'], 406);
  }
- if (!isset($payload['repository']['url']) || !is_string($payload['repository']['url'])) {
- return new JsonResponse(['status' => 'error', 'message' => '{repository: {url: string}} expected in payload'], 406);
+ if (isset($payload['repository']['url']) && is_string($payload['repository']['url'])) { // supported for BC
+ $url = $payload['repository']['url'];
+ } elseif (isset($payload['repository']) && is_string($payload['repository'])) {
+ $url = $payload['repository'];
+ } else {
+ return new JsonResponse(['status' => 'error', 'message' => '{repository: string} expected in payload'], 406);
  }
 
  $user = $this->findUser($request);
  if (null === $user) {
- return new JsonResponse(['status' => 'error', 'message' => 'Missing username or apiToken in request'], 406);
+ return new JsonResponse(['status' => 'error', 'message' => 'Missing or invalid username/apiToken in request'], 406);
  }
 
- $url = $payload['repository']['url'];
  $package = new Package;
  $package->addMaintainer($user);
  $package->setRepository($url);
@@ -133,20 +152,23 @@ public function updatePackageAction(Request $request, string $githubWebhookSecre
  }
 
  if (isset($payload['project']['git_http_url'])) { // gitlab event payload
- $urlRegex = '{^(?:ssh://git@|https?://|git://|git@)?(?P<host>[a-z0-9.-]+)(?::[0-9]+/|[:/])(?P<path>[\w.-]+(?:/[\w.-]+?)+)(?:\.git|/)?$}i';
+ $urlRegex = self::REGEXES['gitlab'];
  $url = $payload['project']['git_http_url'];
  $remoteId = null;
- } elseif (isset($payload['repository']['url'])) { // github/anything hook
- $urlRegex = '{^(?:ssh://git@|https?://|git://|git@)?(?P<host>[a-z0-9.-]+)(?::[0-9]+/|[:/])(?P<path>[\w.-]+(?:/[\w.-]+?)*)(?:\.git|/)?$}i';
+ } elseif (isset($payload['repository']) && is_string($payload['repository'])) { // anything hook
+ $urlRegex = self::REGEXES['any'];
+ $url = $payload['repository'];
+ $remoteId = null;
+ } elseif (isset($payload['repository']['url']) && is_string($payload['repository']['url'])) { // github hook
+ $urlRegex = self::REGEXES['any'];
  $url = $payload['repository']['url'];
- $url = str_replace('https://api.github.com/repos', 'https://github.com', $url);
  $remoteId = isset($payload['repository']['id']) && (is_string($payload['repository']['id']) || is_int($payload['repository']['id'])) ? $payload['repository']['id'] : null;
  } elseif (isset($payload['repository']['links']['html']['href'])) { // bitbucket push event payload
- $urlRegex = '{^(?:https?://|git://|git@)?(?:api\.)?(?P<host>bitbucket\.org)[/:](?P<path>[\w.-]+/[\w.-]+?)(?:\.git)?/?$}i';
+ $urlRegex = self::REGEXES['bitbucket_push'];
  $url = $payload['repository']['links']['html']['href'];
  $remoteId = null;
  } elseif (isset($payload['canon_url']) && isset($payload['repository']['absolute_url'])) { // bitbucket post hook (deprecated)
- $urlRegex = '{^(?:https?://|git://|git@)?(?P<host>bitbucket\.org)[/:](?P<path>[\w.-]+/[\w.-]+?)(?:\.git)?/?$}i';
+ $urlRegex = self::REGEXES['bitbucket_hook'];
  $url = $payload['canon_url'].$payload['repository']['absolute_url'];
  $remoteId = null;
  } else {
@@ -155,14 +177,19 @@ public function updatePackageAction(Request $request, string $githubWebhookSecre
 
  $statsd->increment('update_pkg_api');
 
- return $this->receivePost($request, $url, $urlRegex, $remoteId, $githubWebhookSecret);
+ $url = str_replace('https://api.github.com/repos', 'https://github.com', $url);
+
+ return $this->receiveUpdateRequest($request, $url, $urlRegex, $remoteId, $githubWebhookSecret);
  }
 
  #[Route(path: '/api/packages/{package}', name: 'api_edit_package', requirements: ['package' => '[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+?'], defaults: ['_format' => 'json'], methods: ['PUT'])]
  public function editPackageAction(Request $request, Package $package, ValidatorInterface $validator, StatsDClient $statsd): JsonResponse
  {
  $user = $this->findUser($request);
- if ((!$user || !$package->getMaintainers()->contains($user)) && !$this->isGranted('ROLE_EDIT_PACKAGES')) {
+ if (!$user) {
+ return new JsonResponse(['status' => 'error', 'message' => 'Missing or invalid username/apiToken in request'], 406);
+ }
+ if (!$package->getMaintainers()->contains($user)) {
  throw new AccessDeniedException;
  }
 
@@ -173,6 +200,10 @@ public function editPackageAction(Request $request, Package $package, ValidatorI
  $payload = json_decode($request->getContent(), true);
  }
 
+ if (!isset($payload['repository']) || !is_string($payload['repository'])) {
+ return new JsonResponse(['status' => 'error', 'message' => '{repository: string} expected in request body'], 406);
+ }
+
  $package->setRepository($payload['repository']);
 
  $errors = $validator->validate($package, null, ["Update"]);
@@ -366,9 +397,9 @@ protected function getDefaultPackageAndVersionId(string $name): array|false
  * Perform the package update
  *
  * @param string $url the repository's URL (deducted from the request)
- * @param non-empty-string $urlRegex the regex used to split the user packages into domain and path
+ * @param value-of<self::REGEXES> $urlRegex the regex used to split the user packages into domain and path
  */
- protected function receivePost(Request $request, string $url, string $urlRegex, string|int|null $remoteId, string $githubWebhookSecret): JsonResponse
+ protected function receiveUpdateRequest(Request $request, string $url, string $urlRegex, string|int|null $remoteId, string $githubWebhookSecret): JsonResponse
  {
  // try to parse the URL first to avoid the DB lookup on malformed requests
  if (!Preg::isMatchStrictGroups($urlRegex, $url, $match)) {
@@ -406,7 +437,7 @@ protected function receivePost(Request $request, string $url, string $urlRegex,
 
  if (!$user) {
  // find the user
- $user = $this->findUser($request);
+ $user = $this->findUser($request, ApiType::Safe);
  }
 
  if (!$user && $match['host'] === 'github.com' && $request->getContent()) {
@@ -424,7 +455,7 @@ protected function receivePost(Request $request, string $url, string $urlRegex,
 
  if (!$packages) {
  if (!$user) {
- return new JsonResponse(['status' => 'error', 'message' => 'Invalid credentials'], 403);
+ return new JsonResponse(['status' => 'error', 'message' => 'Missing or invalid username/apiToken in request'], 403);
  }
 
  // try to find the user package
@@ -453,7 +484,7 @@ protected function receivePost(Request $request, string $url, string $urlRegex,
  /**
  * Find a user by his username and API token
  */
- protected function findUser(Request $request): ?User
+ protected function findUser(Request $request, ApiType $apiType = ApiType::Unsafe): ?User
  {
  $username = $request->request->has('username') ?
  $request->request->get('username') :
@@ -468,7 +499,14 @@ protected function findUser(Request $request): ?User
  }
 
  $user = $this->getEM()->getRepository(User::class)
- ->findOneBy(['usernameCanonical' => $username, 'apiToken' => $apiToken]);
+ ->createQueryBuilder('u')
+ ->where('u.usernameCanonical = :username')
+ ->andWhere($apiType === ApiType::Safe ? '(u.apiToken = :apiToken OR u.safeApiToken = :apiToken)' : 'u.apiToken = :apiToken')
+ ->setParameter('username', $username)
+ ->setParameter('apiToken', $apiToken)
+ ->setMaxResults(1)
+ ->getQuery()
+ ->getOneOrNullResult();
 
  if ($user && !$user->isEnabled()) {
  return null;
@@ -480,7 +518,7 @@ protected function findUser(Request $request): ?User
  /**
  * Find a user package given by its full URL
  *
- * @param non-empty-string $urlRegex
+ * @param value-of<self::REGEXES> $urlRegex
  * @return list<Package>
  */
  protected function findPackagesByUrl(User $user, string $url, string $urlRegex, string|int|null $remoteId): array

src/Controller/ProfileController.php

@@ -165,8 +165,9 @@ public function tokenRotateAction(Request $request, #[CurrentUser] User $user, U
  }
 
  $user->initializeApiToken();
- $userNotifier->notifyChange($user->getEmail(), 'Your API token has been rotated');
- $this->addFlash('success', 'Your API token has been rotated');
+ $user->initializeSafeApiToken();
+ $userNotifier->notifyChange($user->getEmail(), 'Your API tokens have been rotated');
+ $this->addFlash('success', 'Your API tokens have been rotated');
 
  $this->getEM()->persist($user);
  $this->getEM()->flush();

src/Controller/RegistrationController.php

@@ -54,8 +54,6 @@ public function register(Request $request, UserPasswordHasherInterface $password
  )
  );
 
- $user->initializeApiToken();
-
  $this->getEM()->persist($user);
  $this->getEM()->flush();
 

src/DataFixtures/UserFixtures.php

@@ -28,7 +28,6 @@ public function load(ObjectManager $manager): void
  $dev->setPassword($this->passwordHasher->hashPassword($dev, 'dev'));
  $dev->setEnabled(true);
  $dev->setRoles(['ROLE_SUPERADMIN']);
- $dev->initializeApiToken();
 
  $manager->persist($dev);
 
@@ -38,7 +37,6 @@ public function load(ObjectManager $manager): void
  $user->setPassword($this->passwordHasher->hashPassword($user, 'user'));
  $user->setEnabled(true);
  $user->setRoles([]);
- $user->initializeApiToken();
 
  $manager->persist($user);
  $manager->flush();

src/Entity/User.php

@@ -101,9 +101,12 @@ class User implements UserInterface, TwoFactorInterface, BackupCodeInterface, Eq
  #[ORM\Column(type: 'datetime')]
  private DateTimeInterface $createdAt;
 
- #[ORM\Column(type: 'string', length: 20)]
+ #[ORM\Column(type: 'string', length: 40)]
  private string $apiToken;
 
+ #[ORM\Column(type: 'string', length: 40)]
+ private string $safeApiToken;
+
  #[ORM\Column(type: 'string', length: 255, nullable: true)]
  private string|null $githubId = null;
 
@@ -129,6 +132,8 @@ public function __construct()
  {
  $this->packages = new ArrayCollection();
  $this->createdAt = new \DateTimeImmutable();
+ $this->initializeApiToken();
+ $this->initializeSafeApiToken();
  }
 
  public function __toString(): string
@@ -175,6 +180,16 @@ public function getApiToken(): string
  return $this->apiToken;
  }
 
+ public function setSafeApiToken(string $apiToken): void
+ {
+ $this->safeApiToken = $apiToken;
+ }
+
+ public function getSafeApiToken(): string
+ {
+ return $this->safeApiToken;
+ }
+
  public function getGithubId(): string|null
  {
  return $this->githubId;
@@ -521,7 +536,12 @@ public function isEqualTo(UserInterface $user): bool
 
  public function initializeApiToken(): void
  {
- $this->apiToken = bin2hex(random_bytes(10));
+ $this->apiToken = bin2hex(random_bytes(20));
+ }
+
+ public function initializeSafeApiToken(): void
+ {
+ $this->safeApiToken = bin2hex(random_bytes(20));
  }
 
  public function getConfirmationToken(): string|null

src/Entity/UserRepository.php

@@ -51,6 +51,18 @@ public function findUsersMissingApiToken(): array
  return $qb->getQuery()->getResult();
  }
 
+ /**
+ * @return list<User>
+ */
+ public function findUsersMissingSafeApiToken(): array
+ {
+ $qb = $this->createQueryBuilder('u')
+ ->where('u.safeApiToken IS NULL')
+ ->setMaxResults(500);
+
+ return $qb->getQuery()->getResult();
+ }
+
  public function getPackageMaintainersQueryBuilder(Package $package, ?User $excludeUser = null): QueryBuilder
  {
  $qb = $this->createQueryBuilder('u')

src/Security/GitHubAuthenticator.php

@@ -139,7 +139,6 @@ public function authenticate(Request $request): Passport
  );
 
  $user->setLastLogin(new \DateTimeImmutable());
- $user->initializeApiToken();
  $user->setEnabled(true);
 
  $this->getEM()->persist($user);