WIP: Change to cryptonite

[Vlix]

Started the switch to more than just scrypt.

• Changed over from scrypt package to the cryptonite package
• Added a Symbol parameter to the PassHash to keep it obvious which algorithm was used
• Added tests to check the hash is the same as from the scrypt package.
• Added functions to tweak the parameters for hashing

[Vlix]

I'll be implementing the bcrypt functions somewhere this weekend hopefully.

Note to self: also update password version to 2.0.0.0 and add to ChangeLogs

[cdepillabout]

I think this is looking really good.

I did a brief read-through and the only thing I really noticed is the phantom parameter. It'd probably be easier to just declare an empty data type like

data Scrypt

and use it like the following:

newtype PassHash a = PassHash
  { unPassHash :: Text
  } deriving (Eq, Ord, Read, Show)

hashPass :: MonadIO m => Pass -> m (PassHash Scrypt)
hashPass = hashPassWithParams defaultParams

[cdepillabout]

Also, if you're interested, I'd like to give you commit rights to this repo (as well as the package on Hackage) to make it easier for you to make changes like this.

It would also be helpful for you (and others?) if I were to ever go AWOL, especially if you're thinking of using this at work.

[Vlix]

I think this is looking really good.

I did a brief read-through and the only thing I really noticed is the phantom parameter. It'd probably be easier to just declare an empty data type like

data Scrypt

and use it like the following:

newtype PassHash a = PassHash
  { unPassHash :: Text
  } deriving (Eq, Ord, Read, Show)

hashPass :: MonadIO m => Pass -> m (PassHash Scrypt)
hashPass = hashPassWithParams defaultParams

Hmmm, true, better to not force pragmas on users.

Also, if you're interested, I'd like to give you commit rights to this repo (as well as the package on Hackage) to make it easier for you to make changes like this.

It would also be helpful for you (and others?) if I were to ever go AWOL, especially if you're thinking of using this at work.

Might be a good idea. Just to be sure. Not that I'll probably do a lot here, but who knows.

[Vlix]

Also @cdepillabout , have you any idea why this happens in the cabal travis checks?

src/Data/Password/Scrypt.hs:35:1: error:
    Ambiguous module name ‘Data.ByteString.Base64’:
      it was found in multiple packages:
      base64-0.4.0 base64-bytestring-1.0.0.3
   |
35 | import qualified Data.ByteString.Base64 as Base64

[cdepillabout]

any idea why this happens in the cabal travis checks

I think it is due to doctest weirdness (basically, just not running in the right environment). The easiest way to fix it is to just upgrade to using cabal-doctest.

For now you can ignore these failing tests. We can update to cabal-doctest in a future PR.

[Vlix]

Ok, I'll ignore the cabal tests.

Also, I'm not a big fan of unnecessary abbreviation (like with PassCheckSucc) so I was thinking of changing all mentions of Pass to Password. Like hashPass -> hashPassword, PassHash -> PasswordHash, etc. Do you have any objection to this?
(this is going to be a big rework of the library anyway, so if there's ever a time to do so it'd be now)

[cdepillabout]

I'm not a big fan of unnecessary abbreviation (like with PassCheckSucc) so I was thinking of changing all mentions of Pass to Password. Do you have any objection to this?

In general, I feel the same way.

In this specific instance, I was thinking that Pass is usually immediately obvious even when abbreviated.

However, I don't feel that strongly either way. And most apps don't have huge amounts of code dealing with passwords, so using the full word "password" shouldn't have a huge affect on how verbose the code feels.

So please feel free to change it.

[cdepillabout]

I've just given you commit access here on github.

If you let me know your username on Hackage, I will give you access there as well.

[Vlix]

I've just given you commit access here on github.

If you let me know your username on Hackage, I will give you access there as well.

Thanks, I'll DM you my Hackage username on twitter. My twitter handle is @FPaulusma

[cdepillabout]

This looks really good, I'm really happy with how it turned out.

I left a few comments, but none of these should be considered blockers.

I'd be happy if you wanted to merge this in, cut a release, and make an upload to hackage.

[Vlix]

Yeah, just the added documentation for all the defaultParams is something I'd like to get in there. I'm also thinking of removing unsafeShowPasswordText and make unsafeShowPassword :: Password -> Text, since anyone can change it to String if they want with Data.Text.unpack.

[cdepillabout]

I'm also thinking of removing unsafeShowPasswordText and make unsafeShowPassword :: Password -> Text

Yeah, that sounds good to me!

[Vlix]

Ok, I've implemented all the changes. Also added instances in password-instances with custom type errors. It looks __like_this__

EDIT: also wondering what cabal: --enable-tests was specified, but tests can't be enabled in a remote means in the GHCVER=head job

[cdepillabout]

@Vlix Okay, this looks good to me. Do you want to merge this in and make a release to Hackage?

You probably don't need it, but here is my checklist for making a release to Hackage:

https://functor.tokyo/blog/2018-07-16-release-haskell-packages-to-hackage

[Vlix]

Sure, let's merge this in. #10 can be closed then, I think.

Your checklist doesn't seem to include uploading the documentation and doesn't use the upload candidate functionality. I've only uploaded to hackage with cabal, so I wonder if stack upload just immediately uploads it for real, or does it start an interactive menu or something?

[cdepillabout]

Your checklist doesn't seem to include uploading the documentation and doesn't use the upload candidate functionality.

I normally tend to let Hackage build the docs for me, unless it can't, in which case I upload the documentation.

Running stack upload password and then stack upload password-instances should upload the two packages, and then Hackage will hopefully build the documentation for them.

However, if you're used to doing development with cabal, then using cabal to upload the packages (and documentation?) is completely okay with me!

[Vlix]

I took a last pass over the documentation for links etc. I'm also not sure what the proper etiquette is about the "Copyright" field at the top of the modules. Do I add my name there if I add (significantly) to the module? What if I just shuffle functions around? Should just the person be named that created the library?
I'm not sure what the regular procedures are for open-source projects. 🤔

[cdepillabout]

I'm also not sure what the proper etiquette is about the "Copyright" field at the top of the modules. Do I add my name there if I add (significantly) to the module? What if I just shuffle functions around? Should just the person be named that created the library?

I'm also not sure about this. There is also the LICENSE file (and maybe a copyright section in the .cabal file?).

I'd be happy if you wanted to add your name in all these places, as well as the author/maintainer section in the .cabal files, especially if you'll help me keep an eye on this library in the future.

Feel free to add your name, merge this in, and cut a new release.

[Vlix]

I don't mind maintaining the package as well. I've added my name in the appropriate places and will merge + upload later today.

[cdepillabout]

@Vlix I'd like to write a blog post about this update to the password library and post it to /r/haskell and twitter so more people are aware of it.

However, I imagine you'd be able to write a better post than me, given that you did all of the rewrite. Do you have a blog and would be willing to create a post about this?

If not, no worries, I'll throw something together.

[Vlix]

@cdepillabout I don't have a blog, no. So you can go ahead and write something up. Maybe a good idea to let me do a read over and adjust it here and there if need be.