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.

Sign up for GitHub

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

Jump to bottom

chore(ref-imp): remove controller and add context to deactivated docu… #1006

Merged
merged 1 commit into from
Jan 20, 2021
Merged

chore(ref-imp): remove controller and add context to deactivated docu… #1006

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions lib/core/versions/latest/DocumentComposer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ import SidetreeError from '../../../common/SidetreeError';
*/
export default class DocumentComposer {

private static resolutionObjectContextUrl = 'https://w3id.org/did-resolution/v1';
private static didDocumentContextUrl = 'https://www.w3.org/ns/did/v1';

/**
* Transforms the given DID state into a DID Document.
*/
Expand All @@ -37,7 +40,6 @@ export default class DocumentComposer {
const id = '#' + publicKey.id;
const didDocumentPublicKey = {
id: id,
controller: '',
type: publicKey.type,
publicKeyJwk: publicKey.publicKeyJwk
};
Expand All @@ -48,7 +50,7 @@ export default class DocumentComposer {
verificationMethod.push(didDocumentPublicKey);

if (purposeSet.size > 0) {
const reference = didDocumentPublicKey.controller + didDocumentPublicKey.id;
const reference = didDocumentPublicKey.id;

for (const purpose of purposeSet) {
if (!verificationRelationships.has(purpose)) {
Expand Down Expand Up @@ -79,7 +81,7 @@ export default class DocumentComposer {
const baseId = did.isShortForm ? did.shortForm : did.longForm;
const didDocument: any = {
id: baseId,
'@context': ['https://www.w3.org/ns/did/v1', { '@base': baseId }],
'@context': [DocumentComposer.didDocumentContextUrl, { '@base': baseId }],
service: services
};

Expand All @@ -92,7 +94,7 @@ export default class DocumentComposer {
});

const didResolutionResult: any = {
'@context': 'https://w3id.org/did-resolution/v1',
'@context': DocumentComposer.resolutionObjectContextUrl,
didDocument: didDocument,
didDocumentMetadata: {
method: {
Expand All @@ -119,7 +121,7 @@ export default class DocumentComposer {
private static createDeactivatedResolutionResult (did: string, published: boolean) {
const didDocument = {
id: did,
'@context': ['https://www.w3.org/ns/did/v1', { '@base': did }]
'@context': [DocumentComposer.didDocumentContextUrl, { '@base': did }]
};
const didDocumentMetadata = {
method: {
Expand All @@ -128,6 +130,7 @@ export default class DocumentComposer {
canonicalId: did
};
return {
'@context': DocumentComposer.resolutionObjectContextUrl,
didDocument,
didDocumentMetadata
};
Expand Down
3 changes: 0 additions & 3 deletions tests/core/DocumentComposer.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,19 +47,16 @@ describe('DocumentComposer', async () => {
verificationMethod: [
{
id: '#anySigningKey',
controller: '',
type: 'EcdsaSecp256k1VerificationKey2019',
publicKeyJwk: { kty: 'EC', crv: 'secp256k1', x: anySigningPublicKey.publicKeyJwk.x, y: anySigningPublicKey.publicKeyJwk.y }
},
{
id: '#authPublicKey',
controller: '',
type: 'EcdsaSecp256k1VerificationKey2019',
publicKeyJwk: { kty: 'EC', crv: 'secp256k1', x: authPublicKey.publicKeyJwk.x, y: authPublicKey.publicKeyJwk.y }
},
{
id: '#noPurposePublicKey',
controller: '',
type: 'EcdsaSecp256k1VerificationKey2019',
publicKeyJwk: { kty: 'EC', crv: 'secp256k1', x: noPurposePublicKey.publicKeyJwk.x, y: noPurposePublicKey.publicKeyJwk.y }
}
Expand Down
1 change: 0 additions & 1 deletion tests/vectors/resolution/afterCreate.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
"verificationMethod": [
{
"id": "#publicKeyModel1Id",
"controller": "",
"type": "EcdsaSecp256k1VerificationKey2019",
"publicKeyJwk": {
"kty": "EC",
Expand Down
1 change: 1 addition & 0 deletions tests/vectors/resolution/afterDeactivate.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{
"@context": "https://w3id.org/did-resolution/v1",
"didDocument": {
"id": "did:sidetree:EiDyOQbbZAa3aiRzeCkV7LOx3SERjjH93EXoIM3UoN4oWg",
"@context": [
Expand Down
1 change: 0 additions & 1 deletion tests/vectors/resolution/afterRecover.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
"verificationMethod": [
{
"id": "#newKey",
"controller": "",
"type": "EcdsaSecp256k1VerificationKey2019",
"publicKeyJwk": {
"kty": "EC",
Expand Down
2 changes: 0 additions & 2 deletions tests/vectors/resolution/afterUpdate.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
"verificationMethod": [
{
"id": "#publicKeyModel1Id",
"controller": "",
Copy link
Contributor

@OR13 OR13 Jan 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

please read did core before making changes to test vectors.... https://w3c.github.io/did-core/#verification-methods

The properties MUST include the id, type, controller, and specific verification method properties, and MAY include additional properties.

I ideally you would also have tests that showed that JSON-LD was valid.

Copy link
Collaborator

@sandrask sandrask Jan 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@OR13 This change is my fault. I read the spec but I looked at wrong section (controller at top level of the document). Sorry about this. Can the controller be empty though?

Copy link
Contributor

Choose a reason for hiding this comment

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

the controller should equal didDocument.id in order for verifiable credentials to be valid.... see: https://github.com/OR13/did-params-and-you/blob/master/examples/case-0-legal-working.json#L35

and our version of sidetree: https://github.com/transmute-industries/sidetree.js/blob/main/packages/core/src/DocumentComposer.ts#L65

If his repo had CI tests that used the result of test vectors, with verifiable credentials, it would be obvious when breaking changes like this were introduced.

btw, the reason that "" is legal here, is because of @base / relative IRIs in sidetree did documents....

You can check the nquads to see that its the same...

if the iri is blank, it will get mapped to the didDocument.id... making what we had before legal (but only for relative ref based did documents).

Copy link
Collaborator

Choose a reason for hiding this comment

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

So it is legal because it will be populated when processing @base - thank you for this clarification.

Copy link
Contributor

Choose a reason for hiding this comment

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

I strongly recommend against returning the empty string for the controller property.... if makes sidetree look different than every other did method, and its not guaranteed to work accross JSON-LD implementations... the controller property should match the didDocument.id.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching it. I will have a PR out fixing this soon.

"type": "EcdsaSecp256k1VerificationKey2019",
"publicKeyJwk": {
"kty": "EC",
Expand All @@ -29,7 +28,6 @@
},
{
"id": "#additional-key",
"controller": "",
"type": "EcdsaSecp256k1VerificationKey2019",
"publicKeyJwk": {
"kty": "EC",
Expand Down
1 change: 0 additions & 1 deletion tests/vectors/resolution/longFormResponseDidDocument.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
"verificationMethod": [
{
"id": "#publicKeyModel1Id",
"controller": "",
"type": "EcdsaSecp256k1VerificationKey2019",
"publicKeyJwk": {
"crv": "secp256k1",
Expand Down