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

Should "deactivated" be an error code? #468

Closed
peacekeeper opened this issue Nov 24, 2020 · 15 comments
Closed

Should "deactivated" be an error code? #468

peacekeeper opened this issue Nov 24, 2020 · 15 comments
Assignees
Labels
pr exists There is an open PR to address this issue

Comments

@peacekeeper
Copy link
Contributor

Right now, in the DID resolution metadata section we have listed an error code deactivated. Some people have argued that perhaps "deactivated" should not be considered an error. Instead, we could e.g. introduce a "deactivated" property with a boolean value in the DID document metadata.

@OR13 @csuwildcat @cihanss

@OR13
Copy link
Contributor

OR13 commented Nov 30, 2020

I agree it's certainly a property of did document metadata.

I think a boolean is probably correct, but worth asking if an enum would be better:

didDocumentState = ["active", "deactivated", ...] ?

If we forsee more complicated states in the future with different semantics, this may be a better solution... assuming a did document can only be in a single state.

@peacekeeper
Copy link
Contributor Author

peacekeeper commented Dec 1, 2020

I think a boolean is probably correct, but worth asking if an enum would be better:

I think I'd prefer a "deactivated" boolean property over a more generic state property, because then we would have to define somewhere what "DID document state" means, and what other values it could potentially have....

@peacekeeper peacekeeper self-assigned this Dec 9, 2020
@peacekeeper peacekeeper added the ready for pr Issue is ready for a PR label Dec 9, 2020
@jricher
Copy link
Contributor

jricher commented Dec 11, 2020

I can see the argument for a deactivated state not being an "error", but a property of the document itself -- but only in the case that the document itself is returned as well. If you're not returning the document, and the reason you're not returning the document is that it's been "deactivated", then that is an error and the nature of the current error code response. But if you're going to return the document anyway, and you want to flag something about the state of the document, then yes that's something for the DID Document Properties map.

The problem with either a boolean or a state enum is going to be the combination of all of these flags. What does it mean for a document to be "deactivated" but still returned, for example? Is that different from not returning a document for the same reason? I think the solution should follow what signals you want to send.

@OR13
Copy link
Contributor

OR13 commented Dec 11, 2020

AFAIK, everyone is always returning a DID Document even when its deactivated... would be good to know of anyone who is NOT returning a did document if the did is deactivated.

@peacekeeper
Copy link
Contributor Author

I thought about this a bit more, and I think the problem with returning a deactivated DID document is that you're not supposed to use a deactivated DID (document) anymore. Wouldn't there be a security risk if developers for some reason fail to check a "deactivated" flag and use the deactivated DID document in their application?

At an earlier point in time, we used to call the operation "Delete" instead of "Deactivate".

So perhaps NOT returning a deactivated DID document at all is actually the right thing to do...

@OR13 I think you raised this topic originally (e.g. w3c/did-extensions#151 (comment)), what are your current thoughts on this?

@OR13
Copy link
Contributor

OR13 commented Jan 5, 2021

@peacekeeper my original thoughts are:

  1. deactivation does not destroy the identifier, so the identifier's use (resolve) should not return an error.
  2. deactivation is meant to prevent future changes, so if authoritative keys for the did are in the did document, I would expect them to be removed when deactivation occurs (this is logically equivalent to transferring crypto to a null address / key pair you through away...
  3. should deactivated DIDs be allowed to verify credentials / capabilities? IMO NO, so I would expect all the verification methods in a deactivated DID to be empty

Given the spec text today, its highly likely that deactivate has been implemented very differently by a large number of ledgers, I would love text that at least said:

If a DID is deactivated a DID Document MUST be returned and its id property must be present.

If a DID is deactivated its verification relationship SHOULD be empty.

@peacekeeper
Copy link
Contributor Author

@OR13 I'd be fine with that too. I think I'd slightly prefer not returning a DID document, but returning one that's (almost?) empty also sounds reasonable to me.

@OR13
Copy link
Contributor

OR13 commented Jan 5, 2021

@peacekeeper the reason I suggest returning a document with only an id is that the type signature is the same... otherwise the type signature for resolve will be DIDDocument | undefined.

and everyone will be checking:

if (!didDocument){
// handle deactivation or error will be thrown
}

This way, the use of the did document will error in the same way for deactivation as removed keys, etc.... and if folks want to handle deactivation specifically they can check the metadata.

@OR13
Copy link
Contributor

OR13 commented Jan 5, 2021

here is an example of a did resolution response object:

{
  "didDocument": {
   "@context": [
      "https://www.w3.org/ns/did/v1"
    ],
    "id": "did:key:z6Mktrny5sALHZM4ejSuuHVt5TREAwVEKZpJ9h7DgWFSj6b7",
  },
  "didDocumentMetaData": {
    "content-type": "application/did+json",
     "deactivated": true

  },
  "didResolutionMetaData": {}
}

I would suggest this is what deactivate should be:

@peacekeeper
Copy link
Contributor Author

@OR13 I think in your example didDocumentMetadata and didResolutionMetadata should be swapped, or..?

@OR13
Copy link
Contributor

OR13 commented Jan 5, 2021

@peacekeeper I think you are right, I updated it

@msporny
Copy link
Member

msporny commented Jan 24, 2021

PR #543 addresses this issue, which will be closed once that PR is merged.

@csuwildcat
Copy link
Contributor

Being handled in PR #581

@msporny
Copy link
Member

msporny commented Jan 31, 2021

PR #581 addresses this issue, which will be closed once that PR is merged.

@msporny
Copy link
Member

msporny commented Feb 9, 2021

PR #581 has been merged, closing.

@msporny msporny closed this as completed Feb 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr exists There is an open PR to address this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants