feature/SDK-56_oauth-status-list #302

sanderPostma · 2025-01-17T09:04:19Z

Unfortunately this is became a big PR.
The data store also needed work to accommodate multiple status list types and tests were missing for it altogether in the data-store.

…th-status-list

nklomp

See remarks. Migration comment is the biggest issue

nklomp · 2025-01-17T10:32:34Z

packages/data-store/src/migrations/postgres/1693866470001-CreateStatusList.ts

    await queryRunner.query(
-      `CREATE TABLE "StatusListEntry" ("statusListId" character varying NOT NULL, "statusListIndex" integer NOT NULL, "credentialId" character varying, "credentialHash" character varying(128), "correlationId" character varying(255), "value" character varying(50), CONSTRAINT "PK_68704d2d13857360c6b44a3d1d0" PRIMARY KEY ("statusListId", "statusListIndex"))`,


Please provide an actual migration instead of updating the existing migration file. We have a customer in production that is using StatusLists

nklomp · 2025-01-17T10:45:32Z

packages/ssi-types/src/types/status-list.ts

+
+export type StatusListCredential = W3CVerifiableCredential | CWT
+
+export type ProofFormat = VmoProofFormat | 'cbor'


Please just use the word Veramo in here

nklomp · 2025-01-17T10:47:39Z

packages/data-store/src/utils/statusList/MappingUtils.ts

+  throw new Error(`Invalid status list type ${typeof entity}`)
+}
+
+function setBaseFields(entity: StatusListEntity, args: IStatusListEntity) {


Mixing of arrow functions with regular functions in the same file

nklomp · 2025-01-17T10:47:46Z

packages/data-store/src/utils/statusList/MappingUtils.ts

+  entity.statusListCredential = args.statusListCredential
+}
+
+function getBaseFields(entity: StatusListEntity): Omit<IStatusListEntity, 'type'> {


Mixing of arrow functions with regular functions in the same file

nklomp · 2025-01-17T10:49:33Z

packages/vc-status-list-issuer-drivers/src/drivers.ts

-    }
-    this._statusListLength = details.length
+
+    // (StatusListStore does the duplicate entity check)


Yes with a less descriptive error

Oh?
in develop we have

if (entity) { throw Error(`Status list ${details.id}, correlationId ${args.correlationId} already exists`) }

In StatusListStore we have

if (result) { throw Error(`Status list for id ${id}, correlationId ${correlationId} already exists`) }

nklomp · 2025-01-17T11:03:11Z

packages/vc-status-list/src/impl/OAuthStatusList.ts

+export const DEFAULT_LIST_LENGTH = 250000
+export const DEFAULT_PROOF_FORMAT = 'jwt' as ProofFormat
+export const STATUS_LIST_JWT_HEADER: StatusListJWTHeaderParameters = {
+  alg: 'EdDSA',


Why is this fixed? If this is a default object, make it clear in the name

removed
That came from the sd-jwt library test, should not have been there anymore.
Alg is comming from

getSigningAlgo(resolution.key.type)

nklomp · 2025-01-17T11:03:52Z

packages/vc-status-list/src/impl/OAuthStatusList.ts

+
+type IRequiredContext = IAgentContext<ICredentialPlugin & IJwtService & IIdentifierResolution>
+
+export const BITS_PER_STATUS_DEFAULT = 2 // 2 bits are sufficient for 0x00 - "VALID"  0x01 - "INVALID" & 0x02 - "SUSPENDED"


What's with the inconsistencies of the word DEFAULT?

fixed (happened when using ctrl-alt-c in different sessions)

nklomp · 2025-01-17T11:06:03Z

packages/vc-status-list/src/impl/StatusListFactory.ts

+    return StatusListFactory.instance
+  }
+
+  public getImplementation(type: StatusListType): IStatusList {


nklomp · 2025-01-17T11:08:55Z

packages/vc-status-list/src/impl/encoding/cbor.ts

+  statusListMap: com.sphereon.cbor.CborMap<com.sphereon.cbor.CborString, com.sphereon.cbor.CborItem<any>>,
+) {
+  const exp = Math.floor(new Date().getTime() / 1000)
+  const ttl = 65535 // FIXME figure out what value should be / come from and what the difference is with exp


Please figure out what the difference is. Otherwise create a ticket and link it here

@nklomp

ttl: OPTIONAL. The ttl (time to live) claim, if present, MUST
specify the maximum amount of time, in seconds, that the Status
List Token can be cached by a consumer before a fresh copy SHOULD
be retrieved. The value of the claim MUST be a positive number
encoded in JSON as a number.

Do we want to use / implement this now?

nklomp · 2025-01-17T11:10:30Z

packages/vc-status-list/src/utils.ts

+  } else if (CredentialMapper.isMsoMdocOid4VPEncoded(credential)) {
+    // Just assume Cbor for now, I'd need to decode at least the header to what type of Cbor we have
+    return 'cbor'
+  } else if (CredentialMapper.isCredential(credential)) {


This is dangerous. It now assumes everything else is json-ld

refactored using detectDocumentType

… expiresAt handling

…th-status-list

…th-status-list # Conflicts: # packages/ssi-types/src/types/index.ts

ronal2do · 2025-01-30T13:27:27Z

packages/data-store/src/__tests__/statusList.entities.test.ts

+    expect(fromDb).toBeDefined()
+    expect(fromDb.issuer).toEqual(statusList.issuer)
+    expect(typeof fromDb.issuer).toEqual('object')
+    expect((fromDb.issuer as any).id).toEqual('did:example:123')


"any" alternative here?

ronal2do · 2025-01-30T13:29:22Z

packages/data-store/src/statusList/StatusListStore.ts

@@ -158,11 +144,37 @@ export class StatusListStore implements IStatusListStore {
    return !error
  }

+  async removeStatusListEntryByIndex(args: IGetStatusListEntryByIndexArgs): Promise<boolean> {


Suggested change

async removeStatusListEntryByIndex(args: IGetStatusListEntryByIndexArgs): Promise<boolean> {

async removeStatusListEntryByIndex(args: IGetStatusListEntryByIndexArgs, error: boolean = false): Promise<boolean> {

than don't need to add a let on the body

sanderPostma added 19 commits January 10, 2025 14:31

chore: prepare interface for OAuth2StatusList args

64fd2a5

chore: prepare interface for OAuth2StatusList args

f928c22

chore: prepare interface for OAuth2StatusList args

0e3b6e4

chore: prepare interface for OAuth2StatusList args

321e6e8

chore: implementing OAuthStatusList

b11f791

chore: implementing OAuthStatusList

7e70318

chore: saving work

ca08072

chore: saving work

227bf10

chore: tests passing

10b3d15

chore: saving CBOR work

0fa3d5f

chore: CBOR test passing

d7cb05c

chore: datastore test passing

bff9ebc

chore: fixed imports & tsconfigs

4798cc0

chore: fixed issue-verify-flow-statuslist.test

2ee21ef

chore: small refactor

2d7644b

chore: fixed more tests

79cab93

chore: self-review

59610f1

Merge remote-tracking branch 'origin/develop' into feature/SDK-56_oau…

e6e3476

…th-status-list

chore: lockfile

1f3364c

sanderPostma marked this pull request as ready for review January 17, 2025 10:21

chore: split out migrations to -UpdateStatusList.ts

6d68df1

nklomp requested changes Jan 17, 2025

View reviewed changes

sanderPostma added 6 commits January 25, 2025 13:04

chore: PR feedback

6ac08a7

chore: Added test coverage to statusListCredentialToDetails and fixed…

0fb7aec

… expiresAt handling

chore: fixed expiresAt type for sqlite

48c95e1

Merge remote-tracking branch 'origin/develop' into feature/SDK-56_oau…

0829f7a

…th-status-list

Merge remote-tracking branch 'origin/develop' into feature/SDK-56_oau…

eea06ce

…th-status-list # Conflicts: # packages/ssi-types/src/types/index.ts

chore: fixed expiresAt in data store test

be93b0b

ronal2do reviewed Jan 30, 2025

View reviewed changes

ronal2do approved these changes Jan 30, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature/SDK-56_oauth-status-list #302

feature/SDK-56_oauth-status-list #302

sanderPostma commented Jan 17, 2025 •

edited

Loading

nklomp left a comment

nklomp Jan 17, 2025

sanderPostma Jan 25, 2025

nklomp Jan 17, 2025

sanderPostma Jan 25, 2025

nklomp Jan 17, 2025

sanderPostma Jan 25, 2025

nklomp Jan 17, 2025

sanderPostma Jan 25, 2025

nklomp Jan 17, 2025

sanderPostma Jan 25, 2025

nklomp Jan 17, 2025

sanderPostma Jan 25, 2025 •

edited

Loading

nklomp Jan 17, 2025

sanderPostma Jan 25, 2025 •

edited

Loading

nklomp Jan 17, 2025

sanderPostma Jan 25, 2025

nklomp Jan 17, 2025

sanderPostma Jan 25, 2025

nklomp Jan 17, 2025

sanderPostma Jan 25, 2025

ronal2do Jan 30, 2025

ronal2do Jan 30, 2025

		await queryRunner.query(
		`CREATE TABLE "StatusListEntry" ("statusListId" character varying NOT NULL, "statusListIndex" integer NOT NULL, "credentialId" character varying, "credentialHash" character varying(128), "correlationId" character varying(255), "value" character varying(50), CONSTRAINT "PK_68704d2d13857360c6b44a3d1d0" PRIMARY KEY ("statusListId", "statusListIndex"))`,


		export type StatusListCredential = W3CVerifiableCredential \| CWT

		export type ProofFormat = VmoProofFormat \| 'cbor'


		type IRequiredContext = IAgentContext<ICredentialPlugin & IJwtService & IIdentifierResolution>

		export const BITS_PER_STATUS_DEFAULT = 2 // 2 bits are sufficient for 0x00 - "VALID" 0x01 - "INVALID" & 0x02 - "SUSPENDED"

	async removeStatusListEntryByIndex(args: IGetStatusListEntryByIndexArgs): Promise<boolean> {
	async removeStatusListEntryByIndex(args: IGetStatusListEntryByIndexArgs, error: boolean = false): Promise<boolean> {

feature/SDK-56_oauth-status-list #302

Are you sure you want to change the base?

feature/SDK-56_oauth-status-list #302

Conversation

sanderPostma commented Jan 17, 2025 • edited Loading

nklomp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sanderPostma Jan 25, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sanderPostma Jan 25, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sanderPostma commented Jan 17, 2025 •

edited

Loading

sanderPostma Jan 25, 2025 •

edited

Loading

sanderPostma Jan 25, 2025 •

edited

Loading