getAccessToken doesn't verifies the clientId and clientSecret in optional parameters.

[Arijeet996]

Package containing the bug

next-drupal (NPM package)

Describe the bug

The getAccessToken function currently doesn't verifies the optional authentication parameters leading to an error stating that 'clientId' and 'clientSecret' required., even when provided in the optional parameters.

Expected behavior

The getAccessToken function needs to verify the existence of clientId and clientSecret in both the default authentication and optional authentication parameters. If these credentials are not present in either of them, an error should be thrown indicating that both clientId and clientSecret are required.

Steps to reproduce:

1. Try fetching a resource which needs oauth authentication.
2. Don't pass the auth credentials as default params with DrupalClient
3. Pass the auth credentials as optional params.

Example:

const translatedPath = await drupal.translatePath(path, {
      withAuth:  {
          clientId: YOUR_CLIENT_ID,
          clientSecret: YOUR_CLIENT_SECRET,
          url: YOUR_URL
      } 
})

[JohnAlbin]

I'm copying my comment over from PR #658:

First: good catch! This is definitely buggy and needs fixing.

Looking at this closer, I can see that the underlying problem is that isBasicAuth() and isClientIdSecretAuth() are both implemented incorrectly. They are currently implemented to detect if some of the related auth properties are present in the auth object, but they don't guarantee that the auth is valid.

Looking at the code, it's clear that those functions are supposed to verify the auth object has valid credential properties:

function isClientIdSecretAuth(
  auth: DrupalClient["auth"]
): auth is DrupalClientAuthClientIdSecret {
  return (
    (auth as DrupalClientAuthClientIdSecret)?.clientId !== undefined ||
    (auth as DrupalClientAuthClientIdSecret)?.clientSecret !== undefined
  )
}

The name of the function is literally isClientIdSecretAuth implying that it is verifying that the auth is a valid pair of id/secret properties.

More importantly, you can see the return type of isClientIdSecretAuth() is auth is DrupalClientAuthClientIdSecret, a boolean indicated that the provided auth: DrupalClient["auth"] parameter is actually a DrupalClientAuthClientIdSecret. The function is designed to let TypeScript know that the parameter has been verified to be a particular Type.

The problem is that isClientIdSecretAuth({ clientId: 'some-id' }) and isClientIdSecretAuth({ clientSecret: {} }) both return true. BUT, in order for auth to be valid it must match this type definition:

export interface DrupalClientAuthClientIdSecret {
  clientId: string
  clientSecret: string
  url?: string
  scope?: string
}

And you can see that both clientId and clientSecret are required strings. And that's not what the body of the function checks. :(

When we fix isBasicAuth() and isClientIdSecretAuth() it will break set auth because its logic is dependent on the incorrect logic of the existing (buggy) implementation.

[JohnAlbin]

Fixed with #707