Skip to content
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.

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

SDK: Correctly encode auth header, and ser/de Credentials #68

Merged
merged 2 commits into from
Jul 13, 2023

Conversation

gefjon
Copy link
Contributor

@gefjon gefjon commented Jul 11, 2023

Description of Changes

Prior to this commit, the Rust SDK incorrectly encoded a Token passed to connect, making it impossible to re-connect an existing user. (Shows what I get for never testing that.) With this commit, auth tokens are correctly base64-encoded in an Authorization: Basic header with the username token, which allows re-connecting as an existing user.

Also, it was difficult to re-use Credentials from the Rust SDK, as:

  • they were opaque types with non-exported members
  • they did not implement any serialization / deserialization

This commit makes token.string and identity.bytes public fields, and implements SATN Serialize and Deserialize for Identity, Token and Credentials, allowing clients to save their credentials for re-use, e.g. to a file.

EDIT: A few more small changes, none of which felt important enough to be their own PR:

  • subscribe_owned accepts Vec<String>; behaves like subscribe.
    • While developing letrs, a demo game, we needed to generate query strings at runtime, and passing them to subscribe would have involved an unergonomic dance of taking references which would then be converted to owned containers by the SDK. subscribe_owned eliminates that, allowing client authors to construct a set of queries at runtime and pass it directly to the SDK, without intervening referencies and copies.
  • Docstrings for subscribe and subscribe_owned.
  • Methods on Identity and Token for converting to/from strings and byte-vectors.
  • Removed long-forgotten println within connect which wrote the computed URI to stdout.

API

  • This is a breaking change to the module API
  • This is a breaking change to the ClientAPI

If the API is breaking, please state below what will break

Prior to this commit, the Rust SDK incorrectly encoded a `Token` passed to `connect`,
making it impossible to re-connect an existing user.
(Shows what I get for never testing that.)
With this commit, auth tokens are correctly base64-encoded
in an `Authorization: Basic` header with the username `token`,
which allows re-connecting as an existing user.

Also, it was difficult to re-use `Credentials` from the Rust SDK, as:
- they were opaque types with non-exported members
- they did not implement any serialization / deserialization
This commit makes `token.string` and `identity.bytes` public fields,
and implements SATN `Serialize` and `Deserialize`
for `Identity`, `Token` and `Credentials`,
allowing clients to save their credentials for re-use, e.g. to a file.
@kim
Copy link
Contributor

kim commented Jul 11, 2023

What I ended up doing looks like this:

    fn client_credentials(&self) -> anyhow::Result<sdk::identity::Credentials> {
        let creds = SpacetimeCreds::encode_token(&self.private_key, self.identity)?;
        let token = axum::headers::authorization::Credentials::encode(&creds)
            .to_str()?
            .to_owned();
        Ok(sdk::identity::Credentials {
            identity: sdk::identity::Identity {
                bytes: self.identity.to_vec(),
            },
            token: sdk::identity::Token { string: token },
        })
    }

I wonder if it would be worth to have a dedicated spacetimedb-auth crate which handles these subtleties, and perhaps exports an opaque Token type?

@kim
Copy link
Contributor

kim commented Jul 11, 2023

Linking clockworklabs/SpacetimeDBPrivate#526, as that will probably need to be updated to work with this patch

- `subscribe_owned` accepts `Vec<String>`; behaves like `subscribe`.
  - While developing `letrs`, a demo game, we needed to generate query strings at runtime,
    and passing them to `subscribe` would have involved an unergonomic dance
    of taking references which would then be converted to owned containers
    by the SDK.
    `subscribe_owned` eliminates that, allowing client authors
    to construct a set of queries at runtime and pass it directly to the SDK,
    without intervening referencies and copies.
- Docstrings for `subscribe` and `subscribe_owned`.
- Methods on `Identity` and `Token` for converting to/from strings and byte-vectors.
- Removed long-forgotten `println` within `connect`
  which wrote the computed URI to stdout.
@gefjon
Copy link
Contributor Author

gefjon commented Jul 12, 2023

Pushed another commit with some more minor changes. These are purely additions to the API or docs, so shouldn't effect Kim's work at all.

Copy link
Collaborator

@jdetter jdetter left a comment

Choose a reason for hiding this comment

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

These only really seems to add things to the API (doesn't change any existing funcitons) so I think we're good to go on this. I did a pass on testing the SDK so I tested some of this functionality but not some of the more specific parts (functions for getting the token, identity, etc.)

@gefjon gefjon merged commit b543036 into master Jul 13, 2023
4 checks passed
cloutiertyler pushed a commit that referenced this pull request Aug 1, 2023
* SDK: Correctly encode auth header, and ser/de `Credentials`

Prior to this commit, the Rust SDK incorrectly encoded a `Token` passed to `connect`,
making it impossible to re-connect an existing user.
(Shows what I get for never testing that.)
With this commit, auth tokens are correctly base64-encoded
in an `Authorization: Basic` header with the username `token`,
which allows re-connecting as an existing user.

Also, it was difficult to re-use `Credentials` from the Rust SDK, as:
- they were opaque types with non-exported members
- they did not implement any serialization / deserialization
This commit makes `token.string` and `identity.bytes` public fields,
and implements SATN `Serialize` and `Deserialize`
for `Identity`, `Token` and `Credentials`,
allowing clients to save their credentials for re-use, e.g. to a file.

* SDK: Handful of small changes

- `subscribe_owned` accepts `Vec<String>`; behaves like `subscribe`.
  - While developing `letrs`, a demo game, we needed to generate query strings at runtime,
    and passing them to `subscribe` would have involved an unergonomic dance
    of taking references which would then be converted to owned containers
    by the SDK.
    `subscribe_owned` eliminates that, allowing client authors
    to construct a set of queries at runtime and pass it directly to the SDK,
    without intervening referencies and copies.
- Docstrings for `subscribe` and `subscribe_owned`.
- Methods on `Identity` and `Token` for converting to/from strings and byte-vectors.
- Removed long-forgotten `println` within `connect`
  which wrote the computed URI to stdout.
cloutiertyler pushed a commit that referenced this pull request Aug 1, 2023
* SDK: Correctly encode auth header, and ser/de `Credentials`

Prior to this commit, the Rust SDK incorrectly encoded a `Token` passed to `connect`,
making it impossible to re-connect an existing user.
(Shows what I get for never testing that.)
With this commit, auth tokens are correctly base64-encoded
in an `Authorization: Basic` header with the username `token`,
which allows re-connecting as an existing user.

Also, it was difficult to re-use `Credentials` from the Rust SDK, as:
- they were opaque types with non-exported members
- they did not implement any serialization / deserialization
This commit makes `token.string` and `identity.bytes` public fields,
and implements SATN `Serialize` and `Deserialize`
for `Identity`, `Token` and `Credentials`,
allowing clients to save their credentials for re-use, e.g. to a file.

* SDK: Handful of small changes

- `subscribe_owned` accepts `Vec<String>`; behaves like `subscribe`.
  - While developing `letrs`, a demo game, we needed to generate query strings at runtime,
    and passing them to `subscribe` would have involved an unergonomic dance
    of taking references which would then be converted to owned containers
    by the SDK.
    `subscribe_owned` eliminates that, allowing client authors
    to construct a set of queries at runtime and pass it directly to the SDK,
    without intervening referencies and copies.
- Docstrings for `subscribe` and `subscribe_owned`.
- Methods on `Identity` and `Token` for converting to/from strings and byte-vectors.
- Removed long-forgotten `println` within `connect`
  which wrote the computed URI to stdout.
@cloutiertyler cloutiertyler deleted the phoebe/sdk-auth-header branch August 1, 2023 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants