CredentialOfferSession notification_id structure

[auer-martin]

Currently, credential offer sessions contain a notification_id property.
As multiple credentials can be offered and requested during a credential offer session, there should/may be multiple notification IDs.

export interface CredentialOfferSession extends StateType {
  clientId?: string;
  credentialOffer: AssertedUniformCredentialOffer;
  credentialDataSupplierInput?: CredentialDataSupplierInput; // Optional storage that can help the credential Data Supplier. For instance to store credential input data during offer creation, if no additional data can be supplied later on
  txCode?: string; // in here we only store the txCode, previously < V13 this was the userPin. We map the userPin onto this value
  status: IssueStatus;
  error?: string;
  lastUpdatedAt: number;
  notification_id: string;
  issuerState?: string; //todo: Probably good to hash it here, since it would come in from the client and we could match the hash and thus use the client value
  preAuthorizedCode?: string; //todo: Probably good to hash it here, since it would come in from the client and we could match the hash and thus use the client value
}

Therefore, I think we need to restructure this so that multiple credential_offer id properties can exist and be correlated to the particular credential being offered.

[cre8]

@auer-martin what about the notification_id in the CredentialResponse:

export interface CredentialResponse extends ExperimentalSubjectIssuance {
  credential?: W3CVerifiableCredential; // OPTIONAL. Contains issued Credential. MUST be present when acceptance_token is not returned. MAY be a JSON string or a JSON object, depending on the Credential format. See Appendix E for the Credential format specific encoding requirements
  format?: OID4VCICredentialFormat /* | OID4VCICredentialFormat[]*/; // REQUIRED. JSON string denoting the format of the issued Credential  TODO: remove when cleaning <v13
  transaction_id?: string; //OPTIONAL. A string identifying a Deferred Issuance transaction. This claim is contained in the response if the Credential Issuer was unable to immediately issue the credential. The value is subsequently used to obtain the respective Credential with the Deferred Credential Endpoint (see Section 9). It MUST be present when the credential parameter is not returned. It MUST be invalidated after the credential for which it was meant has been obtained by the Wallet.
  acceptance_token?: string; //deprecated // OPTIONAL. A JSON string containing a security token subsequently used to obtain a Credential. MUST be present when credential is not returned
  c_nonce?: string; // OPTIONAL. JSON string containing a nonce to be used to create a proof of possession of key material when requesting a Credential (see Section 7.2). When received, the Wallet MUST use this nonce value for its subsequent credential requests until the Credential Issuer provides a fresh nonce
  c_nonce_expires_in?: number; // OPTIONAL. JSON integer denoting the lifetime in seconds of the c_nonce
  notification_id?: string;
}

I haven't tested if it's the same. But when they are different, we should remove it from the credentialoffersession, shouldn't we? Otherwise it gets difficult to map the notification_ids to the different types in the session.

[auer-martin]

I am not entirely sure what you mean. I thought a structure like this might make sense.

export interface CredentialOfferSession extends StateType {
   ...
   credentialIssuanceState: {
        [notificationId]: {
           credentialId: string,
           notificationEvent?: notificationEvent
        }
   }

Each CredentialResponse has a unique notification_id, and you send that id to the notification endpoint.
Whenever a notification event is received, we set it on the credentialIssuanceState.
So we don't remove the notification_id, we just set the value. This way, we also can check later which event we did receive.
We error if we get a notification with the same id again.
Did not spend too much time thinking about it. Does this make sense to you?

[cre8]

My fault, I was talking from the client side, not from the RP side.
I like your proposed suggestion. But according to the docs we need to return a sucess:
The notification from the Wallet is idempotent. When the Credential Issuer receives multiple identical calls from the Wallet for the same notification_id, it returns success. Due to the network errors, there are no guarantees that a Credential Issuer will receive a notification within a certain time period or at all.

So we need also to set a flag that we already got a repsonse and do not need to run an internal function again. When the access token got invalid, we are safe to remove the notification id.

[nklomp]

Yeah, although I am not entirely sure about not calling an internal function again.
I am leaning towards using the suggestion by @auer-martin to start storing the notification ids in the session. Then whenever we get the notification we always now it is okay and dispatch the internal events to listeners about the wallet outcome.

The problem lies whenever we get a call from a wallet if the session is already cleaned up because it was expired. Up until now the sessions were typically relatively short lived. This would need a more durable storage.

I am leaning towards a solution where we still store the notification ides in the session. It is then on any session persistence implementation, how long they keep sessions around (that is not necessarily the expiration time). If the notification id is found in the session we add an additional flag to the internal event that the notification id was correlated with the session. If not found we have a value that we could not match it to the session. In all cases we generate the events, to notify any listeners. That way any downstream listeners can decide how to handle it.

So to the outside world we are always returning okay. Internally we are always generating events, with a flag that shows whether the notification id was found in the sessions or not. There is no notion of duplicates or anything.

The other route would be to only generate the event internally once. But that would mean we need durable persistence for notification ids and keep track of a time at which we received the notification from the wallet