TokenAuth: DRF BaseAuthentication return value

[aprams]

Hi,

the first look in the PR looked fine, I just stumbled across an error in the ShopifyTokenAuthentication(BaseAuthentication) class. I had it overwritten (and still have) for a bit of customization, so I did not realize at first until I merged in your latest changes.

BaseAuthentication's return value is expected to be a tuple of length two:

class BaseAuthentication:
    """
    All authentication classes should extend BaseAuthentication.
    """

    def authenticate(self, request):
        """
        Authenticate the request and return a two-tuple of (user, token).
        """

The current implementation returns "just" the user which leads to an error when trying to unpack that tuple.

class ShopifyTokenAuthentication(BaseAuthentication):
    keyword = "Bearer"

    @staticmethod
    def get_hostname(url):
        return urlparse(url).netloc

    def authenticate(self, request):
        UserModel = get_user_model()
        auth = get_authorization_header(request).split()
        if not auth or auth[0].lower() != self.keyword.lower().encode():
            return None
        if len(auth) == 1:
            msg = "Invalid token header. No credentials provided."
            raise AuthenticationFailed(msg)
        elif len(auth) > 2:
            msg = "Invalid token header. Token string should not contain spaces."
            raise AuthenticationFailed(msg)

        try:
            token = auth[1].decode()
        except UnicodeError:
            msg = "Invalid token header. Token string should not contain invalid characters."
            raise AuthenticationFailed(msg)

        try:
            decoded_payload = jwt.decode(
                token,
                settings.SHOPIFY_APP_API_SECRET,
                algorithms=["HS256"],
                audience=settings.SHOPIFY_APP_API_KEY,
                options={"verify_sub": False, "verify_nbf": False},
            )
            dest_host = self.get_hostname(decoded_payload["dest"])
            iss_host = self.get_hostname(decoded_payload["iss"])
            if dest_host != iss_host:
                raise AuthenticationFailed(INVALID_TOKEN_MESSAGE)

            try:
                return UserModel.objects.get(myshopify_domain=dest_host)
            except UserModel.DoesNotExist:
                raise AuthenticationFailed(INVALID_TOKEN_MESSAGE)

        except (ExpiredSignatureError, JWTError, JWTClaimsError) as e:
            logging.warning(f"Login user failed: {e}.")
            raise AuthenticationFailed(INVALID_TOKEN_MESSAGE)

My suggestion would be to change to:

            try:
                return (UserModel.objects.get(myshopify_domain=dest_host), None)
            except UserModel.DoesNotExist:
                raise AuthenticationFailed(INVALID_TOKEN_MESSAGE)

If you agree to these changes, I can create a PR

[stlk]

Ah yeah, that's the one thing I didn't test ;) Thank you. Let's return token instead of None if API dictates it.

return (UserModel.objects.get(myshopify_domain=dest_host), token)

Please submit a PR. I'll release it right away.

[aprams]

Sure, that makes sense :)

[stlk]

Released, thanks for the PR. :)