search

chulkilee / ex_force

A Salesforce REST API wrapper for Elixir
https://hex.pm/packages/ex_force
MIT License
38 stars 27 forks source link

Use :crypto.mac/4 for OAuth due to api deprecations in OTP 24 #55

Closed jeremy-hanna closed 3 years ago

jeremy-hanna commented 3 years ago

👋 Hi there, we are in the process of upgrading our systems and use of :crypto.hmac/3 was scheduled for removal in OTP 24

This PR is an update of the oauth.ex to the new api of :crypto.mac/4. I've tested our fork authenticating against our sandbox account and its been working fine

This will unfortunately put a lower bound on the version of OTP you can use as it was added in OTP 22 so it may be a breaking change, so let me know how you would like to proceed 🙂

hexpunk commented 3 years ago

If keeping backwards compatibility is important, this could be done:

crypto_args = [:sha256, client_secret, id <> issued_at_raw)]

if function_exported?(:crypto, :mac, 4) do
  apply(:crypto, :mac, [:hmac] ++ crypto_args)
else
  apply(:crypto, :hmac, crypto_args)
end
|> Base.encode64()

It ain't pretty, but it should work when :crypto.hmac is yanked. I got this idea from https://www.kianmeng.org/2020/12/ensure-backward-compatibility-for-your.html

chulkilee commented 3 years ago

@jeremy-hanna could you make it work in older version with funciton_exported?? Also could you add OTP 21 to CI config ? Since elixir 1.11 supports OTP 21 ref, and we need to test it :heart: for this change.

See https://github.com/elixir-lang/elixir/blob/v1.11.4/lib/elixir/lib/system.ex#L606 as an example. Let's define wrap the function def with the body as it does.

@JayAndCatchFire thanks for pointing it out! :heart:

sourcelevel-bot[bot] commented 3 years ago

SourceLevel has finished reviewing this Pull Request and has found:

See more details about this review.

coveralls commented 3 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 4864efdeb6d8ba26da21d4c96c1721b18580997b on jeremy-hanna:use-crypto-mac-for-otp-24-deprecations into da45d32b3cb4d8fbb88b9e195246403f77de0880 on chulkilee:main.

chulkilee commented 3 years ago

Thanks @jeremy-hanna ! I made CI changes for recent Elixir (1.12) and OTP (24) releases and picked up changes of this PR into #56, which is just merged. Thanks again :tada:

Dacello commented 2 years ago

@chulkilee any chance we can get a new release with this fix? Im getting errors using erlang 24 with ex_force version 0.4.0

chulkilee commented 2 years ago

@Dacello I can release it this weekend. Please ping me again if I forget to do that!

Dacello commented 2 years ago

@chulkilee pinging you since looks like we still dont have a new release! 😏