ga4gh / task-execution-schemas

Apache License 2.0
80 stars 27 forks source link

Require JWT Bearer Token authentication #198

Open uniqueg opened 9 months ago

uniqueg commented 9 months ago

Problem

Currently, the TES API specification does not mandate implementers to require any authentication scheme. The specification merely mentions the following relevant paragraphs in its service description:

Is is envisaged that most TES API instances will require users to authenticate to use the endpoints. However, the decision if authentication is required should be taken by TES API implementers.

If authentication is required, we recommend that TES implementations use an OAuth2 bearer token, although they can choose other mechanisms if appropriate.

Arguably, any public API must be protected from abuse by authentication (and authorization) policies, especially when offering access to compute resources, so mandating an authentication policy is prudent.

Moreover, enforcing at least one common, widely used authentication policy required to be supported by all implementations makes it considerably easier to design systems/environments in which multiple TES implementations are used concurrently, e.g., in federated TES networks.

Overview

OpenAPI currently (version 3.x) supports the following authentication (or authorization) schemes:

Other GA4GH OpenAPI specifications define the following securitySchemes:

Specification SecuritySchemes Applied
Data Connect bearerAuth:
type: http
scheme: bearer
Globally
Data Repository Service (DRS) BasicAuth:
type: http
scheme: basic
description: |
A valid authorization token must be passed in the 'Authorization' header,
e.g. "Basic ${token_string}"
BearerAuth:
type: http
scheme: bearer
description:
A valid authorization token must be passed in the 'Authorization' header,
e.g. "Bearer ${token_string}"
PassportAuth:
type: http
scheme: bearer
x-in: body
bearerFormat: JWT
description:
A valid GA4GH Passport must be passed in the body of an HTTP POST request
as a tokens[] array.
Globally
Service Info bearerAuth:
type: http
scheme: bearer
bearerFormat: JWT
Globally
Service Registry bearerAuth:
type: http
scheme: bearer
bearerFormat: JWT
Globally
Tool Registry Service (TRS) BEARER:
type: apiKey
name: Authorization
in: header
Individually for all endpoints
Workflow Execution Service (WES) N/A

"Users must supply credentials that establish their identity and authorization in order to use a WES endpoint. We recommend that WES implementations use an OAuth2 bearer token, although they can choose other mechanisms if appropriate. WES callers can use the auth_instructions_url from the service-info endpoint to learn how to obtain and use a bearer token for a particular implementation."
N/A

Beacon was omitted due to its complex structure of several OpenAPI specificatoin files, but does not appear to prescribe any authentication method

Bearer token authentication seems to be the consensus for those API specifcations that do prescribe authentication schemes, with the following exceptions:

Proposed solution

Mandate implementers to support JWT OAuth2 bearer tokens for authentication and apply it globally to all endpoints.

Alternative solutions

denis-yuen commented 9 months ago
  • While the TRS securityScheme is called BEARER, it actually mandates apiKey-based authentication; assuming that its name expresses its intention to be used equivalently to bearer token authentication, this is presumably a remnant from migrating the specification over from Swagger/OpenAPI 2.x, where bearer authentication was not explicitly supported with its own scheme

Very possible. The other part of the explanation is that the endpoints are default public. i.e. If a workflow is public, you don't need auth, you only need auth if the workflow is not public yet In other words, auth isn't heavily used yet AFAIK

We should probably fix the security scheme before it is

uniqueg commented 9 months ago

Good point @denis-yuen. Although it's probably still good practice to implement an authentication scheme in TRS implementations to better guard against, e.g., DDoS attacks. And as it is, as long as you define any security Scheme in the specs and apply it, fully compliant implementations would have to support it. Which, for TES, is the main reason why we, or at least I, want to put it in. But your mileage may vary for TRS.

But if you wanna keep it in, I agree that it's probably a good idea to change it.