Problem/Motivation

I haven't seen that the token expiration is saved anywhere, I think on top of forcing a refresh, the service should be smart enough to handle auto expiration.

Proposed resolution

Save the token expiration and calculate when to expire.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pcambra created an issue. See original summary.

pcambra’s picture

FWIW this is the code I have in my project for the token expiration

  public function token($forceLogin = FALSE) {
    // @TODO inject this.
    $state = \Drupal::state();
    if ($state->get('unhcr_marketing_cloud_requesting_token') == TRUE) {
      // Wait n seconds to prevent overloading token request with simultaneous
      // requests.
      sleep($this->config
        ->get('requestToken_wait'));
    }

    $token = $state->get('unhcr_marketing_cloud_token');
    $expired = \Drupal::time()->getCurrentTime() >= $state->get('unhcr_marketing_cloud_token_expire');
    $recalculate_token = $forceLogin || empty($token) || $expired;
    if (!$recalculate_token) {
      return $token;
    }

    // Prevent flooding by setting requesting_token to TRUE.
    $state->set('unhcr_marketing_cloud_requesting_token', TRUE);
    $token = FALSE;

    // Validate required params for token request.
    $token_requisites = TRUE;
    $clientId = $this->config->get('client_id');

    if (empty($clientId)) {
      \Drupal::logger(__METHOD__)->error('Bad config data: %missingData', ['%missingData' => 'client_id']);
      $token_requisites = FALSE;
    }
    $clientSecret = $this->config->get('client_secret');
    if (empty($clientSecret)) {
      \Drupal::logger(__METHOD__)->error('Bad config data: %missingData', ['%missingData' => 'client_secret']);
      $token_requisites = FALSE;
    }

    $scope = $this->config->get('scope');
    $accountId = $this->config->get('account_id');
    // Make token request/s.
    if ($token_requisites) {
      $loginAttempts = 0;
      $loginAttemptsMax = $this->config->get('login_attempts_max');
      while (!$token && $loginAttempts++ < $loginAttemptsMax) {
        $token = $this->requestToken($clientId, $clientSecret, $scope, $accountId);
      }
    }

    $state->set('unhcr_marketing_cloud_requesting_token', FALSE);
    $state->set('unhcr_marketing_cloud_token', $token->access_token);
    $state->set('unhcr_marketing_cloud_token_expire', \Drupal::time()->getCurrentTime() + $token->expires_in);

    return $token;
  }

And then on the token method, I return the whole thing:

      $response = \Drupal::httpClient()->post($url, [
        'verify' => FALSE,
        'headers' => ['Content-Type' => 'application/x-www-form-urlencoded'],
        'form_params' => $formParams,
      ]);
      return json_decode($response->getBody());

Needs DI as well, but this handles more or less this issue and #3195684: State API is a better fit to store the token, I can try to make a patch if you're interested

john_a’s picture

That's an interesting idea. I double checked on The MC site, and it's set to a fixed 20 minutes.

Name Type Description
access_token string Acts as a session ID that the application uses to make requests. Maximum length is 512 characters. Lifetime is 20 minutes.

Most builders would probably not care and would use the default TTL from MC. However, although there are no guidelines on specific times for TTL, it should be as short as possible.

Thanks for the code sample, I think we could use the same approach and set a default value on install for token TTL=20 minutes, to stay in line with MC. That way we get the maximum life (if the developer wants that), or they can set it to a lower value if they are security conscious and feel they do not need 20 minutes per session.

john_a’s picture

I didn't see your offer for a patch, and have already created one for #3195684: State API is a better fit to store the token, which includes changes to the functional tests and a hook update.

If you agree with my patch, I can treat that as a dependency, merge that in and then create the patch for this.

pcambra’s picture

Sounds good, even if the docs say 20 mins, my experience is that MC is a bit... stingy? and never gives you 20 minutes, which was causing edges case on my use case

john_a’s picture

Patch here. I've not implemented your change to use only access_token from the body in the request token. We had a previous ticket to fix the fetching of the token for v2, where MC for some strange reason changed the key - so this code needs to work for v1 & v2.

the patch also includes a hook update.

john_a’s picture

Updated patch - there is a bug in the tests updated form last time. Not sure why the tests passed last time...

john_a’s picture

Status: Active » Needs review

  • john_a authored d534365 on 3195685-handle-token-exipration-in-the-marketingcloud-class
    Issue #3195685 by john_a: Handle token expiration automatically in the...
john_a’s picture

Committing this to dev and marking as ready for release

john_a’s picture

Status: Needs review » Reviewed & tested by the community

  • john_a authored c903edd on 8.x-1.x
    Issue #3195685 by john_a: Handle token expiration automatically in the...
john_a’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.