Identity / IdentityProvider

[thiagodelgado111]

I was looking at the feat/identity PR again. This is how the Identity looks now on the left and how I imagine it could look on the right (sorry for the horrible handwriting):

Key

• publicKey : String | Buffer
• privateKey : String | Buffer
• encoding : String (e.g: hex)

The basic Identity object would look like this:

• id: String
• key: Key
• provider : IdentityProvider (?)
• type: String (eg. ColonyIdentity, Ethereum address, IPFS PeerID, Dat address, DID, even Twitter can be an IdentityProvider)
• sign: (e: Object) => { key: Key, signature: String }
• verify : (e: Entry) => boolean
• (signature) orbitKeyOwnershipProof : String
• (pkSignature) externalIdentityOwnershipProof : String

sign and verify would be provided by the IdentityProvider so when we create an identity the object get a hold on whatever the implementation is.

This way we don't need to pass the provider to the log but just the identity object which will be responsible for signing and verifying entries (and to provide the necessary info for the AccessController canAppend method).

How does it sound?

[thiagodelgado111]

If that looks good, we could create a orbit-chained-ethereum-identity-provider which would depend on ethers, ethereumjs-account or web3, whatever we need, and extends the identity API to include an ethereum account property to the identity

[shamb0t]

Hey thanks for checking this out, Thiago! The identity needs to be serialized, how do you envision serializing the sign and verify functions? Also it is for public information and therefore shouldn't include the private key imo, its a job for the keystore, which means you need another in-memory-only object to handle interfacing with that

[thiagodelgado111]

The identity needs to be serialized, how do you envision serializing the sign and verify functions

We don't have to rely on stringify for it, we could provide a serialize function

Also it is for public information and therefore shouldn't include the private key imo, its a job for the keystore, which means you need another in-memory-only object to handle interfacing with that

Yeah but it wouldn't be added to the entry or anything. Actually, I only mentioned the private key because it's in the orbit-db-keystore format of key

My view is that it would make sense, that way the Identity is responsible for signing/verifying entries and the IdentityProvider would only provide an Identity, not the signing/verification functionality

[shamb0t]

It would make sense if we could serialize the verifyFn and allow identities to be self-verifying but its not clear how to do that...and without that not sure its the responsibility of identity to verify other identities. @haadcode made the point that we need to hit disk for every sign(entry) rn (perhaps this was your thinking) so could be a reason to have an ephemeral .key/sign field that we can load from disk once

[thiagodelgado111]

The serialization here only matters when we're adding the identity to the Entry, isn't that right? We wouldn't serialize the functions, I'm proposing that so ipfs-log would depend on the Identity only and not the provider

[thiagodelgado111]

@shamb0t 11:18 
@thiagodelgado111 thanks for your thoughts on the shape of identity! Id love to have self-verifying identities but its not clear to me how we serialize the verifyFn, can you elaborate how peers would verify other identities? Or is your objection to the naming of identity-provider? The way I see it is we will have a type in identity which something like provider (or manager if you prefer) can feed to a resolver which tells it how to verify the identity...is what you are suggesting the same but doing away with provider and keeping that functionality in identity? (the non-serialized part)

@thiagodelgado111 11:21
Yes, I'm not proposing we serialize those functions, I'm proposing that so ipfs-log would depend on the Identity only and not the provider
And yes, I think the IdentityProvider should only provide identities, so the naming maybe IdentityManager would make more sense and then we pass that to the log instead of an identity
I just thought that making the Identity responsible for signing/verification and adding a serialization function would be easier

@shamb0t 11:28
Okay I see what you mean with the naming.... identity being responsible for signing makes sense to me, but not so much verifying other identities or entries. There will still need to be a resolver step between reading the entry/identity and retrieving the appropriate verify function for that type of identity and perhaps just that part can be fed to the provider/manager

@thiagodelgado111 11:35
Ok, I think it makes sense to go with the name change to manager. I'd still recommend calling signature and pkSignature as something like orbitKeyOwnershipProof and externalIdentityOwnershipProof :)

@shamb0t 11:35
Either way I think log will still depend on both identity and a provider/manager unless we collapse those into one, but again dont think the identity object should be verifying other identities...this was initially why we have the provider field in identity so we could just pass identity. The type is what is really describing the kind of verification function needed...and provider is kind of a misnomer since its really just a wrapper for signer/verifier so maybe manager is a better name
Ok agreed the names can be more descriptive, those are pretty good

[thiagodelgado111]

So Identity will look like this?

• id
• publicKey
• orbitKeyOwnershipProof
• externalIdentityOwnershipProof
• type

Will it change depending on the Identity "type"? I mean, each different IdentityManager implementation could have their own Identity format? If that's correct then the default Identity format could be even simpler, no? We could have just the id, publicKey and type 🤔

[haadcode]

Thanks for the discussion and proposals on this! I agree there's an opportunity to clarify the identity and the identity provider a little.

I don't think IdentityProvider should be called a manager. It doesn't manage identities as in allow users to modify the identity through it nor internally keep a track of identities. It just provides an orbit-db identity object that describes the identity being used.

Agreed on the naming of the signature field, these could be improved. I think they should still be called signatures since that's what they are. The first on (pkSignature) is a signature for the id and signature is a signature for the "id in possession of publicKey". Perhaps they could be called idSignature and signature respectively, or keySignature and identitySignature.

Re. adding a .key property to Identity: I agree with @shamb0t that we shouldn't pass the keys around if possible (even if just in-memory) to keep the security boundaries as clear as possible (eg. to prevent accidental private key usage). Second, the key itself is not needed by the Identity and I don't think we have a need to expose it either (ie. no user/module requires direct access to the key). Third, the key itself has a certain API, so if we use the key directly in Identity, it would make the key (from keystore) a dependency of Identity which now is removed by passing a (hex) string of the key instead of the key itself. I'd rather keep it this way to facilitate usage other types of keys and keystores in the future (in fact, we had previously the direct dependency on the key in log and had to do all the .getPublic('hex') calls because of that dep).

On the "log depending on IdentityProvider instead of just identity" and "passing only identity instead of identity provider": we discussed this at some point with @shamb0t and decided not to add .sign/.verify functions to Identity as per the reasoning above: it makes sense an identity can sign for their data, but an (individual) identity to verify other identities doesn't feel as natural and fitting. The identity will regardless need the provider internally and call its sign/verify functions. More generally, the idea is that IdentityProvider can "resolve" different identities (based on the .type field in Identity that @shamb0t was going to add), so for example a user can use ColonyIdentity but still use IdentityProvider to verify another type of identity. Third point to this is that Access Controller in .capAppend will need the IdentityProvider to call .verifyIdentity as per user's need (we don't verify it by default on every append) and even though this could be passed in via the identity, I feel putting that too into identity doesn't feel like the right place.

To summarize:

• I think there's an opportunity to improve the naming of the pkSignature and signature fields in Identity
• I don't think we should add the key to the Identity class/instance
• I don't think IdentityProvider should be called IdentityManager
• I don't think we should add sign() and verify() to Identity but keep them in IdentityProvider. We could add sign() as a convenience method though...