imbo / behat-api-extension

API extension for Behat, used to ease testing of JSON-based APIs
MIT License
109 stars 42 forks source link

Support JWT auth for request #63

Closed pwolanin closed 3 years ago

pwolanin commented 7 years ago

Adds extension config and step definitions for defining JWT auth for a request.

I'm getting this working with Drupal with jsonapi and jwt modules. The scenario looks partly like:

  Scenario: Ensure as admin, I can create a container node using jsonapi.
    Given I am authenticating with JWT
    And the JWT will have claim "drupal[uid]" with value 1
    And the JWT will have claim "exp" with value "now + 10 minutes"
    And the "Accept" request header is "application/vnd.api+json"
    And the "Content-type" request header is "application/vnd.api+json"
    Given the request body is:
      """
{
  "data": {
    "type": "node--container",
      "attributes": {
        "title": "hello"
      },
      "relationships": {
        "og_audience": {
          "data": {
            "type": "node--group",
            "id": "70faaf2a-e06c-4caf-b930-098d10f64786"
          }
        }
      }
  }
}
      """

    When I request "/jsonapi/node/container" using HTTP "POST"
    Then the response code is 201
Zwartpet commented 7 years ago

Why not just pre-generating the JWT token (optionally without an expire so that tests will keep working) and setting it to the authorization header:

Background:

    Given the "Authorization" request header is "JWT TOKEN"
pwolanin commented 7 years ago

@Zwartpet I want to be able to easily write tests that use different JWT claims or even expiration times, etc.

Zwartpet commented 7 years ago

That's all possible with pre-generated JWT tokens. Creating the JWT's in the feature file will make it less easy to read in my opinion. Generating the JWT in a tool like https://jwt.io, and optionally commenting the claims if you really want that information directly readable in the feature file, would keep the test readable. Which is the purpose of a gherkin test.

pwolanin commented 7 years ago

@Zwartpet it's not possible, for example, to verify that a token with an exp time less than 1 minute in the past is still accepted (per rfc7519). I need to generate any time base claims dynamically.

In addition, I will want to make claims tied to a user ID or user name that may be generated during the test.

Zwartpet commented 7 years ago

Ah, you want to test the leeway. That indeed cannot be done with pre-generated tokens

pwolanin commented 6 years ago

I realize this PR is still roughly a PoC and doesn't have test coverage for the new feature, but hoping for feedback from the maintainers on the approach.

For example, I added a new method to the ApiClientAwareContext interface, but perhaps that's not the preferred approach.

christeredvartsen commented 6 years ago

@pwolanin Can you grant me access to your fork so I can do a rebase on develop?

pwolanin commented 6 years ago

I just rebased and force pushed, but I will add you also.

-Peter

On Thu, Jan 11, 2018 at 2:00 AM, Christer Edvartsen < notifications@github.com> wrote:

@pwolanin https://github.com/pwolanin Can you grant me access to your fork so I can do a rebase on develop?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/imbo/behat-api-extension/pull/63#issuecomment-356844130, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGkq01JOvQkHCLwN-MNLD7WrQ0qBpAsks5tJbGDgaJpZM4QoMYc .

christeredvartsen commented 4 years ago

First off: I'm very sorry that I haven't tended to this PR before.

If you are still interested in having this included in the project, could you please rebase on develop and push the updated branch?

pwolanin commented 4 years ago

Just saw this, will work on rebasing and solving conflicts. Looks like there are also a couple methods to make public that would be helpful in terms of custom steps.

christeredvartsen commented 4 years ago

Looks like there are also a couple methods to make public that would be helpful in terms of custom steps.

I'm not too keen on making methods public. Could you share some info as to why you need this? An example with some code would be great.

pwolanin commented 4 years ago

The 2 methods are:

\Imbo\BehatApiExtension\Context\ApiContext::requestPath \Imbo\BehatApiExtension\Context\ApiContext::sendRequest

I needed to generate a request path in a custom step based on a just-uploaded file. The back-end sets a file directory that is time based, so there is no way to set a fixed path in the step. Here's the code (Drupal api)

/**
 * Defines application features from the specific context.
 */
class FeatureContext extends RawDrupalContext implements SnippetAcceptingContext {

  /**
   * The API context.
   *
   * @var \Imbo\BehatApiExtension\Context\ApiContext
   */
  protected $apiContext;

  /**
   * Pre-scenario hook.
   *
   * @BeforeScenario
   */
  public function gatherContexts(BeforeScenarioScope $scope) {
    $environment = $scope->getEnvironment();

    $this->apiContext = $environment->getContext('Imbo\BehatApiExtension\Context\ApiContext');
  }

  /**
   * Request a file attachment.
   *
   * @param string $filename To request
   *
   * @When I request private file :filename
   */
  public function requestPrivateFile($filename) {
    $storage = \Drupal::entityTypeManager()->getStorage('file');
    $files = $storage->loadByProperties(['filename' => $filename]);
    if (!$files) {
      throw new ExpectationException(sprintf('No file found by name: %s', $filename));
    }
    if (count($files) > 1) {
      throw new ExpectationException(sprintf('Found multiple files by name: %s', $filename));
    }
    $file = end($files);
    if (strpos($file->uri->value, 'private://') !== 0) {
      throw new ExpectationException(sprintf('File URI is not private: %s', $file->uri->value));
    }
    $this->apiContext->setRequestPath($file->uri->url);

    $this->apiContext->sendRequest();
  }
}
christeredvartsen commented 4 years ago

The two last lines in your method:

$this->apiContext->setRequestPath($file->uri->url);
$this->apiContext->sendRequest();

As far as I can tell they can be swapped with a call to the (already public) requestPath method:

$this->apiContext->requestPath($file->uri->url);

Here is the complete requestPath method from the ApiContext class:

public function requestPath($path, $method = null) {
    $this->setRequestPath($path);

    if (null === $method) {
        $this->setRequestMethod('GET', false);
    } else {
        $this->setRequestMethod($method);
    }

    return $this->sendRequest();
}

I might be missing something though, but won't that work?

pwolanin commented 4 years ago

Oh, maybe I missed the easy solution! Will try that instead.

pwolanin commented 4 years ago

Yes, that seems to work, thanks. Will work on a re-roll of the other parts based on the current branch

christeredvartsen commented 4 years ago

Yes, that seems to work, thanks. Will work on a re-roll of the other parts based on the current branch

Perfect!

pwolanin commented 4 years ago

Had to add a phpstan ignore rule, seems to be the same problem reported here: https://github.com/phpstan/phpstan/issues/844

pwolanin commented 4 years ago

Had a duplicate method in test code causing that fail. Started adding a JWT auth middleware like the basic auth one, but apparently having 2 middlewares is causing everything else to fail.

christeredvartsen commented 4 years ago

Had a duplicate method in test code causing that fail. Started adding a JWT auth middleware like the basic auth one, but apparently having 2 middlewares is causing everything else to fail.

Want me to have a go at it?

pwolanin commented 4 years ago

Sorry for the slow response - have been vacationing (of a sort). I don't know why adding a very simple middleware broke the existing one for basic auth, but if you can get it working I think it would be more elegant than adding auth just to a single endpoint.

christeredvartsen commented 4 years ago

I just took your branch for a spin and there are some syntax errors within it. Do you have some local changes that you have forgotten to commit?