camaraproject / QualityOnDemand

Repository to describe, develop, document and test the QualityOnDemand API family
https://wiki.camaraproject.org/x/zwOeAQ
Apache License 2.0
37 stars 60 forks source link

Use header to carry API key instead of query string in callback notifications #157

Closed emil-cheung closed 1 year ago

emil-cheung commented 1 year ago

Problem description

   apiKey:
      type: apiKey
      description: API key to authorize requests
      name: apikey
      in: query
  /notifications:
    post:
      tags:
        - Session notifications callback
      summary: "Session notifications callback"
      description: |
        Important: this endpoint is to be implemented by the API consumer.
        The QoD server will call this endpoint whenever any network related event occurs.
        Currently only QOS_STATUS_CHANGED event is defined.
      operationId: postNotification
      requestBody:
        required: true
        content:
          application/json:
            schema:
              $ref: "#/components/schemas/Notification"
      responses:
        "204":
          description: Successful notification
        "400":
          $ref: "#/components/responses/Generic400"
        "401":
          $ref: "#/components/responses/Generic401"
        "403":
          $ref: "#/components/responses/Generic403"
        "500":
          $ref: "#/components/responses/Generic500"
        "503":
          $ref: "#/components/responses/Generic503"
      security:
        - apiKey: []

Possible evolution Change to use header to carry API key.

Alternative solution Use cookie to carry the API key. However, Camara API design guideline also mentions authentication/authorization requests should not rely on cookies or sessions.

Additional context Camara API design guideline

Information must not be exposed in the URLs Usernames, passwords, session tokens, and API keys should not appear in the URL, as this can be captured in web server logs, making them easily exploitable. For example, this URL (https://api.domain.com/user-management/users/{id}/someAction?apiKey=abcd123456789) exposes the API key. Therefore, never use this kind of security.

One of the key points in the API definition process is to specify and validate the security needs that will be maintained to guarantee data integrity and access control. There are multiple ways to secure a RESTful API, e.g. basic authentication, OAuth, etc., but one thing is for sure: RESTful APIs should be stateless, so authentication/authorization requests should not rely on cookies or sessions. Instead, each API request must come with some form of authentication credentials that must be validated on the server for each request.

jlurien commented 1 year ago

Hi @emil-cheung,

This issue will be likely obsolete once we update the spec to the new guidelines for Notifications (PR #155 ), as apiKey is currently only mentioned in relation to notifications and guidelines require to use a header for Authorization:

notificationAuthToken | string | OAuth2 token to be used by the callback API endpoint. It MUST be indicated within HTTP Authorization header e.g. Authorization: Bearer $notificationAuthToken | optional -- | -- | -- | --
emil-cheung commented 1 year ago

Hi @emil-cheung,

This issue will be likely obsolete once we update the spec to the new guidelines for Notifications (PR #155 ), as apiKey is currently only mentioned in relation to notifications and guidelines require to use a header for Authorization:

notificationAuthToken string OAuth2 token to be used by the callback API endpoint. It MUST be indicated within HTTP Authorization header e.g. Authorization: Bearer $notificationAuthToken optional

@jlurien thanks for the reply. When I check the PR, I see the similar concern raised. We could use the PR to discuss this issue.

hdamker commented 1 year ago

Decision taken within today's call to omit apiKey completely from the API spec. To be done in #155 (@akoshunyadi).

hdamker commented 1 year ago

Closed with #155