Phase 2 Support #15
Merged
Phase 2 Support #15
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- httpx for making API calls - vcr and webmock to help with caching and testing responses - amazing_print to help examine responses.
Closed
This is a lot of changes in one commit, which is something I’ve been trying to avoid, but I spent A LOT of work getting this to finally work and this does have a passing unit test that generates the same exact signed simplified invoice as ZATCA’s sample.
The samples in ZATCA’s docs have all kinds of formats.
This is currently purposefully failing with the diff at the end of the message. The goal is to make it pass once we figure out the things I wrote below.
(the red diff or the one with minus sign is the version that’s in ZATCA’s samples, the green or plus is ours)
1. Find out why the signature method is different
```diff
- <ds:SignatureMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"/>
+ <ds:SignatureMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#ecdsa-sha256"/>
```
2. Find out if closing styles matter
```diff
- <ds:SignatureValue></ds:SignatureValue>
+ <ds:SignatureValue/>
```
3. Find out if keeping schemeAgencyID and schemeID on TaxCategory is alright
```diff
- <cbc:ID>S</cbc:ID>
+ <cbc:ID schemeAgencyID="6" schemeID="UN/ECE 5305">S</cbc:ID>
```
```diff
- <cbc:ID>VAT</cbc:ID>
+ <cbc:ID schemeAgencyID="6" schemeID="UN/ECE 5153">VAT</cbc:ID>
```
Entire diff:
```diff
@@ -11,7 +11,7 @@
<ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#" Id="signature">
<ds:SignedInfo>
<ds:CanonicalizationMethod Algorithm="http://www.w3.org/2006/12/xml-c14n11"/>
- <ds:SignatureMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"/>
+ <ds:SignatureMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#ecdsa-sha256"/>
<ds:Reference Id="invoiceSignedData" URI="">
<ds:Transforms>
<ds:Transform Algorithm="http://www.w3.org/TR/1999/REC-xpath-19991116">
@@ -33,7 +33,7 @@
<ds:DigestValue>M2ZkZWViYTg3OGYwNGQ3ZjhkOGJiNWUyZjlhODViMTc1YTg0MmE4MDFmNjU1MWJhYmYyYWFlMDc4MjRmMGVlOQ==</ds:DigestValue>
</ds:Reference>
</ds:SignedInfo>
- <ds:SignatureValue></ds:SignatureValue>
+ <ds:SignatureValue/>
<ds:KeyInfo>
<ds:X509Data>
<ds:X509Certificate>MIID3DCCA4KgAwIBAgITbwAAZIQwd/uzGGbr+QABAABkhDAKBggqhkjOPQQDAjBjMRUwEwYKCZImiZPyLGQBGRYFbG9jYWwxEzARBgoJkiaJk/IsZAEZFgNnb3YxFzAVBgoJkiaJk/IsZAEZFgdleHRnYXp0MRwwGgYDVQQDExNUU1pFSU5WT0lDRS1TdWJDQS0xMB4XDTIyMDMxNTA4MjkwNVoXDTIyMDMxNzA4MjkwNVowTzELMAkGA1UEBhMCU0ExEzARBgNVBAoTCmhheWEgeWFnIDMxFzAVBgNVBAsTDmFtbWFuIEJyYW5jaGNoMRIwEAYDVQQDEwkxMjcuMC4wLjEwVjAQBgcqhkjOPQIBBgUrgQQACgNCAATTAK9lrTVko9rkq6ZYcc9HDRZP4b9S4zA4Km7YXJ+snTVhLkzU0HsmSX9Un8jDhRTOHDKaft8C/uuUY934vuMNo4ICKjCCAiYwgYsGA1UdEQSBgzCBgKR+MHwxHTAbBgNVBAQMFDEtaGF5YXwyLTIzNHwzLTcxMTExMR8wHQYKCZImiZPyLGQBAQwPMzAwMDU1MTg0NDAwMDAzMQ0wCwYDVQQMDAQxMTExMREwDwYDVQQaDAhaYXRjYSAxMjEYMBYGA1UEDwwPRm9vZCBCdXNzaW5lc3MzMB0GA1UdDgQWBBSgmIWD6bPfbbKkmTwOJRXvIbH9HjAfBgNVHSMEGDAWgBR2YIz7BqCsZ1c1nc+arKcrmTW1LzBOBgNVHR8ERzBFMEOgQaA/hj1odHRwOi8vdHN0Y3JsLnphdGNhLmdvdi5zYS9DZXJ0RW5yb2xsL1RTWkVJTlZPSUNFLVN1YkNBLTEuY3JsMIGtBggrBgEFBQcBAQSBoDCBnTBuBggrBgEFBQcwAYZiaHR0cDovL3RzdGNybC56YXRjYS5nb3Yuc2EvQ2VydEVucm9sbC9UU1pFaW52b2ljZVNDQTEuZXh0Z2F6dC5nb3YubG9jYWxfVFNaRUlOVk9JQ0UtU3ViQ0EtMSgxKS5jcnQwKwYIKwYBBQUHMAGGH2h0dHA6Ly90c3RjcmwuemF0Y2EuZ292LnNhL29jc3AwDgYDVR0PAQH/BAQDAgeAMB0GA1UdJQQWMBQGCCsGAQUFBwMCBggrBgEFBQcDAzAnBgkrBgEEAYI3FQoEGjAYMAoGCCsGAQUFBwMCMAoGCCsGAQUFBwMDMAoGCCqGSM49BAMCA0gAMEUCIARehvaSyyUUyKpOrF/loNYHOWypNKttFPpUIm4dLwyaAiEAiNlHW6XNGo3sETvQxqVF4bx5AAw14BmXiYic1ZrtAOM=</ds:X509Certificate>
@@ -162,10 +162,10 @@
<cbc:TaxableAmount currencyID="SAR">900.00</cbc:TaxableAmount>
<cbc:TaxAmount currencyID="SAR">135.00</cbc:TaxAmount>
<cac:TaxCategory>
- <cbc:ID>S</cbc:ID>
+ <cbc:ID schemeAgencyID="6" schemeID="UN/ECE 5305">S</cbc:ID>
<cbc:Percent>15</cbc:Percent>
<cac:TaxScheme>
- <cbc:ID>VAT</cbc:ID>
+ <cbc:ID schemeAgencyID="6" schemeID="UN/ECE 5153">VAT</cbc:ID>
</cac:TaxScheme>
</cac:TaxCategory>
</cac:TaxSubtotal>
```
This is a giant commit I spent too long on but it mostly: 1. Adds (under progress) support for hashing/signing 2. Extracting certificate signatures 3. Using Dry Initializer for larger classes
The public key of the certificate is supposed to be in bytes, and we needed the certificate’s signature in the QR code rather than its hash. Also match ZATCA’s timestamp and VAT total format on this QR code.
ZATCA previously instructed to convert strings to 8-bit ASCII but now they allow UTF8 characters and throw errors if you return 8-bit ASCII.
- Ensure invoice signer generates both Base64 and SHA256 hashes, as both are needed in different contexts. - Ensure we generate hashes on canonicalized XML only, for both the invoice and signed properties. - Update specs with correct values.
In order to match wes4m/zatca-xml-js, the tag order that was in our repo was already correct before.
This actually integrates the change that wes4m did in `lib/zatca/signing/certificate.rb`
ZATCA’s latest update is that SignedProperties should not be canonicalized anymore. You should use a specific version of the XML element with specific whitespace and add a single attribute only when hashing SignedProperties but remove it in all other instances. We now use a HEREDOC just for generating this version of the XML for hashing because it does not have consistent indentation. See: https://zatca1.discourse.group/t/what-do-signed-properties-look-like-when-hashing https://web.archive.org/web/20230925182417/https://zatca1.discourse.group/t/what-do-signed-properties-look-like-when-hashing/717
obahareth
force-pushed
the
feature/api_wrapper
branch
from
eaa4fd4
to
a3588fe
Compare
Lint issues: An extra new line Tests: A variable removed in the last commit was still being referenced
After generating the XML, we modify the QualifyingProperties block’s indentation to match ZATCA’s. ZATCA’s server performs hashing serverside on the SignedProperties as-is without transforming them to their required indentation format. This forces us to have to make the SignedProperties have the same identation as in their files.
obahareth
force-pushed
the
feature/api_wrapper
branch
from
1b434ae
to
043fdda
Compare
Apparently our approach from many months ago (with the exception of SignedProperties spacing) works fine, ZATCA says they did not change anything on their end but suddenly that approach is passing their validator.
obahareth
force-pushed
the
feature/api_wrapper
branch
from
1bea6e6
to
d88eef5
Compare
|
Good news, this is passing ZATCA's compliance checks. I'm just doing some final updates to the wiki and verifications and will be merging this in the coming weeks. |
This should help avoid active_support errors like NoMethodError on simple functions like `blank?` in GitHub Actions and non-Rails environments where ActiveSupport isn’t loaded.
obahareth
force-pushed
the
feature/api_wrapper
branch
from
d33aac7
to
10b4370
Compare
This is the latest version from their website, the odd things are: 1. The Java JAR file is gone 2. The version number changed from 3.1.8 to 3.2.7 Download link: https://zatca.gov.sa/en/E-Invoicing/SystemsDevelopers/ComplianceEnablementToolbox/Pages/DownloadSDK.aspx https://sandbox.zatca.gov.sa/downloadSDK Both links give different files, not sure which is correct.
obahareth
force-pushed
the
feature/api_wrapper
branch
from
471a4b7
to
9d564ba
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Support for #11 and #8.
Update Wiki with
For a future PR