Skip to content

Commit

Permalink
Changed contextId semantics to support "sub-contexts" (#684)
Browse files Browse the repository at this point in the history
This paves the way to support roles defined in "sub-context".

With this PR you can also query using any valid contextId, it will return you everything under that sub-context.
  • Loading branch information
thehenrytsai committed Feb 9, 2024
1 parent b4e0934 commit e762e0f
Show file tree
Hide file tree
Showing 22 changed files with 983 additions and 650 deletions.
28 changes: 25 additions & 3 deletions Q_AND_A.md
@@ -1,7 +1,7 @@

# DWN Q&A

## Message ID (`messageCid`) and Record ID (`recordId`)
## Message ID (`messageCid`), Record ID (`recordId`) and Context ID (`contextId`)

- Why can't/don't we use the message ID (`messageCid`) of the initial `RecordsWrite` as the record ID?

Expand All @@ -18,15 +18,37 @@

- Why is `recordId` required in a `RecordsWrite`?

(Last updated: 2023/05/16)
(Last updated: 2024/02/06)

This question should be further split into two:
1. Why is `recordId` required in an initial `RecordsWrite` (create)?
1. Why is `recordId` required in a subsequent `RecordsWrite` (update)?

The latter question is much easier to answer: an update needs to reference the record that it is updating.

The answer to the first-part question is more complicated: `recordId` technically is not needed in an initial `RecordsWrite`, but we chose to include it for data model consistency with subsequent `RecordsWrite`, such that we can simply return the latest message of a record as the response to `RecordsRead` and `RecordsQuery` (for the most part, we still remove `authorization`) without needing to re-inject/rehydrate `recordId` into any initial `RecordsWrite`. It is also the same reason why `contextId` is required for the initial `RecordsWrite` of a protocol-authorized record.
The answer to the first-part question is more complicated: `recordId` technically is not needed in an initial `RecordsWrite`, but we chose to include it for data model consistency with subsequent `RecordsWrite`, such that we can simply return the latest message of a record as the response to `RecordsRead` and `RecordsQuery` without needing to re-inject/rehydrate `recordId` into any initial `RecordsWrite`. It is also the same reason why `contextId` is required for the initial `RecordsWrite` of a protocol-authorized record.

- Why is `recordId` and `contextId` outside the `descriptor`.

- Because of the chicken-and-egg problem: `recordId` computation requires the `descriptor` as the input, so we cannot have `recordId` itself as part of the `descriptor`. `contextId` is similar in the sense that a record's `contextId` contains its own `recordId`, so it also cannot be inside the `descriptor`.

(Last updated: 2024/02/07)

- Why do we require `contextId` for protocol-based `RecordsWrite`? Can't it be inferred from `parentId` and `protocolPath`?

Yes, it can be inferred. But it is required for the same reason why `recordId` is required: for both implementation and developer convenience.

For example: for decryption, one would need to know the `contextId` to derive the decryption key at the right context, without this information readily available, the client would need to compute this value by walking up the ancestral chain themselves.

An alternative viable approach is to still not require it in `RecordsWrite` message and compute it internally, and return a constructed `contextId` as additional metadata along side of the `RecordsWrite` message when handling a query. This could incur cognitive load on the developers because they will likely need to pass the `contextId` in addition to the `RecordsWrite` message around instead of just passing `RecordsWrite` message. This would also mean we need to store this constructed `contextId` in the store as metadata (not just as index) so that we can return it as part of the a query (e.g. looking up the `contextId` of the parent). While this is a bigger change, open to feedback if this is indeed the preferred approach.

(Last update: 2024/02/07)

- Why does the `contextId` include the `recordId` of the record itself? Couldn't we adopt an alternative approach where the `contextId` is a path that ends at a record's parent?

Yes, we could opt to exclude the `recordId` of the record from the `contextId` of the record itself. However, this would complicate the process of querying for all records of a given context when the root record itself needs to be included. For instance, if we have a root "Thread" context record and we want to retrieve all the records of this Thread, including the root Thread record, the absence of a `contextId` containing its own `recordId` would necessitate a separate or more complex query to fetch the Thread record.

(Last update: 2024/02/07)


## Encryption
Expand Down
Expand Up @@ -11,7 +11,8 @@
"type": "string"
},
"contextId": {
"type": "string"
"type": "string",
"pattern": "^[a-zA-Z0-9]+(\/[a-zA-Z0-9]+)*$"
},
"attestation": {
"$ref": "https://identity.foundation/dwn/json-schemas/general-jws.json"
Expand Down
4 changes: 2 additions & 2 deletions src/core/dwn-error.ts
Expand Up @@ -62,6 +62,7 @@ export enum DwnErrorCode {
ProtocolAuthorizationDuplicateContextRoleRecipient = 'ProtocolAuthorizationDuplicateContextRoleRecipient',
ProtocolAuthorizationDuplicateGlobalRoleRecipient = 'ProtocolAuthorizationDuplicateGlobalRoleRecipient',
ProtocolAuthorizationIncorrectDataFormat = 'ProtocolAuthorizationIncorrectDataFormat',
ProtocolAuthorizationIncorrectContextId = 'ProtocolAuthorizationIncorrectContextId',
ProtocolAuthorizationIncorrectProtocolPath = 'ProtocolAuthorizationIncorrectProtocolPath',
ProtocolAuthorizationInvalidSchema = 'ProtocolAuthorizationInvalidSchema',
ProtocolAuthorizationInvalidType = 'ProtocolAuthorizationInvalidType',
Expand All @@ -70,7 +71,7 @@ export enum DwnErrorCode {
ProtocolAuthorizationMissingRuleSet = 'ProtocolAuthorizationMissingRuleSet',
ProtocolAuthorizationParentlessIncorrectProtocolPath = 'ProtocolAuthorizationParentlessIncorrectProtocolPath',
ProtocolAuthorizationNotARole = 'ProtocolAuthorizationNotARole',
ProtocolAuthorizationParentNotFound = 'ProtocolAuthorizationParentNotFound',
ProtocolAuthorizationParentNotFoundConstructingAncestorChain = 'ProtocolAuthorizationParentNotFoundConstructingAncestorChain',
ProtocolAuthorizationProtocolNotFound = 'ProtocolAuthorizationProtocolNotFound',
ProtocolAuthorizationQueryWithoutRole = 'ProtocolAuthorizationQueryWithoutRole',
ProtocolAuthorizationRoleMissingRecipient = 'ProtocolAuthorizationRoleMissingRecipient',
Expand Down Expand Up @@ -113,7 +114,6 @@ export enum DwnErrorCode {
RecordsWriteAttestationIntegrityInvalidPayloadProperty = 'RecordsWriteAttestationIntegrityInvalidPayloadProperty',
RecordsWriteAuthorizationFailed = 'RecordsWriteAuthorizationFailed',
RecordsWriteCreateMissingSigner = 'RecordsWriteCreateMissingSigner',
RecordsWriteCreateContextIdAndParentIdMutuallyInclusive = 'RecordsWriteCreateContextIdAndParentIdMutuallyInclusive',
RecordsWriteCreateDataAndDataCidMutuallyExclusive = 'RecordsWriteCreateDataAndDataCidMutuallyExclusive',
RecordsWriteCreateDataCidAndDataSizeMutuallyInclusive = 'RecordsWriteCreateDataCidAndDataSizeMutuallyInclusive',
RecordsWriteCreateProtocolAndProtocolPathMutuallyInclusive = 'RecordsWriteCreateProtocolAndProtocolPathMutuallyInclusive',
Expand Down
109 changes: 74 additions & 35 deletions src/core/protocol-authorization.ts
Expand Up @@ -7,6 +7,8 @@ import type { RecordsSubscribe } from '../interfaces/records-subscribe.js';
import type { RecordsWriteMessage } from '../types/records-types.js';
import type { ProtocolActionRule, ProtocolDefinition, ProtocolRuleSet, ProtocolsConfigureMessage, ProtocolType, ProtocolTypes } from '../types/protocols-types.js';

import { FilterUtility } from '../utils/filter.js';
import { Records } from '../utils/records.js';
import { RecordsWrite } from '../interfaces/records-write.js';
import { DwnError, DwnErrorCode } from './dwn-error.js';
import { DwnInterfaceName, DwnMethodName } from '../enums/dwn-interface-method.js';
Expand Down Expand Up @@ -37,7 +39,7 @@ export class ProtocolAuthorization {
);

// validate `protocolPath`
await ProtocolAuthorization.verifyProtocolPath(
await ProtocolAuthorization.verifyProtocolPathAndContextId(
tenant,
incomingMessage,
messageStore,
Expand All @@ -49,8 +51,8 @@ export class ProtocolAuthorization {
protocolDefinition,
);

// If the incoming message is writing a $globalRole record, validate that the recipient is unique
await ProtocolAuthorization.verifyUniqueRoleRecipient(
// Validate as a role record if the incoming message is writing a role record
await ProtocolAuthorization.verifyAsRoleRecordIfNeeded(
tenant,
incomingMessage,
inboundMessageRuleSet,
Expand Down Expand Up @@ -283,7 +285,6 @@ export class ProtocolAuthorization {
}

const protocol = newestRecordsWrite.message.descriptor.protocol!;
const contextId = newestRecordsWrite.message.contextId!;

// keep walking up the chain from the inbound message's parent, until there is no more parent
let currentParentId = newestRecordsWrite.message.descriptor.parentId;
Expand All @@ -293,15 +294,18 @@ export class ProtocolAuthorization {
interface : DwnInterfaceName.Records,
method : DwnMethodName.Write,
protocol,
contextId,
recordId : currentParentId
};
const { messages: parentMessages } = await messageStore.query(tenant, [query]);

// We already check the immediate parent in `verifyProtocolPath`, so if it triggers,
// it means a bug that caused an invalid message to be saved to the DWN.
// We already check the immediate parent in `verifyProtocolPathAndContextId` at the time of writing, so if this condition is triggered,
// it means there is an unexpected bug that caused an invalid message being saved to the DWN.
// We add additional defensive check here because returning an unexpected/incorrect ancestor chain could lead to security vulnerabilities.
if (parentMessages.length === 0) {
throw new DwnError(DwnErrorCode.ProtocolAuthorizationParentNotFound, `no parent found with ID ${currentParentId}`);
throw new DwnError(
DwnErrorCode.ProtocolAuthorizationParentNotFoundConstructingAncestorChain,
`Unexpected error that should never trigger: no parent found with ID ${currentParentId} when constructing ancestor message chain.`
);
}

const parent = parentMessages[0] as RecordsWriteMessage;
Expand All @@ -314,7 +318,7 @@ export class ProtocolAuthorization {
}

/**
* Gets the rule set corresponding to the given message chain.
* Gets the rule set corresponding to the given protocolPath.
*/
private static getRuleSet(
protocolPath: string,
Expand All @@ -332,7 +336,7 @@ export class ProtocolAuthorization {
* Verifies the `protocolPath` declared in the given message (if it is a RecordsWrite) matches the path of actual ancestor chain.
* @throws {DwnError} if fails verification.
*/
private static async verifyProtocolPath(
private static async verifyProtocolPathAndContextId(
tenant: string,
inboundMessage: RecordsWrite,
messageStore: MessageStore
Expand All @@ -348,26 +352,43 @@ export class ProtocolAuthorization {
`Declared protocol path '${declaredProtocolPath}' is not valid for records with no parentId'.`
);
}
} else {
const protocol = inboundMessage.message.descriptor.protocol!;
const contextId = inboundMessage.message.contextId!;
const query: Filter = {
interface : DwnInterfaceName.Records,
method : DwnMethodName.Write,
protocol,
contextId,
recordId : parentId
};
const { messages: parentMessages } = await messageStore.query(tenant, [query]);
const parentProtocolPath = (parentMessages as RecordsWriteMessage[])[0]?.descriptor?.protocolPath;
const actualProtocolPath = `${parentProtocolPath}/${declaredTypeName}`;
if (parentProtocolPath === undefined || actualProtocolPath !== declaredProtocolPath) {
throw new DwnError(
DwnErrorCode.ProtocolAuthorizationIncorrectProtocolPath,
`Could not find matching parent record to verify declared protocol path '${declaredProtocolPath}'.`
);
}
return;
}

// Else `parentId` is defined, so we need to verify both protocolPath and contextId

// fetch the parent message
const protocol = inboundMessage.message.descriptor.protocol!;
const query: Filter = {
isLatestBaseState : true, // NOTE: this filter is critical, to ensure are are not returning a deleted parent
interface : DwnInterfaceName.Records,
method : DwnMethodName.Write,
protocol,
recordId : parentId
};
const { messages: parentMessages } = await messageStore.query(tenant, [query]);
const parentMessage = (parentMessages as RecordsWriteMessage[])[0];

// verifying protocolPath of incoming message is a child of the parent message's protocolPath
const parentProtocolPath = parentMessage?.descriptor?.protocolPath;
const expectedProtocolPath = `${parentProtocolPath}/${declaredTypeName}`;
if (expectedProtocolPath !== declaredProtocolPath) {
throw new DwnError(
DwnErrorCode.ProtocolAuthorizationIncorrectProtocolPath,
`Could not find matching parent record to verify declared protocol path '${declaredProtocolPath}'.`
);
}

// verifying contextId of incoming message is a child of the parent message's contextId
const expectedContextId = `${parentMessage.contextId}/${inboundMessage.message.recordId}`;
const actualContextId = inboundMessage.message.contextId;
if (actualContextId !== expectedContextId) {
throw new DwnError(
DwnErrorCode.ProtocolAuthorizationIncorrectContextId,
`Declared contextId '${actualContextId}' is not the same as expected: '${expectedContextId}'.`
);
}

}

/**
Expand Down Expand Up @@ -456,7 +477,17 @@ export class ProtocolAuthorization {
'Could not verify $contextRole because contextId is missing'
);
}
roleRecordFilter.contextId = contextId;

// Compute `contextId` prefix filter for fetching the invoked role record.
// e.g. if invoked role path is `Thread/Participant`, and the `contextId` of the message is `threadX/messageY/attachmentZ`,
// then we need to add a prefix filter as `threadX` for the `contextId`
// because the `contextId` of the Participant record would be in the form of be `threadX/participantA`
const ancestorSegmentCountOfRole = protocolRole.split('/').length - 1;
const contextIdSegments = contextId.split('/');
const contextIdPrefix = contextIdSegments.slice(0, ancestorSegmentCountOfRole).join('/');
const contextIdPrefixFilter = FilterUtility.constructPrefixFilterAsRangeFilter(contextIdPrefix);

roleRecordFilter.contextId = contextIdPrefixFilter;
}

const { messages: matchingMessages } = await messageStore.query(tenant, [roleRecordFilter]);
Expand Down Expand Up @@ -560,6 +591,7 @@ export class ProtocolAuthorization {
// else the incoming message must be a RecordsDelete because only `update` and `delete` are allowed recipient actions
recordsWriteMessage = ancestorMessageChain[ancestorMessageChain.length - 1];
}

if (recordsWriteMessage.descriptor.recipient === author) {
return;
}
Expand All @@ -580,27 +612,30 @@ export class ProtocolAuthorization {
}

/**
* Verifies that writes to a $globalRole or $contextRole record do not have the same recipient as an existing RecordsWrite
* to the same $globalRole or the same $contextRole in the same context.
* If the given RecordsWrite is not a role record, this method does nothing and succeeds immediately.
*
* Else it verifies the validity of the given `RecordsWrite` including:
* 1. The same role has not been assigned to the same entity/recipient.
*/
private static async verifyUniqueRoleRecipient(
private static async verifyAsRoleRecordIfNeeded(
tenant: string,
incomingMessage: RecordsWrite,
inboundMessageRuleSet: ProtocolRuleSet,
messageStore: MessageStore,
): Promise<void> {
const incomingRecordsWrite = incomingMessage;
if (!inboundMessageRuleSet.$globalRole && !inboundMessageRuleSet.$contextRole) {
return;
}

const incomingRecordsWrite = incomingMessage;
const recipient = incomingRecordsWrite.message.descriptor.recipient;
if (recipient === undefined) {
throw new DwnError(
DwnErrorCode.ProtocolAuthorizationRoleMissingRecipient,
'Role records must have a recipient'
);
}

const protocolPath = incomingRecordsWrite.message.descriptor.protocolPath!;
const filter: Filter = {
interface : DwnInterfaceName.Records,
Expand All @@ -610,9 +645,13 @@ export class ProtocolAuthorization {
protocolPath,
recipient,
};

if (inboundMessageRuleSet.$contextRole) {
filter.contextId = incomingRecordsWrite.message.contextId!;
const parentContextId = Records.getParentContextFromOfContextId(incomingRecordsWrite.message.contextId)!;
const prefixFilter = FilterUtility.constructPrefixFilterAsRangeFilter(parentContextId);
filter.contextId = prefixFilter;
}

const { messages: matchingMessages } = await messageStore.query(tenant, [filter]);
const matchingRecords = matchingMessages as RecordsWriteMessage[];
const matchingRecordsExceptIncomingRecordId = matchingRecords.filter((recordsWriteMessage) =>
Expand Down
14 changes: 8 additions & 6 deletions src/core/records-grant-authorization.ts
Expand Up @@ -174,12 +174,14 @@ export class RecordsGrantAuthorization {
);
}

// If grant specifies either contextId, check that record is that context
if (grantScope.contextId !== undefined && grantScope.contextId !== recordsWriteMessage.contextId) {
throw new DwnError(
DwnErrorCode.RecordsGrantAuthorizationScopeContextIdMismatch,
`Grant scope specifies different contextId than what appears in the record`
);
// If grant specifies a contextId, check that record falls under that contextId
if (grantScope.contextId !== undefined) {
if (recordsWriteMessage.contextId === undefined || !recordsWriteMessage.contextId.startsWith(grantScope.contextId)) {
throw new DwnError(
DwnErrorCode.RecordsGrantAuthorizationScopeContextIdMismatch,
`Grant scope specifies different contextId than what appears in the record`
);
}
}

// If grant specifies protocolPath, check that record is at that protocolPath
Expand Down

0 comments on commit e762e0f

Please sign in to comment.