Test for security and stability

[nickevansuk]

There are currently no tests to ensure that correct security is implemented for the Orders feed.

Additional tests

booking-partner-authentication feature should be split into:

• booking-partner-openid-authentication-for-multiple-sellers - the authorization-code-flow related tests (required if multiple-sellers feature is enabled)
• booking-partner-openid-authentication - the client-credentials-flow tests (required if openid-authentication-for-multiple-sellers feature is enabled)
• booking-partner-authentication - this is a set of authentication-agnostic tests to check the booking partner partitioning is implemented correctly (always required)
  • booking-partner-partitioning-for-orders test
    • Orders from two different bookings partners must not be visible to each other, and UUID must be unique within each booking partner
    • Using primary booking partner auth, make a booking using a UUID, cancel it, and check for it in the Orders feed. Using secondary booking auth, assert that an Order with the same UUID is not present in the orders feed. Then using secondary booking partner auth, make a different booking using the same UUID, cancel it, and check for it in the Orders feed. Both booking should succeed.
  • booking-partner-partitioning-for-order-proposals test
    • Order Proposals from two different bookings partners must not be visible to each other, and UUID must be unique within each booking partner
    • (if minimal-proposal is implemented) Using primary booking partner auth, make a P call using a UUID, approve it, and check for it in the Order Proposals feed. Using secondary booking auth, assert that an Order Proposal with the same UUID is not present in the Order Proposals feed. Then using secondary booking partner auth, make a different P call using the same UUID, approve it, and check for it in the Orders feed.

multiple-sellers feature should also include:

• seller-access-restricted-by-auth test
  • Credentials for Seller (a) must not provide access make bookings for Seller (b)
  • Using primary seller auth, make a call to C1, C2, and P/B for the secondary seller, expecting all calls to fail with InvalidAuthorizationDetailsError (this is supported on the coverage/additional-auth-tests branch)

Additionally to assure stability of tests that use the Orders feed, the following tests should be added to the order-deletion feature:

• order-lifecycle test

  • Order should only be added to Orders feed after it updated following B
  • Make a booking using C1, C2, and B, then assert that the Order has not yet appeared in the Orders feed (does this need a timeout of some sort? is there a better way of doing this?)

• order-proposal-lifecycle test

  • Order should only be added to Orders feed after it updated following P
  • Create an Order Proposal using C1, C2, and P, then assert that the Order does not appear in the Orders feed (does this need a timeout of some sort? is there a better way of doing this?)

Additional test suite features to support these

To implement the above, recommend additional features in the broker microservice: 1) Allow Orders feed listening to take into account the booking-partner creds to use (primary or secondary, already configured) - this likely means harvesting double the number of Orders feeds (harvesting running for each set of creds, getOrdersFeedHeader looks like it could easily be parameterised to support primary and secondary). 2) Keep track of the UUIDs seen in each Orders/Order Proposal feed for each booking-partner creds. Add a new endpoint that allows test suite to assert that a particular UUID has not been seen yet.

[lukehesluke]

@nickevansuk

• the booking-partner-openid-authentication-for-multiple-sellers feature states

  (required if multiple-sellers feature is enabled)

  but the Booking Spec only says that it is recommended (https://openactive.io/open-booking-api/EditorsDraft/1.0CR3/#openid-connect-booking-partner-authentication-for-multiple-seller-systems). Should this instead be that multiple-sellers is required in order for this feature to be enabled?

• For the *-lifecycle tests, we could do the following to check that an Order/OrderProposal doesn't appear in the feed before some sort of update:

  1. Create an Order/ProposalFeedListener around steps 2 -> 4
  2. Create the Order/Proposal (C1 -> C2 -> B/P)
  3. Wait for 1 second.
  4. Make an update (e.g. cancellation/amendment/etc)
  5. Check that the returned Order from the feed is the updated one.

  If the Booking System was doing RPDE properly but not implementing the lifecycle properly then the first returned Order from the feed would be the un-updated Order.

  If the Booking System was not even doing RPDE properly (and therefore the feed includes an un-updated Order which then morphs into the updated Order without updating modified), then this will be shown with the 1 second gap.

  I'm suggesting this over the timeout approach because it only ends when something does appear in the feed, so will also work for Booking Systems which are just slow.

[nickevansuk]

Should this instead be that multiple-sellers is required in order for this feature to be enabled?

Yes that's right, good catch

Wait for 1 second.

Is there a risk here that the broker microservice harvester doesn't pick up these updates in time? I.e that the update happens in the RPDE feed before the page is harvested, and effectively hides the previous update (so we'd get false-positives?)

[lukehesluke]

I've moved this to the OpenActive Infrastructure project with the view that the unsettled questions here should be resolved to the point where implementation could reasonably be started