Add pgcrypto crypt() functions

[3kyro]

As described in #256

Before submitting your PR, check that you've:

• [x ] Bumped the version number.
• [ x] Documented new APIs with Haddock markup.
• [ x] Added @since declarations to the Haddock.
• [ x] Ran stylish-haskell and otherwise adhered to the style guide.

After submitting your PR:

• [ x] Update the Changelog.md file with a link to your PR.
• [ x] Check that CI passes (or if it fails, for reasons unrelated to your change, like CI timeouts).

[3kyro]

Some comments:

• I have not bumped the version number, will it be 3.5.1.0?
• For the tests, the esqutest database should have the pgcrypto module installed. Could this simply be put as an additional step in the readme or do I create a migration step?

[belevy]

Could you put the functions in their own module. Is pgcrypto installed by default in the github actions container or does that need to be added? Perhaps we should be able to run without the crypto functions?

[3kyro]

CI gives "function gen_salt(unknown) does not exist" which means pgcrypto is not installed by default.

I think the easiest would be to have a rawSql call inside the test

rawSql "CREATE EXTENSION IF NOT EXISTS pgcrypto" []

I'll put the function in their own module.

[3kyro]

Could you put the functions in their own module. Is pgcrypto installed by default in the github actions container or does that need to be added? Perhaps we should be able to run without the crypto functions?

Another way would be to query pg_extension during the test and abort if pgcrypto is not found. Would you prefer this rather than enabling it each time?

[belevy]

can you update the changelog and version for this.

@parsonsmatt im still not sure how the numbering scheme works, would this be 3.5.1.0 or 3.5.0.1?

[3kyro]

can you update the changelog and version for this.

@parsonsmatt im still not sure how the numbering scheme works, would this be 3.5.1.0 or 3.5.0.1?

I guess if we follow Haskell PVP this is a non breaking change that adds functionality so 3.5.1.0 is appropriate

Non-breaking change. Otherwise, if only new bindings, types, classes, non-orphan instances or modules (but see below) were added to the interface, then A.B MAY remain the same but the new C MUST be greater than the old C. Note that modifying > imports or depending on a newer version of another package may cause extra non-orphan instances to be exported and thus > force a minor version change."

[3kyro]

Hi @belevy , @parsonsmatt . Is there something I can do to have this merged? Thanks!

[3kyro]

Hi @parsonsmatt , thanks for the great review and sorry for the delay in answering. I agree that your proposed API is much more concise and elegant. I have a slight worry that we allow the user to use the same salt when storing passwords, eg:

-- when storing a password
crypt (val pass) (RegularValue (val "a very bad salt"))

however this is a inherent limitation of the original crypt function and the user can simply be warned about it in the documentation. Would you be OK with this?

I have rebased to master and will move on with your suggestions shortly

[3kyro]

Hi @parsonsmatt, sorry to drag this on, but I have another issue with the proposed API. Pg's crypt function annoyingly swaps arguments depending on the type of operation (store/retrieve).

// Example of setting a new password:
UPDATE ... SET pswhash = crypt('new password', gen_salt('md5'));
//-----------------------------------------------------^ GenSalt 

//Example of authentication:
SELECT pswhash = crypt('entered password', pswhash) FROM ... ;
//-------------------------^RegularValue

Since we're not able to coerce the SaltOr value once wrapped in a SqlExpr we won't be able to correctly place the arguments in the call to unsafeSqlFunction. The way I see it there are three possibilities:

1. Use separate toCrypt and fromCrypt functions but with all arguments wrapped in a SqlExpr This has the benefit of addressing my previous concern about contant salts, but of course isn't so cool as a single crypt function
2. Use a single crypt function, but with a pure SalrOr argument so that we can pattern match on it. Apart from breaking the library's idiom, wrapping the hash algorithm / requested password would be unwieldy.
3. Use a single crypt function and try to deduce the context by parsing the ERaw constructor. Seems too hacky.

I would go for option no 1 but I would greatly appreciate your opinion.