mdsol / dice_bag

DiceBag is a library of rake tasks for configuring web apps in the style of The Twelve-Factor App.
MIT License
19 stars 4 forks source link

MCC-56949 Add fix to change if private key string is provided into RSA format #56

Closed lgreene-mdsol closed 11 years ago

lgreene-mdsol commented 11 years ago

Please review :)

asmith-mdsol commented 11 years ago

Thanks for contributing this @lgreene-mdsol! Three thoughts:

Firstly, having had time to think this through, rather than putting the complete ASCII-armoured key in the environment, why don't we just put the encoded (base64?) string in the environment. Then instead of parsing out the header, footer and body, we can just add the ASCII-armour to the output and bypass all the complicated splitting code. Does that makes sense? I can give a longer example if you like but that leads me to...

Secondly, I want to have scenario coverage for all new features, so that we can maintain quality through regression testing and also gain documentation. Take a look in the "features" directory for examples. I think you'll want a new feature file with something like this:

Scenario: Format RSA keys with ASCII armour
  Given a file named "Rakefile" with:
    """
    require 'dice_bag/tasks'
    """
  And a file named "private_key.dice" with:
    """
    <%= ensure_is_private_key(configured.private_key) %>
    """
  When I run `rake PRIVATE_KEY=abcdef123456... config`
  Then the file "private_key" should contain:
    """
    -----BEGIN RSA PRIVATE KEY-----
    abcdef123456...
    -----END RSA PRIVATE KEY-----
    """

Please ensure it passes by running the scenarios. I know we don't have this documented yet but it should just be a case of running cucumber. I'll add instructions to the repo if you encounter any difficulties.

Thirdly, make sure you add yourself to the list of contributors in the README! :smile:

harryw commented 11 years ago

@asmith-mdsol From the looks of it, this is not just for formatting the header and footer, it's for reformatting an entire multi-line key that's been provided on a single line.

This is artifact of the fact that Medistrano's config UI doesn't allow you to enter multiline strings. So we have a lot of private keys entered as single-line strings (although not all of them), and that's been dealt with so far by reformatting private keys on deployment.

The config UI will have to change at some point but this seems like a reasonable and well-established workaround for now.

asmith-mdsol commented 11 years ago

@harryw I had a chat with @lgreene-mdsol yesterday and he talked me around to dealing with both cases because they're out there in the wild and we can't ignore that fact. I asked him to refactor the formatting code to be simpler by stripping out the header and footer as fixed strings (e.g. .gsub(/\A-----BEGIN RSA PRIVATE KEY-----/, '')) before putting them back in the output, so that we'll handle all combinations of single vs multi-line and armoured vs non-armoured.

lgreene-mdsol commented 11 years ago

@asmith-mdsol Made the change and created the ff, ran it and it all passes

asmith-mdsol commented 11 years ago

Verified that the scenarios all pass locally. Thanks @lgreene-mdsol, that's so much simpler and robust now, plus the scenarios nicely explain what's going on.