taxjar / vat_check

Validate EU VAT numbers the easy way with VIES!
23 stars 16 forks source link

Update https url & follow redirects #3

Closed bittersweet closed 4 years ago

bittersweet commented 4 years ago

We had some issues starting today, where the VAT validation was failing because ec.europa.eu is redirecting.

The http URL for the .wsdl file redirects to https now, I've changed that in the gem.

However, the links in the actual wsdl are still http, like so:

$ tail checkVatService.wsdl
        <wsdlsoap:body use="literal"/>
      </wsdl:output>
    </wsdl:operation>
  </wsdl:binding>
  <wsdl:service name="checkVatService">
    <wsdl:port name="checkVatPort" binding="impl:checkVatBinding">
      <wsdlsoap:address location="http://ec.europa.eu/taxation_customs/vies/services/checkVatService"/>
    </wsdl:port>
  </wsdl:service>
</wsdl:definitions>

That means I had to enable follow_redirects in Savon, otherwise it is going to error out on the 302 status. Another option was to bundle the wsdl file, and update http to https, but I thought this solution was more future proof.

I am not too familiar with SOAP/WSDL or this vat check service itself, but I expect them to update their wsdl at some point so it uses https :-)

bittersweet commented 4 years ago

FYI the build is failing because I updated webmock which updated public_suffix, which then only works on ruby >2.3.

Let me know how you think I should proceed :-)

prsimp commented 4 years ago

@fastdivision -- do we want to update this gem to require 2.3+ to stay in-sync with taxjar-ruby?

MuhammadYossry commented 4 years ago

The service updated its wsdl to https. would be great to refresh the issue. I think the redirect logic is still beneficial although for the future changes of this service.

bittersweet commented 4 years ago

Thanks for the update @MuhammadYossry :+1:

(I have to admit it has been a while since I looked at this, so it's not top of mind for me) I just downloaded the wsdl and I see they still point to a http endpoint. I think this means means the redirect logic is still a requirement

  <wsdl:service name="checkVatService">
    <wsdl:port name="checkVatPort" binding="impl:checkVatBinding">
      <wsdlsoap:address location="http://ec.europa.eu/taxation_customs/vies/services/checkVatService"/>
    </wsdl:port>
  </wsdl:service>
</wsdl:definitions>
MuhammadYossry commented 4 years ago

@bittersweet Thanks for the response, I used the gem internally and it stopped working after the https upgrade. I monkey patched this file to get it to work. I think it's a functional requirement, will be great to see it in the upstream.

tomdmaguire commented 4 years ago

+1 for getting this merged pls!

fastdivision commented 4 years ago

@bittersweet Can you bump VCR as well and we'll get this merged? Thank you for this awesome contribution!

bittersweet commented 4 years ago

Hi @fastdivision , I've just updated VCR, it uses the latest version now.

The build is failing because of the mentioned ruby version, but this is green for me locally ("it works on my machine!.." ;-) )

A possible improvement: I know you can configure the Gemfile with gemspec so it looks at vat_check.gemspec for its dependencies and versioning, but I didn't want to make too many changes here :-)