Instagram PHP Library

Question

I've written a small PHP library to connect with an Instagram application. The repository is on GitHub Instagram-php. Here I am adding only main class code. I require a code review and thoughts on performance and security issues.

namespace Haridarshan\Instagram;

use Haridarshan\Instagram\Exceptions\InstagramException;
use InvalidArgumentException;
use stdClass;

class Instagram
{
    /** @var InstagramApp */
    protected $app;
    /** @var string */
    protected $callbackUrl;
    /** @var array */
    protected $defaultScopes = ['basic', 'public_content', 'follower_list', 'comments', 'relationships', 'likes'];
    /** @var array */
    protected $scopes = [];
    /**
     * Random string indicating the state to prevent spoofing
     *
     * @var string
     */
    protected $state;
    /** @var \GuzzleHttp\Client $client */
    protected $client;
    /** @var InstagramOAuth $oauthResponse */
    protected $oauthResponse;
    /**
     * Default Constructor
     * Instagram Configuration Data
     *
     * @param array $config
     *
     * @throws InstagramException|InvalidArgumentException
     */
    public function __construct(array $config = [])
    {
        if (!is_array($config)) {
            throw new InstagramException('Invalid Instagram Configuration data');
        }
        $this->app = new InstagramApp($config['ClientId'], $config['ClientSecret']);
        $this->setCallbackUrl($config['Callback']);
        $this->state = isset($config['State']) ? $config['State'] : substr(md5(rand()), 0, 7);
        $this->client = HelperFactory::getInstance()->client(Constants::API_HOST);
    }
    /**
     * Returns InstagramApp entity.
     *
     * @return InstagramApp
     */
    public function getApp()
    {
        return $this->app;
    }
    /**
     * Make URLs for user browser navigation
     *
     * @param array $parameters
     *
     * @throws InstagramException
     *
     * @return string
     */
    public function getLoginUrl(array $parameters)
    {
        if (!isset($parameters['scope'])) {
            throw new InstagramException('Missing or Invalid Scope permission used', 400);
        }
        if (count(array_diff($parameters['scope'], $this->defaultScopes)) !== 0) {
            throw new InstagramException('Missing or Invalid Scope permission used', 400);
        }
        $this->scopes = $parameters['scope'];
        $loginUrl = new LoginUrl(
            $this->getApp(),
            $this->getCallbackUrl(),
            $this->getState(),
            $this->scopes
        );
        return $loginUrl->loginUrl();
    }
    /**
     * Get the Oauth Access Token of a user from callback code
     *
     * @param string $code - Oauth2 Code returned with callback url after successfull login
     *
     * @return InstagramOAuth
     */
    public function oauth($code)
    {
        $options = [
            'grant_type' => 'authorization_code',
            'client_id' => $this->app->getId(),
            'client_secret' => $this->app->getSecret(),
            'redirect_uri' => $this->getCallbackUrl(),
            'code' => $code,
            'state' => $this->state,
        ];
        $response = HelperFactory::getInstance()->request($this->client, Constants::API_TOKEN, $options, 'POST');
        $this->oauthResponse = new InstagramOAuth(
            json_decode($response->getBody()->getContents())
        );
        return $this->oauthResponse;
    }
    /**
     * Setter: Callback Url
     *
     * @param string $url
     */
    public function setCallbackUrl($url)
    {
        $this->callbackUrl = $url;
    }
    /**
     * Getter: Callback Url
     *
     * @return string
     */
    public function getCallbackUrl()
    {
        return $this->callbackUrl;
    }
    /**
     * Get InstagramOAuth
     *
     * @return InstagramOAuth
     */
    public function getOAuth()
    {
        if ($this->oauthResponse instanceof InstagramOAuth) {
            return $this->oauthResponse;
        }
        $accessToken = new stdClass;
        $accessToken->access_token = null;
        $this->oauthResponse = new InstagramOAuth($accessToken);
        return $this->oauthResponse;
    }
    /**
     * Get Http Client
     *
     * @return Client
     */
    public function getHttpClient()
    {
        return $this->client;
    }
    /**
     * Set User Access Token
     *
     * @param string $token
     */
    public function setAccessToken($token)
    {
        if (!$this->oauthResponse instanceof InstagramOAuth) {
            $this->oauthResponse = new InstagramOAuth(json_decode(json_encode(['access_token' => $token])));
        }
    }
    /**
     * Get state value
     *
     * @return string|mixed
     */
    public function getState()
    {
        return $this->state;
    }
    /**
     * Get a string containing the version of the library.
     *
     * @return string
     */
    public function getLibraryVersion()
    {
        return Constants::VERSION;
    }
}

Answer 1

It is hard to understand exactly what this class is intended to be used for.

Overall, it seems like the class is doing a lot, including: - instantiating InstagramApp object - performing user/application authentication via oAuth - generating LoginUrl objects - instantiating Guzzle client - managing access tokens - exposing constants form other classes

As such, I think this class could certainly be refactored to better manage only some narrow set of responsibilities.

Some specific notes

• Consider passing InstagramApp project into constructor as dependency. There is probably no reason that this class needs to understand how to set up this dependency here as it is doing. You can type hint that parameter to make sure you have 100% certainty that you class has the appropriate dependency
• I don't understand why you have use statements for built-in PHP classes.
• Why would you have empty array as default for $config parameter on constructor when an empty array would actually not allow you to complete your constructor properly? Consider defining a InstagramConfig or similar class which you would pass into the constructor. Again, you can type hint this parameter so the constructor is 100% certain it is getting an appropriately set up configuration object.
• Similarly, can you pass in the Guzzle client in the as dependency.
• What is value of InstagramException class (which is not shown)? It seems as if you are using is as one might InvalidArgumentException. Does it really need it own exception type?
• I don't fully understand what getLoginUrl() is intending to do. Does this really need to be a public method? Does it really need to recalculated every time, or is the LoginUrl object one that can be create on object instantiation (i.e. this method called via constructor) and be set on the object?
• On oauth() method, you do no validation of parametric input.
• In setCallbackUrl() should htis also prompt regeneration of LoginUrl object if you decide that this is an object which should be stored in property.
• Why is there a method getLibraryVersion() to retrieve a value from a wholly separate class with its own static accessor? You are setting a getter for an item that is not even a property on this class.