Proposed changes to SDK

[tmarkovski]

Objective

With the changes to AccountProfile and it's use in server configurations, we have an opportunity to consolidate the service options and lessen the complexity of service construction.

Proposal

Here are some changes I propose we do.

Changes to Account Profile

• Update the SignIn and CreateAccount endpoints to return a string that's a base64url encoded string of the serialized byte array of the token.
• Change the reference of AccountProfile or Profile to AuthToken or AuthenticationToken. This makes it clearer as to what this profile represents and how it relates to commonly used API tokens, access tokens, etc.
• When SignIn or CreateAccount is called, if the response sends an unprotected profile, set the token for the current service instance as active. This won't happen if the user is using an email to sign in, which should be most of the cases, so they will need to set it again. I really don't like this experience, and hope we can find something that resembles a more common developer flow.
• Ensure that when passing an auth token that is protected, the SDK throws an error with a message indicating how this can be unprotected, with a documentation link explaining why this protection is in place

Changes to Service Options

• Add AuthToken to ServiceOptions (proto) so that tokens can be added in code configurations in server scenarios.
• Move ServerConfig to ServiceOptions (sdk proto). This really just flattens the ServiceOptions instead it having a complex structure. Example:

message ServiceOptions {
  string server_address = 1; // default = "prod.trinsic.cloud"
  int32 server_port = 2; // default = 443
  bool server_use_tls = 3; // default = true
  string auth_token = 4; // default = <empty>
  string default_ecosystem = 5; // default = "default"
  // etc
}

• gRPC Channel reuse. I know there's a separate discussion on this, we can leave this feature intact, but I do want to point out that this can be filled with side effects and for the most part it's really unnecessary that we expose the actual gRPC channel in the services. It introduces complexity in object and service construction, there's the option that users can simply toggle options on it that would make the channel incompatible with the backend (turn of compression). It's also a big surface for security related bugs.

Changes to accommodate ecosystem updates

• Add DefaultEcosystem to the ServiceOptions so clients can configure their SDKs to point to a specific ecosystem instance
• Update default trinsic ecosystem name. Currently this is set to "default". This name appears in schema URLs such as https://schemas.trinsic.cloud/<default>/<credential type>/ etc. We can change this something else, maybe trinsic, or global, or .
• Remove the need to specify ecosystem when signing in, and read this from the service options. Clients would not need to work across different ecosystems, so having one place where this is configured is good.

Changes to Credential Service

• Update documentation to reflect reveal document in proof creation should be empty, unless the users knows how to construct a valid frame
• Ensure methods expose the proto object directly

[fundthmcalculus]

Opinions

• "Update the SignIn and CreateAccount endpoints to return a string that's a base64url encoded string of the serialized byte array of the token." - don't do this. It creates a disparity in the service endpoints for something that should be handled at the SDK level. The server objects should be the server objects, the SDK should handle user presentation and storage.
• "Change the reference of AccountProfile or Profile to AuthToken or AuthenticationToken." - make this AuthenticationToken, we haven't tried to keep our class names short to this point, so let's be consistent.
• "Ensure that passing an auth token that is unprotected" - I assume you mean protected. We currently do this, but the error message could definitely be better.
• I like the ServiceOptions changes. This eliminates the issues of constructor overloading (odd in Python, prohibited in go and typescript). We simply have sane defaults for any parameters not set
• As far as channel reuse: gRPC recommends that you do reuse channels to reduce server overhead, but we may not need that performance/resource gain. Ruby currently doesn't support channel reuse, and as far as I can tell, nobody is using it anyway. It creates interesting issues with channel scoping and garbage collection. I lean towards removing this capability since it will simplify the environment.

[sethjback]

Update Signing and CreateAccount

While I agree with @fundthmcalculus that we want the things that are handled at the SDK level to be handled there, we also want to remove friction for users with things they may not want / need to know how to interact with. Think JWT - it is possible to decode it and figure out what is in there, but no one practically does, unless for some edge cases. I would be OK with having a token be treated as a base64 string for the most part, with the SDK handling the management "under the hood", so long as developers could, if needed get at the information. (But with the change to using a default wallet/ecosysem my guess is that most users won't really need access to any of this and would rather it "just worked")

Short names

I would argue that short names are going to be easier to use but agree that we want consistency either way.

Ensure that passing an auth token that is unprotected

This is a great idea, but could be a little challenging since until we validate the proof we won't really know if the token is blinded (one of the features [?] of using oberon)

GRPC channel reuse

I think we shouldn't fall into the case of premature optimization here - the small performance gains for channel reuse are probably not worth added friction, and if the user of the SDK does need that (for example doing scripted batch calls), they can provide the channel.

Service Options change

I like this keeping things flat and simple seems best.

Default Ecosystem in the service options

I think this is a good idea. Since we are pulling the ecosystem id from the auth token, we will need to parse and decode - should we make sure the ecosystem ID int he service options and the one in the token match? Or should we simply have a convenience function, e.g. GetEcosystemID() that returns it parsed from the token? Just want to make sure we don't allow someone to set an eco ID the token doesn't have access to and then always get an error (that would be really confusing)

[fundthmcalculus]

@sethjback I agree with the user wanting it to "just work" as a base64 string. My point is that the SDK should parse the base64 string and send the token object "over the wire" to the server. That way, the server has its api defined in protobuf, as opposed to a serialized protobuf object (which, while it makes sense for the okapi FFI, does not make as much sense here).

GRPC channel reuse is one of those things that we either have to support, or we don't. If the user has the option to provide the channel, we're right back to square one with the issue of providing the serviceoptions or the channel, the problem we're trying to remove. I would argue that for the scripted API calls, the current SDK API allows you to open a connection to the server, and then do everything inside of that WalletService (for example) instance, then close. You don't have to open and close connections that much, since you can change the profile on the fly. Can we make it such that the user can change the ecosystem on the fly as well? If so, we've eliminated the only other reason to close and reopen the (same) connection - region/zone issues require a channel change for obvious reasons.

[sethjback]

My point is that the SDK should parse the base64 string and send the token object

Makes sense - agree we need to not change the server api with the outgoing calls, and returning a base64 string from login that can be directly exposed to the user and then parsed accordingly under the hood also seems to make sense. I think the idea is that what we show the user (by default) is the encoded token so that's all they have to deal with.

GRPC channel reuse is one of those things that we either have to support, or we don't.

I'm not sure about this...wouldn't it be possible, in theory, to check for a channel ever time a call was made and create a new one if not? There are obvious but perhaps minimal performance penalties to that (e.g. re-negotiating the http2 connection to the server, and you loose all the http2 connection reuse goodness) but allowing for a reasonable default (we create a channel and reuse it) and giving access to replace the channel to the dev if they really want to seems appropriate?

I guess my thinking is if we make is so you have to be explicit if you want to provide a channel (i.e. don't make it the path of least resistance) it gives flexibility if really needed. But honestly, in the SDKs I interact with regularly that are gRPC based you never have to provide a channel and I can't think of a single instance where I needed or wanted to. This really seems like something we should just take care of, and if someone runs into a special case where they need access, make it possible.

[tmarkovski]

The base64 change to profile is simply for convenience i.e. copy/paste uses. I agree that we should add convenience methods GetEcosystemId() and GetWalletId() that simply extracts this from the profile.
@fundthmcalculus we won't be changing the way we deal with profiles over the wire, just how we expose them in the SDK.
The change to protection is something that's already part of the AccountProfile, there's a field called Protection I think, which simply indicates that the server returned a token which was blinded, and you'll need to unprotect it before using it. This is already a feature in some wrappers, that if you pass AccountProfile with protection on, it'll just throw. I'm not suggesting anything new, just making sure this baseline case is covered.

The channel reuse option can still be provided, by exposing the GrpcClient instance, which has full access and control to the Channel field, I'm just suggesting that we remove this from the constructors and config parameters. It can still be there, for those that want it, it just won't confuse developers about what they need to pass there.

Lastly, the AuthToken vs AuthenticationToken item - it might actually be better to go with the short option here, simply to stay true to the nature of the token. The token isn't authentication token specifically, it's also authorization token. It is passed in the Authorization http header. This is why it makes sense to call it AuthToken, as it captures both the N and Z nature of it.

[tmarkovski]

@sethjback @fundthmcalculus I'll use this thread to propose one more addition to the SDK. This change is client side only, it is not associated with an endpoint (directly).

In most scenarios, users will be signing in using their email. This means their tokens will be protected (blinded) and developers will need to take care of the blinding. Currently, this experience is little flimsy.

account_rpc = ctor(account_rpc)

auth_token_blinded = account_rpc.sign_in(email)

auth_token_unblinded = account_rpc.unprotect(auth_token_blinded, security_code)

account_rpc.options.auth_token = auth_token_unblinded

This experience is fine, but it's really confusing and not very familiar. It also leaves room for errors, by entering invalid security code, where users think they have a good token, but they don't, since the process of (un)blinding always succeeds.

I suggest that we add a second API that's a follow up to the Account.SignIn called SignInFinalize or SignInConfirm (or whatever name). The purpose and function of this endpoint will be to ensure that the SignIn process has completed and the outputs (token) are in the correct state and valid. The pseudocode of the implementation of this function would be:

function account_service.sign_in_finalize(auth_token: string, security_code: string)
  account_profile = account_profile.deserialize(base64url_decode(auth_token))

  if not account_profile.has_protection
    return auth_token

  auth_token = oberon.unblind(account_profile.auth_token, security_code)

  // this is new endpoint we're considering adding
  server_key = account_rpc.get_server_key()

  nonce = generate_random()
  proof = oberon.create_proof(auth_token, account_profile.auth_data, nonce)
  valid = oberon.verify_proof(proof, nonce, server_key)

  if valid
    account_profile.auth_token = auth_token
    account_profile.has_protection = false
    new_auth_token = base64url_encode(account_profile)

    self.options.auth_token = new_auth_token
    return new_auth_token
  else
    throw "invalid security code error"

Similar experience can be added for CreateEcosystem. The consistency here is that users can safely call the Finalize endpoint with any token and any security code, if the token has no protection, we simply return it, to support use cases where users login without email.
Essentially, this repurposes Protect and Unprotect and binds them closer to the API they're supposed to be used with. It abstracts the concept of protecting/blinding, by introducing finalize or confirm step, which in my opinion is clearer.

Under this change, the sign in experience will change to:

account_rpc = ctor(account_rpc)

auth_token_blinded = account_rpc.sign_in(email)

auth_token_unblinded = account_rpc.sign_in_finalize(auth_token_blinded, security_code)

What are your thoughts on this approach? Does this make things more clear as SDK user, or is it just same thing wrapped in a different name?

[sethjback]

I think the biggest change you are proposing is to auto-set the auth token in the options once unblinded? (seems like Unprotect + SetProfile do the same thing as SignInFinalize)

One comment on the unblinding - it is possible to blind an auth token multiple times - running unblind once may validly remove a layer of protection while the token is still protected (i.e. a proof won't validate). This wouldn't apply to how our API returns tokens (ATM) but is something to keep in mind as we are making tokens more easily passable between devices. It is conceivable someone will blind multiple times and then try to use something like SignInFinalize that is expecting only a single blind.

That being said I like it, but I think it is also hitting on a larger friction that has been on my mind and that has to do with the service instance pattern. So, for example, the above change will auto assign the auth token to the account service after sign in, but what happens if I want to use the credential service with that same auth token. I have to create the credential service, and then set the auth token in a separate step.

It would be ideal if we could avoid that final step (setting the auth token for every service) if possible.

[tmarkovski]

Yeah, you hit the nail right in the head there with the token setting across services. This is the reason why I think a default token management service that just works is needed, so that you don't have to retroactively set the token in all previous services you have instantiated after logging in - instead, it will lazily load once you have run the sign in logic.

Re: SignInFinalize, I'm thinking that this method might also have a server confirmation step as well, it won't just be unprotect + set token. It should also confirm with the server that they have unblinded the token successfully. This will allow us to add timeout protection to the generated tokens, as someone could potentially brute force the 6 digit security code in short amount of time.

My intent is that currently, we only do unprotect and set token and check against pk, but roll out more implementations after some time.
For advanced users, the ability to additionally blind tokens still remains, we're just introducing an opinionated way of working with tokens.

[sethjback]

@tmarkovski latest PR with token storage got me thinking and I wonder if it would be helpful to think of this in terms of contexts that can be set and used. This thinking is heavily influenced by how kubectl manages configs across multiple cluster, but it works pretty well and seems like it might be applicable here. Just food for conversation.

There are several pieces of information that combined equate to a successful auth: 1. environment (prod/staging/dev), 2. ecosystem, 3. token. All 3 of those need to match in order for auth to succeed. The proposal would be to combine those into a context that can be named and set.

So, for example, you could define the environment:

{
  name: "prod",
  serverEndpoint: "prod.trinsic.cloud",
  serverPort: 443,
  serverUseTLS: true
}

The Ecosystem:

{
  name: "Test",
  id: <id>
}

The AuthToken:

{
  name: "apiToken",
  token: "...."
}

And combine those into a context:

{
  name: "prod-test-api",
  environment: "prod",
  ecosystem: "Test",
  token: "apiToken"
}

Then you could do something like <service>.UseContext(name) and it would use the provided values. Obviously we would need to have reasonable defaults set for all of those values, but it seems like this would allow easy switching between values.

There could be a default context that just takes whatever the current values are if the user doesn't explicitly create one. E.g., if a dev calls SignIn the returned auth token, ecosystemid, and environment would all be set as the default. We could expose a SaveContext(...) that allows providing all the values, and maybe a SaveCurrentContext(<name>) that would take the current values and save them as the context name.