Skip to content

Rename operation ID to token #16

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 4, 2025
Merged

Rename operation ID to token #16

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eeac0c7
Nexus-Operation-Token
bergundy Feb 4, 2025
f772a76
Query or header
bergundy Feb 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 44 additions & 12 deletions SPEC.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,14 @@ An operation is addressed using the following components:
- The containing endpoint, a URL prefix (e.g. `http://api.mycompany.com/services/`)
- Service Name - A grouping of operations (e.g. `payments.v1`)
- Operation Name - A unique name for the given (e.g. `charge`)
- Operation ID - A unique ID assigned by the handler as a response to a [StartOperation](#start-operation) request.
- Operation Token - A unique token assigned by the handler as a response to a [StartOperation](#start-operation)
request.

The service name, operation name, and operation ID MUST not be empty and may contain any arbitrary character sequence as
long as they're encoded into the URL.
The service name and operation name MUST not be empty and may contain any arbitrary character sequence as long as
they're encoded into the URL.

An operation token MUST not be empty and contain only characters that are valid HTTP header values. When passing a token
as part of a URL path make sure any special characters are encoded.

## Schema Definitions

Expand Down Expand Up @@ -57,10 +61,10 @@ The `OperationInfo` object MUST adhere to the given schema:
```yaml
type: object
properties:
id:
token:
type: string
description: |
An identifier for referencing this operation.
A token for referencing this operation.

state:
enum:
Expand Down Expand Up @@ -150,17 +154,27 @@ The body may contain arbitrary data. Headers should specify content type and enc
Request to cancel an operation. The operation may later complete as canceled or any other outcome. Handlers should
ignore multiple cancelations of the same operation and return successfully if cancelation was already requested.

**Path**: `/{service}/{operation}/{operation_id}/cancel`
**Path**: `/{service}/{operation}/cancel`

**Method**: `POST`

#### Request Headers

The operation token received as a response to the Start Operation method. Must be delivered either via the `token` query
param or the `Nexus-Operation-Token` header field.

#### Query Parameters

- `token`: The operation token received as a response to the Start Operation method. Must be delivered either via the
`token` query param or the `Nexus-Operation-Token` header field.

#### Response Codes

- `202 Accepted`: Cancelation request accepted.

**Body**: Empty.

- `404 Not Found`: Operation ID not recognized or references deleted.
- `404 Not Found`: Operation token not recognized or references deleted.

**Headers**:

Expand All @@ -172,12 +186,20 @@ ignore multiple cancelations of the same operation and return successfully if ca

Retrieve operation result.

**Path**: `/{service}/{operation}/{operation_id}/result`
**Path**: `/{service}/{operation}/result`

**Method**: `GET`

#### Request Headers

The operation token received as a response to the Start Operation method. Must be delivered either via the `token` query
param or the `Nexus-Operation-Token` header field.

#### Query Parameters

- `token`: The operation token received as a response to the Start Operation method. Must be delivered either via the
`token` query param or the `Nexus-Operation-Token` header field.

- `wait`: Optional. Duration indicating the waiting period for a result, defaulting to no wait. If by the end of the
wait period the operation is still running, the request should resolve with a 412 status code (see below).

Expand Down Expand Up @@ -217,7 +239,7 @@ Retrieve operation result.

**Body**: A JSON serialized [`Failure`](#failure) object.

- `404 Not Found`: Operation ID not recognized or references deleted.
- `404 Not Found`: Operation token not recognized or references deleted.

**Headers**:

Expand All @@ -229,10 +251,20 @@ Retrieve operation result.

Retrieve operation details.

**Path**: `/{service}/{operation}/{operation_id}`
Copy link

@cretz cretz Feb 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think we should remove the token from the URL path on these. At the very least it should be a query parameter, though in the URL path makes more sense IMO. Using headers as required parameters to specific HTTP calls is strange (as opposed to general metadata like auth). Referencing different operations in a call should be different URLs IMO.

Is the only reason to remove from the URL because after discussion we expect this string to be bigger or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is similar to the callback token where we decided to not put it in the URL since URLs are typically logged in access logs and we don't want potentially sensitive information leaking there.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it is sensitive shouldn't it be encrypted?

Copy link

@cretz cretz Feb 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is token meant to be more sensitive than ID for this use case? Calling a string to reference the operation an "id" or "token" doesn't really change the purpose. Using headers as required parameters for these calls to work is rough devexp I wouldn't expect from normal HTTP APIs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it is sensitive shouldn't it be encrypted?

Yes, but it still doesn't stop someone from potentially stealing your token.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My concern is this is a significant usability regression of the Nexus HTTP API. Is it possible to not go backwards in dev experience? For instance, maybe this header approach can be an alternative instead of the required way to give this parameter?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just thinking the same thing. We could have two approaches, so users that are concerned with tokens leaking could send it as a header. That's essentially what the related Go SDK PR allows.

Let's keep it path based for now and rename, we can add the header approach later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Working on this now and feeling a bit on the fence here. Let me think about this some more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed a change, LMK what you think.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Could also understand if we wanted to reserve some paths for an eventual no-token future, or I could also understand if you wanted token to be a query parameter instead of path piece so it can be more easily made optional in lieu of a header later.

**Path**: `/{service}/{operation}`

**Method**: `GET`

#### Request Headers

The operation token received as a response to the Start Operation method. Must be delivered either via the `token` query
param or the `Nexus-Operation-Token` header field.

#### Query Parameters

- `token`: The operation token received as a response to the Start Operation method. Must be delivered either via the
`token` query param or the `Nexus-Operation-Token` header field.

#### Response Codes

- `200 OK`: Successfully retrieved info.
Expand All @@ -245,7 +277,7 @@ Retrieve operation details.

A JSON serialized [`OperationInfo`](#operationinfo) object.

- `404 Not Found`: Operation ID not recognized or references deleted.
- `404 Not Found`: Operation token not recognized or references deleted.

**Headers**:

Expand Down Expand Up @@ -312,7 +344,7 @@ For invoking a callback URL:
- Issue a POST request to the caller-provided URL.
- Include any callback headers supplied in the originating StartOperation request, stripping away the `Nexus-Callback-`
prefix.
- Include the `Nexus-Operation-Id` header, `Nexus-Operation-Start-Time` and any `Nexus-Link` headers for resources
- Include the `Nexus-Operation-Token` header, `Nexus-Operation-Start-Time` and any `Nexus-Link` headers for resources
associated with this operation to support completing asynchronous operations before the response to StartOperation is
received. `Nexus-Operation-Start-Time` should be in a valid HTTP format described
[here](https://www.rfc-editor.org/rfc/rfc5322.html#section-3.3). If is omitted, the time the completion is received
Expand Down