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

feat: nested ctype val #919

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open
Changes from 4 commits
Commits
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
100 changes: 96 additions & 4 deletions packages/credentials/src/V1/KiltCredentialV1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,15 +327,77 @@ export function fromInput({

const cachingCTypeLoader = newCachingCTypeLoader()

// Check recursively if a value has references
const hasRef = (value: unknown): boolean => {
if (typeof value !== 'object' || value === null) {
return false
}

if ('$ref' in (value as Record<string, unknown>)) {
return true
}

if (Array.isArray(value)) {
return value.some((item) => hasRef(item))
}

return Object.values(value as Record<string, unknown>).some((v) => hasRef(v))
}
Copy link
Contributor

Choose a reason for hiding this comment

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

you're not using this function any more, are you?

Copy link
Author

Choose a reason for hiding this comment

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

Yes forget to remove, fixed at: bbfced2


// Single function to both check for references and extract them
function extractUniqueReferences(
cType: ICType,
references: Set<string> = new Set<string>()
): Set<string> {
if (typeof cType?.properties !== 'object' || cType.properties === null) {
return references
}

const processValue = (value: unknown): void => {
if (typeof value !== 'object' || value === null) {
return
}
const objValue = value as Record<string, unknown>
if ('$ref' in objValue) {
const ref = objValue.$ref as string
if (ref.startsWith('kilt:ctype:')) {
references.add(ref.split('#/')[0])
}
}

if (Array.isArray(value)) {
value.forEach(processValue)
} else {
Object.values(objValue).forEach(processValue)
Copy link
Contributor

Choose a reason for hiding this comment

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

the point of passing down the set of references to extractUniqueReferences was that you can call extractUniqueReferences in recursion instead of an inner function. You don't necessarily need to do it that way, but if you're not using this parameter, you can also just pull it back into the function as a normal variable.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed at: bbfced2

}
}

processValue(cType.properties)
return references
}

/**
* Validates the claims in the VC's `credentialSubject` against a CType definition.
* Supports both nested and non-nested CType validation.
* For non-nested CTypes:
* - Validates claims directly against the CType schema.
* For nested CTypes:
* - Automatically detects nested structure through $ref properties.
* - Fetches referenced CTypes from the blockchain.
* - Performs validation against the main CType and all referenced CTypes.
*
* @param credential A {@link KiltCredentialV1} type verifiable credential.
* @param credential.credentialSubject The credentialSubject to be validated.
* @param credential.type The credential's types.
* @param options Options map.
* @param options.cTypes One or more CType definitions to be used for validation. If `loadCTypes` is set to `false`, validation will fail if the definition of the credential's CType is not given.
* @param options.loadCTypes A function to load CType definitions that are not in `cTypes`. Defaults to using the {@link newCachingCTypeLoader | CachingCTypeLoader}. If set to `false` or `undefined`, no additional CTypes will be loaded.
* @param options.cTypes One or more CType definitions to be used for validation. If loadCTypes is set to false, validation will fail if the definition of the credential's CType is not given.
* @param options.loadCTypes A function to load CType definitions that are not in cTypes. Defaults to using the {@link newCachingCTypeLoader | CachingCTypeLoader}. If set to false or undefined, no additional CTypes will be loaded.
*
* @throws {Error} If the credential type does not contain a valid CType id.
* @throws {Error} If required CType definitions cannot be loaded.
* @throws {Error} If claims do not follow the expected CType format.
* @throws {Error} If referenced CTypes in nested structure cannot be fetched from the blockchain.
* @throws {Error} If validation fails against the CType schema.
*/
export async function validateSubject(
{
Expand All @@ -354,6 +416,7 @@ export async function validateSubject(
if (!credentialsCTypeId) {
throw new Error('credential type does not contain a valid CType id')
}

// check that we have access to the right schema
let cType = cTypes?.find(({ $id }) => $id === credentialsCTypeId)
if (!cType) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to somehow handle both the ctypes passed in as the cTypes argument, and the loadCtypes function, if there is any. I suggest we create a combined ctype loader function from both, which first checks if the cType whose id is being looked up is contained in the cTypes array, and if not, forwards the id to the loadCtypes function, which then tries to look it up.

Hint: checking if we have a CType already in cTypes can be done quicker if we transform the array into a Map of ctype id to ctype definition once.

Copy link
Author

Choose a reason for hiding this comment

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

I am working on a new commit for this.

Copy link
Contributor

Choose a reason for hiding this comment

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

this is still tbd

Expand Down Expand Up @@ -385,6 +448,35 @@ export async function validateSubject(
[key.substring(vocab.length)]: value,
}
}, {})
// validates against CType (also validates CType schema itself)
CType.verifyClaimAgainstSchema(claims, cType)

try {
const references = extractUniqueReferences(cType)

const referencedCTypes = await Promise.all(
Array.from(references).map(async (ref) => {
try {
const referencedCType = await cachingCTypeLoader(ref as any)
Copy link
Contributor

Choose a reason for hiding this comment

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

please use the function passed as the loadCtypes argument

Copy link
Author

Choose a reason for hiding this comment

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

Fixed at: bbfced2

return referencedCType
} catch (error) {
// eslint-disable-next-line no-console
console.error(`Failed to fetch CType for reference ${ref}:`, error)
Copy link
Contributor

Choose a reason for hiding this comment

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

we don't use console statements in library code, that's why the lint rule exists

Copy link
Author

Choose a reason for hiding this comment

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

Fixed at: bbfced2

throw error
}
})
)

const validCTypes = referencedCTypes.filter(
(ctype): ctype is ICType => ctype !== undefined && ctype !== null
)

if (validCTypes.length === references.size) {
await CType.verifyClaimAgainstNestedSchemas(cType, validCTypes, claims)
} else {
throw new Error('Some referenced CTypes could not be fetched')
}
} catch (error) {
// eslint-disable-next-line no-console
console.error('Validation error:', error)
Copy link
Contributor

Choose a reason for hiding this comment

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

we don't use console statements in library code, that's why the lint rule exists

Copy link
Author

Choose a reason for hiding this comment

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

Fixed at: bbfced2

throw error
}
}
Loading