Skip to content

Commonalities r4.4 alignment - #66

Open
GillesInnov35 wants to merge 12 commits into
mainfrom
Commonalities-r4.4-alignment
Open

GillesInnov35 wants to merge 12 commits into
mainfrom
Commonalities-r4.4-alignment

Conversation

@GillesInnov35

@GillesInnov35 GillesInnov35 commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Updated references to error responses in the KYC age verification API specification to point to the correct components in CAMARA_common.yaml.

What type of PR is this?

Add one of the following kinds:

  • correction

What this PR does / why we need it:

Commonalities r4.4 alignment

  • [P-027] info.description template 'additional-error-responses' has drifted from canonical at paragraph 3
  • The Generic* responses below are DEPRECATED as of Commonalities 0.9.0
  • update Test_definitions/kyc-age-verification.feature with 403 and 429 tests features

Which issue(s) this PR fixes:

Fixes #67

Updated references to error responses in the KYC age verification API specification to point to the correct components in CAMARA_common.yaml.
@camara-validation

camara-validation Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

CAMARA Validation — PASS

0 errors, 0 warnings, 6 hints | Profile: standard

View full results

Removed various error response definitions from the KYC age verification API specification, retaining only the 'not_available' enum value.

@PedroDiez PedroDiez left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM for API Spec, minor coments on tests file (editorial aligment)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@GillesInnov35

Copy link
Copy Markdown
Contributor Author

@PedroDiez , could review file changes in kyc-age-verification.feature. Scenarios should be now aligned on Commonalities r4.4 tests scenarios. Thanks

@PedroDiez PedroDiez left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

And the response property "$.message" contains a user friendly text

@kyc-age-verification_verifyAge_403.02_api_client_token_mismatch
Scenario: "/verify" not created by the API client given in the access token

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

NOTE: This can be for WG decision, replace /verify by verifyAge (May be done later in case commented by RM)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

thanks a lot @PedroDiez ,
verifyAge is the operationId not the resource name. In test scenario_template {resource} must be replaced by the API resource which is consumed is /verify, isn't it ?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

To me is fine, just in case that comment would happen later, it will be no problem from our side

@fernandopradocabrillo fernandopradocabrillo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@GillesInnov35

Copy link
Copy Markdown
Contributor Author

@ToshiWakayama-KDDI , could you review this PR. Thanks a lot

And the header "Content-Type" is set to "application/json"
And the header "Authorization" is set to a valid access token
And the header "x-correlator" complies with the schema at "#/components/schemas/XCorrelator"
And the header "x-correlator" complies with the schema at "../common/CAMARA_common.yaml#/components/headers/x-correlator"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This update is not needed.
In the API spec, the reference points to the local CAMARA_common.yaml copy, which is to avoid to change that reference it in case it would be updated (that is the changes are sync via updates of that local copy of commonalities)

When API bundling is performed via RM automation flow, that process makes auto-contained schemas so that the current value is valid

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@ToshiWakayama-KDDI

Copy link
Copy Markdown
Contributor

Thanks, @GillesInnov35 , @PedroDiez , @fernandopradocabrillo .

Really sorry for the delayed comments, but I have some comments and questions.

  1. Regarding 404 IDENTIFIER_NOT_FOUND, by my reading of the CAMARA_common.yaml, there seems to be the following difference:
  • [current] message: The phone number provided is not associated with a customer account
  • [modified] message: The identifier is not found.

Won't this cause any problem?

@ToshiWakayama-KDDI

Copy link
Copy Markdown
Contributor
  1. Regarding 422 errors, by my reading of the CAMARA_common.yaml, there seem to be the following differences:

422 SERVICE_NOT_APPLICABLE

  • [current] message: The service is not applicable for the provided phone number
  • [modified] The service is not available for the provided identifier.

422 MISSING_IDENTIFIER

  • [current] message: No phone number has been provided
  • [modified] The phone number cannot be identified

422 UNNECESSARY_IDENTIFIER

  • [current] message: An explicit phone number has been provided when one is already associated with the access token
  • [modified] The phone number is already identified by the access token.

Won't these cause any problem? WDYT?

@ToshiWakayama-KDDI

Copy link
Copy Markdown
Contributor

Hi @GillesInnov35, @PedroDiez , @fernandopradocabrillo

  1. Regarding 429 errors, by my reading of the CAMARA_common.yaml, there seem to be the following difference:

[current] 429 QUOTA_EXCEEDED is defined.
[modified] 429 QUOTA_EXCEEDED is not defined.

In the latest CAMARA_common.yaml, in Line 624~ TooManyRequests429 is defined, and TOO_MANY_REQUEST is included, but QUOTA_EXCEEDED is not included.

However, in Line 1417~ GENERIC_429_TOO_MANY_REQUESTS is defined, and below, in Line 1423~, GENERIC_429_QUOTA_EXCEEDED is defined as well. It seems that GENERIC_429_QUOTA_EXCEEDED is not refered to, even though it is defined in Line 1423~. I don't know why.

I think simply we could add QUOTA_EXCEEDED into TooManyRequests429 (in Line 624~), which can refer to GENERIC_429_QUOTA_EXCEEDED. I mean we could modify the CAMARA_common.yaml, though.

WDYT?

@ToshiWakayama-KDDI

Copy link
Copy Markdown
Contributor

Hi @GillesInnov35 , @PedroDiez , @fernandopradocabrillo ,

The proposed modification for '[P-027] info.description template 'additional-error-responses' has drifted from canonical at paragraph 3 ' looks good to me.

I am going to look at the test part. Sorry for the delayed action.

Best regards,
Toshi

@ToshiWakayama-KDDI

Copy link
Copy Markdown
Contributor

Hi @GillesInnov35 ,

Merci beaucoup de the test case modification proposal.

May I ask one quick question?
There are many changes from 'When the HTTP “POST” request is sent' to 'When the request “verifyAge” is sent'. Are these because of one of the Commpnalities r4.4 changes?

Thanks,
Toshi

@GillesInnov35

Copy link
Copy Markdown
Contributor Author

May I ask one quick question? There are many changes from 'When the HTTP “POST” ' to 'When the request “verifyAge” is sent'. Are these because of one of the Commpnalities r4.4 changes?

Yes Toshi, changes proposal (replace "When the HTTP “POST” request is sent) are aligned on what has been included in Commonalities r4.4. I think they are not mandatory but to avoid any issues it could be interesting to include those changes.

@PedroDiez

Copy link
Copy Markdown

Hi @ToshiWakayama-KDDI,

cc @GillesInnov35

About the API Specification topics:

  1. 404 IDENTIFIER_NOT_FOUND. Its schema is aligned with the Commonalities r4.4. https://github.com/camaraproject/KnowYourCustomerAgeVerification/blob/main/code/common/CAMARA_common.yaml#L1289. So it reads the canonical message. message is not normative, so it is just the canonical reference, an implementation could send other message. So no problem at all.

  2. 422 errors. Its schema is aligned with the Commonalities r4.4. https://github.com/camaraproject/KnowYourCustomerAgeVerification/blob/main/code/common/CAMARA_common.yaml#L595. So basically, same situation as above

  3. Really, the applicable 429 code is only TOO_MANY_REQUEST. 429 - QUOTA_EXCEEDED is not applicable in this API. This is being aligned across all KYC family APIs

@GillesInnov35

Copy link
Copy Markdown
Contributor Author
  1. Regarding 404 IDENTIFIER_NOT_FOUND, by my reading of the CAMARA_common.yaml, there seems to be the following difference:

    • [current] message: The phone number provided is not associated with a customer account

    • [modified] message: The identifier is not found.

Won't this cause any problem?

I understood that these changes were intended to standardize error messages, particularly those related to the phone number and access token. I didn't see any way to put a local message if we include the CAMARA_common reference. As it is only message I think this is not a problem.
@PedroDiez , @rartych , WDYT ?

@PedroDiez

Copy link
Copy Markdown
  1. Regarding 404 IDENTIFIER_NOT_FOUND, by my reading of the CAMARA_common.yaml, there seems to be the following difference:

    • [current] message: The phone number provided is not associated with a customer account
    • [modified] message: The identifier is not found.

Won't this cause any problem?

I understood that these changes were intended to standardize error messages, particularly those related to the phone number and access token. I didn't see any way to put a local message if we include the CAMARA_common reference. As it is only message I think this is not a problem. @PedroDiez , @rartych , WDYT ?

Not a problem at all to use CAMARA_common reference, Gilles

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Commonalities r4.4 alignment

4 participants