Vonage / vonage-php-sdk-core

Vonage REST API client for PHP. API support for SMS, Voice, Text-to-Speech, Numbers, Verify (2FA) and more.
https://developer.vonage.com/
Apache License 2.0
916 stars 180 forks source link

Allow signed requests with Container #280

Closed boenrobot closed 1 year ago

boenrobot commented 3 years ago

Certain requests like sending an SMS could be performed either with a signed request or an unsigned ones, and others still can only be performed by an unsigned request.

The Container credential type can in theory hold all types, so that the client can use the best ones for the request, from the ones available, or otherwise error.

Expected Behavior

  1. If the request needs a keypair, use a keypair if available or error if not available.
  2. If the request supports a keypair, use a keypair if available or check for signed request if not available.
  3. If the request needs a signed secret, use a signed secret if available, or error if not available.
  4. If the request supports a signed secret, use a signed secret if available, or check for basic if not available.
  5. Otherwise, use basic credentials if available, or error if not available.

Current Behavior

According to this bit here: https://github.com/Vonage/vonage-php-sdk-core/blob/7cfa39414ffb0529ee05d0f4f22225f43cc72ef6/src/Client.php#L504-L511

  1. If the request needs a keypair, use a keypair if available or error if not available.
  2. Otherwise, use basic auth if available or error if not available.

Possible Solution

Implement the expected logic when a container is provided as the credential. I don't know the auth capabilities of all API calls, so the list may be incomplete at first if I was to make a PR.

Without this logic, a workaround is creating two distinct client objects (each with the proper credential), which could work fine, but is just inconvenient.

Context

The verification API seems to only work with basic credentials, but the SMS sending one may optionally use signed secrets. I'm considering using both APIs, but that would mean I can't have signed SMS messages, unless I have two different client objects - one only for verification, using basic auth.

dragonmantank commented 3 years ago

This is actually something we've identified, and will be fixing. In the meantime, you can configure two clients, one with basic auth and one with signature auth. There is no global state with our code, so you can run multiple copies of the client in the same request.

We don't have a cascade effect of authentication as each API generally only supports one auth type, but the core issue is with basic auth and signature auth not being resolved properly in that block. I'll leave this open until we fix it properly.

SecondeJK commented 2 years ago

Work will be underway soon to change how we handle auth in the SDK

SecondeJK commented 1 year ago

v4.0.0 was released last week, that enables you to use multiple credential types that are now handled by new AuthHandlers. If you use SignatureSecret, it will take preference over Basic or Keypair cred types.