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

Support Erlang 24 #24

Merged
merged 4 commits into from
Jun 5, 2021
Merged

Support Erlang 24 #24

merged 4 commits into from
Jun 5, 2021

Conversation

danielberkompas
Copy link
Owner

@danielberkompas danielberkompas commented Jun 5, 2021

This PR replaces the deprecated crypto hmac functions with modern ones when running on OTP > 22, without dropping support for OTP 21 and below. It also updates the dependency on cloak to version 1.1.1, which also supports OTP 24 with a similar strategy. This will allow this change to be a minor release rather than a major (breaking) release.

However, pbkdf2 users will need to use a forked version of that library to upgrade to OTP 24, since pbkdf2 still uses the deprecated HMAC functions. I made Cloak.Ecto rely on the fork at miniclip/erlang-pbkdf2 for its test suite, but users can select their preferred fork using the hex :override feature. For more details, see basho/erlang-pbkdf2#12.

For `pbkdf2` users, upgrading to OTP 24 will require them to use a fork
to no longer rely on the deprecated hmac functions. I chose to use
`miniclip/erlang-pbkdf2` because it seems to be maintained. But users
can choose whichever fork they prefer.
This module will only be partially compiled/tested by each build, so it
isn't necessary to track its test coverage for now.
@danielberkompas danielberkompas merged commit 9386b1c into master Jun 5, 2021
@danielberkompas danielberkompas deleted the cloak-version-1-1-0 branch June 5, 2021 20:52
danielberkompas added a commit that referenced this pull request Jun 5, 2021
It isn't possible to publish a package to Hex.pm with a dependency on a
Github fork which is not also hosted on Hex.pm. The current version of
`pbkdf2` is not compatible with Erlang 24. To use it, you'll need a
forked version, see #24.

For this reason, I needed to make the `pbkdf2` a `:dev, :test`
dependency instead of an `:optional` dependency for `cloak_ecto`. If you
add it to your mix dependencies in your project, `cloak_ecto` should
still detect it; it shouldn't need to be listed as an optional
dependency.
danielberkompas added a commit that referenced this pull request Jun 5, 2021
It isn't possible to publish a package to Hex.pm with a dependency on a
Github fork which is not also hosted on Hex.pm. The current version of
`pbkdf2` is not compatible with Erlang 24. To use it, you'll need a
forked version, see #24.

For this reason, I needed to make the `pbkdf2` a `:dev, :test`
dependency instead of an `:optional` dependency for `cloak_ecto`. If you
add it to your mix dependencies in your project, `cloak_ecto` should
still detect it; it shouldn't need to be listed as an optional
dependency.
danielberkompas added a commit that referenced this pull request Jun 5, 2021
It isn't possible to publish a package to Hex.pm with a dependency on a
Github fork which is not also hosted on Hex.pm. The current version of
`pbkdf2` is not compatible with Erlang 24. To use it, you'll need a
forked version, see #24.

For this reason, I needed to make the `pbkdf2` a `:dev, :test`
dependency instead of an `:optional` dependency for `cloak_ecto`. If you
add it to your mix dependencies in your project, `cloak_ecto` should
still detect it; it shouldn't need to be listed as an optional
dependency.
Comment on lines +5 to +15
if System.otp_release() >= "22" do
@impl Cloak.Ecto.Crypto.Interface
def hmac(algorithm, key, data) do
:crypto.mac(:hmac, algorithm, key, data)
end
else
@impl Cloak.Ecto.Crypto.Interface
def hmac(algorithm, key, data) do
:crypto.hmac(algorithm, key, data)
end
end

Choose a reason for hiding this comment

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

❤️

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.

2 participants