Support X509Certificate in INI and HIA request
#168
Conversation
|
|
There was a problem hiding this comment.
This looks generally ok. I left some comments for which I would suggest a bit of refactoring. I have no EBICS access with which I could test it.
In case where certificates are being used, it should either be all or nothing. Therefore we should probably validate for that.
We probably also do not need to keep all 3 as attr_accessor variables on the client.
The most important piece that is missing here are usage examples for the README.
Without that, the change is not really complete for something as specific as this.
I would not expect anyone to figure it out by reading the code unless they know that it is already an available feature.
| self.x_509_certificate_a_content = options[:x_509_certificate_a_content] | ||
| self.x_509_certificate_x_content = options[:x_509_certificate_x_content] | ||
| self.x_509_certificate_e_content = options[:x_509_certificate_e_content] |
There was a problem hiding this comment.
Is there any case where only one of them should be passed?
There was a problem hiding this comment.
Theoretically, yes—because each request type requires a different x509 certificate. For example, if you only need to make an INI request, you just need x_509_certificate_a_content. For an HIA request, you’ll need both x_509_certificate_x_content and x_509_certificate_y_content.
| Nokogiri::XML::Builder.new do |xml| | ||
| xml.SignaturePubKeyOrderData('xmlns:ds' => 'http://www.w3.org/2000/09/xmldsig#', 'xmlns' => 'http://www.ebics.org/S001') { | ||
| xml.SignaturePubKeyInfo { | ||
| if x_509_certificate_a |
There was a problem hiding this comment.
The x509 xml data looks the same in all 3 places, just using a different input.
I would suggest moving that into a function/method to re-use and shorten the part of the xml document.
| attr_accessor :passphrase, :url, :host_id, :user_id, :partner_id, :keys, :keys_content, :locale, :product_name | ||
| attr_writer :iban, :bic, :name | ||
| attr_accessor :passphrase, :url, :host_id, :user_id, :partner_id, :keys, :keys_content, :locale, :product_name, | ||
| :x_509_certificate_a_content, :x_509_certificate_x_content, :x_509_certificate_e_content, :debug_mode |
There was a problem hiding this comment.
Having individual long method names for each individual certificate does not seem very elegant.
Having the 3 keys combined under 1 method allows for keeping the immediate surface area of the client class way smaller.
This PR adds support for using x509 self-signed certificates in INI and HIA requests.
Details
init_letterto use certificate data instead of a,x and e keys.debug_mode: booleanoption to client initialization, allowing easy logging of requests and responses for debugging purposes. When debug_mode is enabled, Faraday’s logger middleware is activated to output request and response bodies to STDOUTImpact
The changes are tested on plenty of 🇫🇷 banks: BNP, HSBC, Société Générale, LCL.
EBICS - Guide de mise en œuvre en France