discovery-tecnologia / dsc-mercado-livre

Biblioteca de integração com o Mercado Livre
Apache License 2.0
72 stars 29 forks source link

AccessToken abstrato #72

Closed rafa-acioly closed 3 years ago

rafa-acioly commented 3 years ago

Estou desenvolvendo uma integração e me deparei com um acoplamento de Storage e AccessToken, no sistema que estou desenvolvendo eu preciso separar o token dos usuários pois o sistema fara a integração de diversos usuários simultaneamente, porem a lib permite apenas o armazenamento de uma unica chave de token não permitindo que eu define qual a chave sera utilizada para armazenar.

https://github.com/discovery-tecnologia/dsc-mercado-livre/blob/bc6a90401d4f09f937233130f24ba679a424d751/src/AccessToken.php#L9

A classe AccessToken é instanciada diretamente na classe de AuthorizationService impossibilitando de que um adapter seja utilizado.

https://github.com/discovery-tecnologia/dsc-mercado-livre/blob/bc6a90401d4f09f937233130f24ba679a424d751/src/Resources/Authorization/AuthorizationService.php#L111

Existe alguma maneira de resolver essa questão ou é um ponto em que precisamos alterar a lib? O que eu preciso é armazenar N tokens onde a chave do token será um identificador unico para cada usuario.

rafa-acioly commented 3 years ago

Eu resolvi a situação criando alguns Adapters para as classes AuthorizationService, Production e StorageInterface, funciona mas acredito que deveria ser mais simples mudar a estrategia de salvar os dados de tokens 👀

dilowagner commented 3 years ago

Otima observacao @rafa-acioly Sim... realmente temos um problema ae!

Se vc quiser abrir uma PR pra gente analisar... Valeu pela ajuda!

dilowagner commented 3 years ago

Cara, eh um problemao hehe

Fiz uma analise aqui, acredito que conseguimos dois caminhos Primeiro: partir talvez para a criacao de um CustomEnvironment (nao sei ainda hehe) - acho que segue muito a linha que vc esta pensando mesmo, que eu acho legal tambem.

Segundo: talvez um pouco mais simples... seria, usar um terceiro parametro na class Meli Pensei ate em usar o clientId pra isso, mas salvar na chave de cache/session nao acho legal...

Acho que teriamos que alterar a MeliInterface

<?php
namespace Dsc\MercadoLivre;

interface MeliInterface
{
    // outros metodos

    /**
     * @return $tenant
     */
    public function getTenant();

    /**
     * @param string $name
     */
    public function setTenant($name);
}

Alterar a classe Meli para receber o tenant como parametro

/**
 * Class Meli
 */
class Meli implements MeliInterface
{
    // outros atributos

   /**
    * Tentant name
    */
    private $tenant;

    public function __construct($clientId, $clientSecret, Environment $environment = null, $tenant = null)
    {
        $this->clientId     = $clientId;
        $this->clientSecret = $clientSecret;
        $this->environment  = $environment ?: new Production();
        $this->tenant = $tenant;
    }

    public function setTenant($name) {
         $this->tenant = $name;
    }

    public function getTenant() {
         return $this->tenant;
    }

    // outros metodos
}

A AuthorizationService busca a referencia dessa classe sempre

$meli = $this->getMeli();

Ae temos que alterar a classe AccessToken para receber o $tenant e considerar que ele sempre considera esse name com a chave do cache.

<?php
namespace Dsc\MercadoLivre;

use Dsc\MercadoLivre\Storage\SessionManager;
use Dsc\MercadoLivre\Storage\StorageInterface;

class AccessToken
{
    const TOKEN = 'token';
    const REFRESH_TOKEN = 'refresh_token';
    const EXPIRE_IN = 'expire_in';

    /**
     * @var StorageInterface
     */
    private $storage;

    private $tenant;

    public function __construct(StorageInterface $storage = null, $tenant = null)
    {
        $this->storage = $storage ? $storage : new SessionManager();
        $this->tenant   = $tenant ?: static::TOKEN;
    }

    /**
     * @return string|bool
     */
    public function getToken()
    {
        return $this->storage->get(static::TOKEN.$this->tenant);
    }

    /**
     * @param string $token
     */
    public function setToken($token)
    {
        $this->storage->set(static::TOKEN.$this->tenant, $token);
    }

    /**
     * @return string
     */
    public function getRefreshToken()
    {
        return $this->storage->get(static::REFRESH_TOKEN.$this->tenant);
    }

    /**
     * @param string $refreshToken
     */
    public function setRefreshToken($refreshToken)
    {
        $this->storage->set(static::REFRESH_TOKEN.$this->tenant, $refreshToken);
    }

    /**
     * @return int
     */
    public function getExpireIn()
    {
        return $this->storage->get(static::EXPIRE_IN.$this->tenant);
    }

    /**
     * @param int $expireIn
     */
    public function setExpireIn($expireIn)
    {
        $this->storage->set(static::EXPIRE_IN.$this->tenant, time() + $expireIn);
    }

    /**
     * @return bool
     */
    public function isExpired()
    {
        if($this->storage->has(static::EXPIRE_IN.$this->tenant) &&
            $this->storage->get(static::EXPIRE_IN.$this->tenant) >= time()) {
            return false;
        }
        return true;
    }

    /**
     * @return bool
     */
    public function isValid()
    {
        if(! $this->getToken() || $this->isExpired()) {
            return false;
        }
        return true;
    }
}

E alterar no AuthorizationService para "inserir" no AccessToken

/**
     * @param array $data
     * @return string
     * @throws MeliException
     */
    private function getToken($data)
    {
        $environment = $this->getMeli()->getEnvironment(); 
        $tenant = $this->getMeli()->getTenant(); 
        // ......
        $accessToken = new AccessToken($storage, $tenant);
    }

// ......

Quando instanciamos um novo Meli passamos o tenant


        $meli = new Meli(
            'APP-ID', 
            'SECRET-ID',
            new Production(Site::BRASIL),
            'MINHA_LOJA_ID'
        );

O que vc acha?? @rafa-acioly

dilowagner commented 3 years ago

Pensando bem.... Acho que vou usar a classe Environment pra isso hehe

$meli = new Meli(
   'APP-ID', 
   'SECRET-ID',
   new Production(Site::BRASIL, 'MINHA_LOJA_ID')
);
rafa-acioly commented 3 years ago

Acredito que essa opção de inserir o MINHA_LOJA_ID em Environment seja a melhor, ao que parece as alterações seriam menores

dilowagner commented 3 years ago

Cara, mudei de ideia novamente hehe Acredito que nao teriamos problema setando o client-id como chave, ate porque esses dados ficam no server e criptografados.

Entao, nao muda nada na implementacao... usando a nova versao 2.1.0-rc esse suporte ja deve funcionar com as multi-contas.

rafa-acioly commented 3 years ago

@dilowagner mas o client-id é fixo certo? não teriamos como identificar qual é o tenant que esta fazendo a requisição para o mercado livre... não entendi bem a sugestão que você diz para setar o client-id como chave, o unico valor passado para storage é a palavra token, dessa maneira não conseguimos separar diversos tokens de diversos clientes.

dilowagner commented 3 years ago

Opa @rafa-acioly entao.... Acredito que nao, correto? O client-id vai mudar com cada cliente que conectar na sua plataforma/app.

Eu tenho um caso de uso parecido em um sistema que trabalhava... onde o cliente precisava configurar seus dados de acesso ao mercado livre (client-id e client-secret) Voce nao usa o client-id diferente pra cada cliente? como vc separaria as contas? todo mundo publica com o mesmo client-id?

Isso e o que indica o cliente unico la nas aplicacoes do Mercado Livre https://developers.mercadolivre.com.br/pt_br/registre-o-seu-aplicativo

dilowagner commented 3 years ago

Ha e nao comentei.... Exatamente, o que mudei foi para que a chave do storage considerasse o client-id tambem:

$key = static::TOKEN . $this->tenant
rafa-acioly commented 3 years ago

implementado em https://github.com/discovery-tecnologia/dsc-mercado-livre/commit/d175b7bc6342891e15ab9894c20df1652edf446b