nmos-testing icon indicating copy to clipboard operation
nmos-testing copied to clipboard

IS-10/BCP-003-02: Resource Server tests

Open garethsb opened this issue 4 years ago • 8 comments

Issue #520 covers updating the tests for Authorization Servers themselves. Interoperable behaviour of Resource Servers (Nodes and Registry) and Clients (Node acts as a client of an authorization-enabled Registry) is just as important.

For BCP-003-01 (Secure Communications, i.e. TLS), we have a particular test suite to test the details deeply, and then all the IS-04/-05/-08/-09 test suites can also be run in 'secure' mode in order to do broad testing of TLS. What would be the equivalent for authorization? Nodes' client behaviour communicating with the Auth Server and then Registration API needs testing as well as their own server behaviours in validating their clients communicating with authorization-enabled Node and Connection API, etc.

In the case of the 'deep' test suite, as well as testing the basic OAuth 2.0 behaviour, the NMOS-specific bits should be tested thoroughly, e.g. see IS-10 Behaviour - Access Tokens requirements for client_id/azp and x-nmos-* private claims.

In the case of updating the IS-04/-05/-08 test suites to support 'auth' mode, the testing tool infrastructure could be updated to apply access tokens to all requests. These could be kept as simple as possible in most cases, with the simplest possible x-nmos-* private claim for example. A new test case would be needed for the only specific requirement mentioned in BCP-003-02 for client_id verification in IS-04 Registration API.

In order to test these things, an Auth Server will be needed. To keep things self-contained, as for mDNS, DNS-SD, etc. the testing tool could include a mock Auth Server. https://flask-oauthlib.readthedocs.io/en/latest/oauth2.html might be usable?

garethsb avatar Jun 26 '20 08:06 garethsb

The IS04 Suites has a very similar client-server interaction. Perhaps following the same recipe is possible here.

  • IS1001Test.py ( Auth Server )
    • Announcements
    • Endpoints ??? They are dynamic with server metadata
    • Client Registration
    • Correct JWK ( at least one RSA )
  • IS1002Test.py ( Auth Client )
    • Discovery
    • Test Protected and Unprotected endpoints ( IS-04/-05/-08 )
    • Error responses
      • 401 Missing header
      • 400 Malformed header
      • 403 Failed token verification
    • Maybe more Oath specific behavoir?

As for having an ENABLE_AUTH mode might be possible. There's this helper that could handle adding auth to all requests?

The BBC auth server is based on the flask-oauthlib perhaps that could be the mock? It would be an import from pypi possibly.

There's also the auth0 which might be another option for a mock.

prince-chrismc avatar Jun 26 '20 15:06 prince-chrismc

To steal Brad's phrasing, I'd hope we can take a 'crawl walk run' approach here.

As a simple starting point (whilst I accept it wouldn't be very user friendly), configuration parameters could be used to house a sufficiently privileged and long lived token which the user obtains on behalf of the testing tool. This should enable the existing set of tests to be run in an authorised fashion without taking on any responsibility for running or interacting with an auth server. If necessary we could take multiple tokens here in order to test the BCP-003-02 requirements.

An IS1003? (Resource Server) suite could then be added with some very basic starting points, testing access to the base path of the API using the above token, testing access without a token (to ensure access is rejected), and testing access using a compromised token (a false one which may include the correct claims, but signed by an unknown party, for example).

With this starting point, we could then expand down the slightly more involved avenue of either interacting with or running an auth server for a variety of purposes.

andrewbonney avatar Jul 01 '20 14:07 andrewbonney

As a simple starting point (whilst I accept it wouldn't be very user friendly), configuration parameters could be used to house a sufficiently privileged and long lived token which the user obtains on behalf of the testing tool. This should enable the existing set of tests to be run in an authorised fashion without taking on any responsibility for running or interacting with an auth server. If necessary we could take multiple tokens here in order to test the BCP-003-02 requirements.

Sounds like a good strategy. 👍

garethsb avatar Jul 01 '20 16:07 garethsb

Suppose changing https://github.com/AMWA-TV/nmos-testing/blob/1636bd6645b0d5d3e57e21719253863e0e838816/nmostesting/TestHelper.py#L61 to

        req = requests.Request(method, url, headers={'Authorization': 'basic {}'.format(CONFIG.AUTH_TOKEN)}, **kwargs)

would get us crawling? I just wonder if that's enough of a first step :man_shrugging: Where do the tests need to be for this to be before it can be elevated?

But certainly on board :boat: with taking baby steps. It would certainly POC gareth's suggestion for "the test suites can also be run in 'auth' mode in order to do broad testing"

prince-chrismc avatar Jul 01 '20 20:07 prince-chrismc

Ok. How's this for an initial list of jobs (feel free to add/edit/remove/suggest):

  • [x] Add an 'authorized' mode to the testing tool, with space for two tokens (for different clients) in the config file. Use one of these with every request in this mode.
    • NB: This probably ought to work only when in 'https' mode. Initially the mock IS-04-01 registry wouldn't check tokens.
  • [x] Add an IS-10 resource server test suite which tests access to an API using valid, invalid and absent tokens.
    • TBC: How to target a specific NMOS API with this? Or should these be added as 'auto' tests or similar to each individual suite? In the future we'll probably want the option to request specific claims and test specific resources too.
  • [x] Add a BCP-003-02 test suite which tests registry behaviour using two different clients' tokens, with attempts to modify each other's resources (again the alternative is to combine with IS-04-02 in a similar way to how BCP-002 is tested for Nodes).
  • [x] Edit the existing IS-10 auth server suite to remove unnecessary tests, and to comment out those which could be modified to support the current design.

Later...

  • [x] Add support for testing token presence and claims in the IS-04-01 mock registry.
    • Token verification could be ignored initially, use a cert in a config parameter, or based upon the below...
  • [ ] ~~Expand testing tool to discover an existing auth server on the network in order to obtain its JWKs for token verification.~~
  • [ ] Update the IS-10 auth server suite to cover the new discovery and metadata design.
  • [ ] Expand resource server testing to include a unique path test within each test suite, ensuring that very granular read/write claims work correctly.
  • [ ] Test behaviour of IS-04-02 subscription WebSockets in relation to NMOS claim paths (ie. /sources/* but the subscription they're connecting to covers /flows/)
  • [ ] Test behaviour of IS-07 WebSockets using NMOS Source claims

andrewbonney avatar Jul 02 '20 07:07 andrewbonney

Add an IS-10 resource server test suite which tests access to an API using valid, invalid and absent tokens.

We've just added https://github.com/AMWA-TV/nmos-authorization/pull/51 with examples based on RFC 6750 Section 3 and 3.1, which defines which of 400 Bad Request, 401 Unauthorized and 403 Forbidden are correct in each of these cases, and also what the WWW-Authenticate response header (and the JSON body) parameters should be.

garethsb avatar Aug 06 '20 14:08 garethsb

In the case of the 'deep' test suite, as well as testing the basic OAuth 2.0 behaviour, the NMOS-specific bits should be tested thoroughly, e.g. see IS-10 Behaviour - Access Tokens requirements for client_id/azp and x-nmos-* private claims.

@peterbrightwell Could the security group contribute tests that demonstrate a resource server validates requests against the x-nmos-* private claims?

E.g. when the JWT includes:

"x-nmos-connection": {
    "read": [
        "single/senders/c72cca5b-01db-47aa-bb00-03893defbfae/*",
        "single/senders/171d5c80-7fff-4c23-9383-46503eb1c63e/*"
    ]
}

then a request to /single/senders/c72cca5b-01db-47aa-bb00-03893defbfae/active should succeed but a request to /single/senders/a2655c48-8a46-4c82-b9bc-98760d59d7f8/active should be denied.

garethsb avatar Feb 10 '21 10:02 garethsb

@lo-simon @jonathan-r-thorpe it would be good to reevaluate how many of the tasks mentioned in this issue are still relevant!

garethsb avatar Feb 27 '23 10:02 garethsb