[BUG] Deactivating Identities

[nikolockenvitz]

Current Behavior

When setting the owner of an address/identity to 0x0 to deactivate it (as proposed in the spec), this is not visible when resolving the DID.

ethr-did-resolver/doc/did-method-spec.md

Lines 293 to 306 in e9c28d0

	### Delete (Revoke)
	Two cases need to be distinguished:
	- In case no changes were written to ERC1056, nothing needs to be done, and the private key which belongs to the
	Ethereum address needs to be deleted from the storage medium used to protect the keys, e.g., mobile device.
	- In case ERC1056 was utilized, the owner of the smart contract needs to be set to `0x0`. Although, `0x0`is a valid
	Ethereum address, this will indicate the identity has no owner which is a common approach for invalidation,
	e.g., tokens. Other elements of the DID Document may be revoked explicitly by invoking the relevant smart contract
	functions as defined by the ERC1056 standard. This includes the delegates and additional attributes. Please find a
	detailed description in the [ERC1056 documentation](https://github.com/ethereum/EIPs/issues/1056). All these functions
	will trigger the respective Ethereum events which are used to build the DID Document for a given identity as
	described in [Enumerating Contract Events to build the DID Document](#Enumerating-Contract-Events-to-build-the-DID-Document).

Expected Behavior

I would expect to clearly see that a DID has been deactivated when resolving it (either a flag in the document or not returning a document at all). I am missing specification here what the DID document should look like when the DID is deleted/revoked.

Failure Information

The root cause seems to be in the registry where I also opened an issue. But the spec should also clearly define what happens (i.e. what the DID document should look like) when a DID has been deleted.

To avoid updating the registry, one could check for the DIDOwnerChanged event.

[mirceanis]

Thank you for flagging this.
Indeed it is preferable to not have to change the registry and checking for that event seems like a good solution.

[mirceanis]

Thanks @PeterTheOne for proposing a fix for this.

I think a slightly different fix might be better.
Instead of returning null for a deactivated DID, an empty DID document would work better.
No public key entries, no authentication, no services, just the id, and at most the new 0x0 controller.

Systems wouldn't have to check for nullity, nor for a specific error message, but old signatures would be rendered invalid, making the DID effectively disabled.

@nikolockenvitz, @awoie, @PeterTheOne any thoughts?

[nikolockenvitz]

Agree on that. By setting it to null it can cause errors in existing implementations (i.e. accessing a property of the document whithout null-check).

The specs recommend to set the owner to 0x0, so it might be the easiest to return a did document with the owner set to 0x0 (pretending there is no problem with getting the owner from the solidity-mapping).

Public key entry would look like this then, right?

{
    "id": "did:ethr:0x...#controller",
    "type": "Secp256k1VerificationKey2018",
    "controller": "did:ethr:0x...",
    "ethereumAddress": "0x0000000000000000000000000000000000000000"
}

An application could easily check for that.

Regarding whether to return an empty did document except for that entry: I understand the current spec to mean that all needs to be revoked explicitly. That means updating the owner to 0x0 should work as updating it to any other owner (i.e. keep all service and public keys and delegates). In my opinion this would be fine on the library-level. An application could check for the 0x0 address of the controller.

But I also see that it makes sense to return an empty did document (except maybe for the controller entry) so that no one forgets to check and unintentionally uses a service / public key / delegate of a deactivated did. I just would expect the specs to be updated then and clearly say that setting it to 0x0 means deactivating the whole identity including automatically revoking all attributes.

To sum it up, to make it most convenient for users of this library, the latter one would be the best in my opinion. Just returning a did document with the above public key value.

[uport-automation-bot]

🎉 This issue has been resolved in version 3.1.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[uport-automation-bot]

🎉 This issue has been resolved in version 4.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀