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: Brazilian standard require idNumber, back document and email fo… #85

Closed
wants to merge 7 commits into from

Conversation

ailsoncgt
Copy link
Contributor

kyc validation

selfieDocument: `string`,
identificationDocument: `string`
identificationDocument: `string`,
identificationDocumentBack?: `string`,
Copy link
Contributor

Choose a reason for hiding this comment

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

if you require idNumber, email, and identificationDocumentBack, then they shouldn't be optional fields. The question mark indicates that the field is optional.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, I recommend that you create a new KYC schema for these fields, rather than tacking them onto an existing one. Adding a new KYC schema is a non-breaking change, since clients can incrementally add support for new schemas without breaking their existing integrations. However, changing an existing KYC schema is generally a breaking change, and something to avoid without some compelling reason for it

}
```

The `selfieDocument`, `identificationDocument` and `identificationDocumentBack` fields should be base64 encoded binary blobs representing images.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you provide some more detailed information on the fields required in this schema? (For example, how phoneNumber should be formatted, what the idNumber represents and how it should be formatted, etc.) It might seem a bit pedantic but it's important that we document the exact syntax of these fields.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok @jophish , I will do that.

@ailsoncgt ailsoncgt requested review from jophish and cajubelt and removed request for jophish and cajubelt November 29, 2022 16:43
@ailsoncgt
Copy link
Contributor Author

@jophish I made a new commit

Comment on lines +2261 to +2262
identificationDocumentFront: `string`,
identificationDocumentBack: `string`,
Copy link
Contributor

Choose a reason for hiding this comment

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

what kind of identity document is accepted? Any? Or just CPF cards? The description of the schema should probably go into this-- we're currently running into trouble with another KYC schema due to the ambiguity there on what document types are acceptable.

If it's CPF only, the schema should probably also be renamed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cajubelt the docs are those accepted as an official identification document in Brazil: RG, CNH, RNM

@ailsoncgt ailsoncgt requested review from cajubelt and removed request for jophish January 4, 2023 14:55
@@ -2233,6 +2234,41 @@ A KYC schema containing personal data about a user, as well as documents such as

The `selfieDocument` and `identificationDocument` fields should be base64 encoded binary blobs representing images.

#### 9.3.1.2. `PersonalDataAndDocumentsWithBack`
Copy link
Contributor

Choose a reason for hiding this comment

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

this schema name is misleading because it sounds more general than it actually is. Currently the constraints on document type, phone number and ID make this specific to Brazilian KYC checks.

#### 9.3.1.2. `PersonalDataAndDocumentsWithBack`

A brasilian standard KYC schema containing personal data about a user, as well as documents such as an ID, photo and selfie.
The accepted documents are [RG](https://en.wikipedia.org/wiki/Brazilian_identity_card), [CNH](https://en.wikipedia.org/wiki/Driving_licence_in_Brazil) and [RNM](https://en.wikipedia.org/wiki/Registro_Nacional_de_Estrangeiros)
Copy link
Contributor

Choose a reason for hiding this comment

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

all 3 of these documents have CPF on them, which the image would presumably include. Does idNumber still need to be its own field? Seems like it could be redundant, though I don't know the details of how you're doing KYC checks..

Copy link
Contributor

@cajubelt cajubelt Jan 5, 2023

Choose a reason for hiding this comment

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

if you can drop the idNumber field, or clarify that it needs to be CPF specifically for the case where some new field like documentType is equal to RG, CNH, or RNM, then you could potentially make this schema more general / reusable across regions

Copy link
Contributor

Choose a reason for hiding this comment

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

which could be helpful for other providers and consumers of the FC API

@ailsoncgt
Copy link
Contributor Author

ailsoncgt commented Jan 6, 2023

@cajubelt in my new commit I removed idNumber. I confirmed that it is not actually required.
About the field you suggested, documentType, for our purpose it would be a dead field, because it will not be evaluated in real time for the KYC result. And without it, the scheme is open for other providers to receive other types of documents. I indicated the RG, RNM and CNH because they are the documents accepted in Brazil and consequently by our KYC provider. But the provider already recognizes the document type at the time of submission.

@jophish
Copy link
Contributor

jophish commented Jan 19, 2023

@ailsoncgt I believe the intent of the documentType field is to disambiguate, for the client what kind of document is required. For example, if documentType could be one of RG, RNM, or CNH, then you, as a provider can set allowedValues to ["RG", "RNM", "CNH"], which indicates to the client that the user must submit one of those documents, and the client can then provide the specific means to the end-user to do so. If another provider wants to re-use this schema, but only supports, say, RG, then they can set their allowedValues field for documentType to just ["RG"]. If another provider comes along and wants to add support for a new document type, they can do so by extending the list of valid document types and setting their allowedValues field as required.

TLDR, documentType is important in order to allow providers to extend this schema, so that clients can communicate to end-users exactly what kind of document(s) the provider is requesting.

@cajubelt
Copy link
Contributor

superceded by #92

@cajubelt cajubelt closed this Jan 30, 2023
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.

None yet

3 participants