Skip to content

Commit

Permalink
Add some default options, and move config.
Browse files Browse the repository at this point in the history
  • Loading branch information
Erich Menge committed Apr 17, 2013
1 parent 8004292 commit 2a044f6
Show file tree
Hide file tree
Showing 11 changed files with 140 additions and 85 deletions.
29 changes: 14 additions & 15 deletions README.md
Expand Up @@ -40,7 +40,6 @@ end
```

That's it. You're done. Need to add a field? Pop it in the form. You don't need to then update a list of attributes.
`signed_form_for` works just like the standard `form_for`.

Of course, you're free to continue using the standard `form_for`. `SignedForm` is strictly opt-in. It won't change the
way you use standard forms.
Expand Down Expand Up @@ -81,7 +80,7 @@ end

You'll also need to create an initializer:

$ echo 'SignedForm::HMAC.secret_key = SecureRandom.hex(64)' > config/initializers/signed_form.rb
$ echo 'SignedForm.secret_key = SecureRandom.hex(64)' > config/initializers/signed_form.rb

**IMPORTANT** Please read below for information regarding this secret key.

Expand Down Expand Up @@ -109,21 +108,21 @@ you can decide what is right for you with respect to the secret key.
Take for example the case where you have an administrative backend. You might have `/admin/users/edit`. Users can also
change some information about themselves though, so there's `/users/edit` as well. Now you have an admin that gets
demoted, but still has a user account. If that admin were to retain a form signature from `/admin/users/edit` they could
use that signature to modify the same fields from `/users/edit`. As a means of preventing such access SignedForm provides
the `sign_destination` option to `signed_form_for`. Example:
use that signature to modify the same fields from `/users/edit`. As a means of preventing such access SignedForm signs
the form destination by default. With `sign_destination` enabled, a form generated with a destination of
`/admin/users/5` for example will only be accepted at that end point. The form would not be accepted at `/users/5`. So
in the event you would like to use SignedForm on forms for the same resource, but different access levels, you have
protection against the form being used elsewhere.

``` erb
<%= signed_form_for(@user, sign_destination: true) do |f| %>
<%= f.text_field :name %>
<!-- ... -->
<% end %>
```

With `sign_destination` enabled, a form generated with a destination of `/admin/users/5` for example will only be
accepted at that end point. The form would not be accepted at `/users/5`. So in the event you would like to use
SignedForm on forms for the same resource, but different access levels, you have protection against the form being used
elsewhere.
This access protection can be turned off on an indivdual form basis: `signed_form_for(@user, sign_destination: false)`
or globally:

``` ruby
SignedForm.config do |c|
c.secret_key = SecureRandom.hex(64)
c.options[:sign_destination] = false
end
```
### Caching

Another consideration to be aware of is caching. If you cache a form, and then change the secret key that form will
Expand Down
28 changes: 28 additions & 0 deletions lib/signed_form.rb
Expand Up @@ -7,3 +7,31 @@
require "signed_form/hmac"
require "signed_form/action_view/form_helper"
require "signed_form/action_controller/permit_signed_params"

module SignedForm
DEFAULT_OPTIONS = {
sign_destination: true
}.freeze

class << self
attr_reader :secret_key
def secret_key=(key)
@secret_key = key
hmac.secret_key = key if @hmac
end

attr_writer :options
def options
@options ||= DEFAULT_OPTIONS.dup
end

attr_writer :hmac
def hmac
@hmac ||= SignedForm::HMAC.new(secret_key: secret_key)
end

def config
yield self
end
end
end
4 changes: 2 additions & 2 deletions lib/signed_form/action_controller/permit_signed_params.rb
Expand Up @@ -19,10 +19,10 @@ def permit_signed_form_data

signature ||= ''

raise Errors::InvalidSignature, "Form signature is not valid" unless SignedForm::HMAC.verify_hmac signature, data
raise Errors::InvalidSignature, "Form signature is not valid" unless SignedForm.hmac.verify signature, data

allowed_attributes = Marshal.load Base64.strict_decode64(data)
options = allowed_attributes.delete(:__options__)
options = allowed_attributes.delete(:_options_)

raise Errors::InvalidURL if options && (!options[:method].to_s.casecmp(request.method) || options[:url] != request.fullpath)

Expand Down
5 changes: 3 additions & 2 deletions lib/signed_form/action_view/form_helper.rb
Expand Up @@ -6,9 +6,10 @@ module FormHelper
#
# @option options :sign_destination [Boolean] Only the URL given/created will be allowed to receive the form.
def signed_form_for(record, options = {}, &block)
options[:builder] ||= SignedForm::FormBuilder
options_for_form = SignedForm.options.merge options
options_for_form[:builder] ||= SignedForm::FormBuilder

form_for(record, options) do |f|
form_for(record, options_for_form) do |f|
output = capture(f, &block)
f.form_signature_tag + output
end
Expand Down
20 changes: 10 additions & 10 deletions lib/signed_form/form_builder.rb
Expand Up @@ -27,19 +27,19 @@ module Methods

def initialize(*)
super
if options[:signed_attributes_object]
self.signed_attributes_object = options[:signed_attributes_object]
if options[:signed_attributes_context]
@signed_attributes_context = options[:signed_attributes_context]
else
self.signed_attributes = { object_name => [] }
self.signed_attributes_object = signed_attributes[object_name]
@signed_attributes = { object_name => [] }
@signed_attributes_context = signed_attributes[object_name]
end
end

def form_signature_tag
signed_attributes.each { |k,v| v.uniq! if v.is_a?(Array) }
signed_attributes[:__options__] = { method: options[:html][:method], url: options[:url] } if options[:sign_destination]
signed_attributes[:_options_] = { method: options[:html][:method], url: options[:url] } if options[:sign_destination]
encoded_data = Base64.strict_encode64 Marshal.dump(signed_attributes)
signature = SignedForm::HMAC.create_hmac(encoded_data)
signature = SignedForm.hmac.create(encoded_data)
token = "#{encoded_data}--#{signature}"
%(<input type="hidden" name="form_signature" value="#{token}" />\n).html_safe
end
Expand All @@ -52,9 +52,9 @@ def fields_for(record_name, record_object = nil, fields_options = {}, &block)
array = []

if nested_attributes_association?(record_name)
hash["#{record_name}_attributes"] = fields_options[:signed_attributes_object] = array
hash["#{record_name}_attributes"] = fields_options[:signed_attributes_context] = array
else
hash[record_name] = fields_options[:signed_attributes_object] = array
hash[record_name] = fields_options[:signed_attributes_context] = array
end

add_signed_fields hash
Expand All @@ -72,12 +72,12 @@ def fields_for(record_name, record_object = nil, fields_options = {}, &block)
# <% end %>
#
def add_signed_fields(*fields)
signed_attributes_object.push(*fields)
signed_attributes_context.push(*fields)
end

private

attr_accessor :signed_attributes, :signed_attributes_object
attr_reader :signed_attributes, :signed_attributes_context
end

include Methods
Expand Down
30 changes: 18 additions & 12 deletions lib/signed_form/hmac.rb
@@ -1,25 +1,31 @@
require 'openssl'
require "openssl"

module SignedForm
module HMAC
extend self

class HMAC
attr_accessor :secret_key
attr_accessor :digest

def create_hmac(data)
if secret_key.nil? || secret_key.empty?
raise Errors::NoSecretKey, "Please consult the README for instructions on creating a secret key"
end

OpenSSL::HMAC.hexdigest OpenSSL::Digest::SHA1.new, secret_key, data
def self.secret_key=(key)
SignedForm.secret_key = key
warn "SignedForm::HMAC.secret_key is depreciated and will be removed in the next release. "\
"Please use SignedForm.secret_key instead."
end

def verify_hmac(signature, data)
def initialize(options = {})
self.secret_key = options[:secret_key]
self.digest = options.fetch(:digest, OpenSSL::Digest::SHA1.new)

if secret_key.nil? || secret_key.empty?
raise Errors::NoSecretKey, "Please consult the README for instructions on creating a secret key"
end
end

def create(data)
OpenSSL::HMAC.hexdigest digest, secret_key, data
end

secure_compare OpenSSL::HMAC.hexdigest(OpenSSL::Digest::SHA1.new, secret_key, data), signature
def verify(signature, data)
secure_compare OpenSSL::HMAC.hexdigest(digest, secret_key, data), signature
end

private
Expand Down
37 changes: 24 additions & 13 deletions spec/form_builder_spec.rb
Expand Up @@ -23,8 +23,7 @@ def persisted?
describe SignedForm::FormBuilder do
include SignedFormViewHelper

before { SignedForm::HMAC.secret_key = "abc123" }
after { SignedForm::HMAC.secret_key = nil }
before { SignedForm.secret_key = "abc123" }

let(:user) { User.new }
let(:widget) { Widget.new }
Expand All @@ -43,17 +42,31 @@ def persisted?
content.should =~ Regexp.new(regex, Regexp::MULTILINE)
end

it "should set a target" do
content = signed_form_for(User.new, sign_destination: true) do |f|
f.text_field :name
describe "sign_destination" do
after do
@data.should include(:_options_)
@data[:_options_].should include(:method, :url)
@data[:_options_][:method].should == :post
@data[:_options_][:url].should == '/users'
end

data = get_data_from_form(content)
data.size.should == 2
data.should include(:__options__)
data[:__options__].should include(:method, :url)
data[:__options__][:method].should == :post
data[:__options__][:url].should == '/users'
it "should set a target" do
content = signed_form_for(User.new, sign_destination: true) do |f|
f.text_field :name
end

@data = get_data_from_form(content)
end

it "should set a target when the default options are enabled" do
SignedForm.options[:sign_destination] = true

content = signed_form_for(User.new) do |f|
f.text_field :name
end

@data = get_data_from_form(content)
end
end
end

Expand All @@ -72,8 +85,6 @@ def persisted?
:datetime_select, :time_zone_select, :date_select]

after do
@data.should be
@data.size.should == 1
@data['user'].size.should == 1
@data['user'].should include(:name)
end
Expand Down
36 changes: 13 additions & 23 deletions spec/hmac_spec.rb
@@ -1,34 +1,24 @@
require 'spec_helper'

describe SignedForm::HMAC do
describe 'create_hmac' do
it 'should raise if no key is given' do
expect { SignedForm::HMAC.create_hmac "foo" }.to raise_error(SignedForm::Errors::NoSecretKey)
end
it 'should raise if no key is given' do
expect { SignedForm::HMAC.new }.to raise_error(SignedForm::Errors::NoSecretKey)
end

context 'when a key is present' do
before { SignedForm::HMAC.secret_key = "superdupersecret" }
after { SignedForm::HMAC.secret_key = nil }
describe 'create' do
let(:hmac) { SignedForm::HMAC.new(secret_key: "superdupersecret") }

it 'should create a hex signature' do
SignedForm::HMAC.create_hmac("my signed message").length.should == 40
end
it 'should create a hex signature' do
hmac.create("my signed message").length.should == 40
hmac.create("my signed message").should == "93c1ecd4c10122cbf873ca6cf9eff08888565054"
end
end

describe 'verify_hmac' do
it 'should raise if no key is given' do
expect { SignedForm::HMAC.verify_hmac 'foo', 'bar' }.to raise_error(SignedForm::Errors::NoSecretKey)
end

context 'when a key is present' do
before { SignedForm::HMAC.secret_key = "superdupersecret" }
after { SignedForm::HMAC.secret_key = nil }

let(:signature) { SignedForm::HMAC.create_hmac "My super secret" }
describe 'verify' do
let(:hmac) { SignedForm::HMAC.new(secret_key: "superdupersecret") }
let(:signature) { hmac.create "My super secret" }

specify { SignedForm::HMAC.verify_hmac(signature, "My super secret").should be_true }
specify { SignedForm::HMAC.verify_hmac(signature, "My bad secret").should_not be_true }
end
specify { hmac.verify(signature, "My super secret").should be_true }
specify { hmac.verify(signature, "My bad secret").should_not be_true }
end
end
14 changes: 6 additions & 8 deletions spec/permit_signed_params_spec.rb
Expand Up @@ -10,14 +10,12 @@ class Controller < ActionController::Base
let(:controller) { Controller.new }

before do
SignedForm::HMAC.secret_key = "abc123"
SignedForm.secret_key = "abc123"

Controller.any_instance.stub(request: double('request', method: 'POST', fullpath: '/users'))
Controller.any_instance.stub(params: { "user" => { name: "Erich Menge", occupation: 'developer' } })
end

after { SignedForm::HMAC.secret_key = nil }

it "should raise if signature isn't valid" do
controller.params['form_signature'] = "bad signature"
expect { controller.permit_signed_form_data }.to raise_error(SignedForm::Errors::InvalidSignature)
Expand All @@ -27,7 +25,7 @@ class Controller < ActionController::Base
params = controller.params

data = Base64.strict_encode64(Marshal.dump("user" => [:name]))
signature = SignedForm::HMAC.create_hmac(data)
signature = SignedForm.hmac.create(data)

params['form_signature'] = "#{data}--#{signature}"

Expand All @@ -39,8 +37,8 @@ class Controller < ActionController::Base
it "should verify current url matches targeted url" do
params = controller.params

data = Base64.strict_encode64(Marshal.dump("user" => [:name], :__options__ => { method: 'post', url: '/users' }))
signature = SignedForm::HMAC.create_hmac(data)
data = Base64.strict_encode64(Marshal.dump("user" => [:name], :_options_ => { method: 'post', url: '/users' }))
signature = SignedForm.hmac.create(data)

params['form_signature'] = "#{data}--#{signature}"

Expand All @@ -53,8 +51,8 @@ class Controller < ActionController::Base
it "should reject if url doesn't match" do
params = controller.params

data = Base64.strict_encode64(Marshal.dump("user" => [:name], :__options__ => { method: 'post', url: '/admin' }))
signature = SignedForm::HMAC.create_hmac(data)
data = Base64.strict_encode64(Marshal.dump("user" => [:name], :_options_ => { method: 'post', url: '/admin' }))
signature = SignedForm.hmac.create(data)

params['form_signature'] = "#{data}--#{signature}"

Expand Down
11 changes: 11 additions & 0 deletions spec/signed_form_spec.rb
@@ -0,0 +1,11 @@
require 'spec_helper'

describe SignedForm do
it "should reset the hmac object secret key if the secret key changes" do
SignedForm.secret_key = "foo"

SignedForm.hmac.secret_key.should == "foo"
SignedForm.secret_key = "bar"
SignedForm.hmac.secret_key.should == "bar"
end
end
11 changes: 11 additions & 0 deletions spec/spec_helper.rb
Expand Up @@ -12,6 +12,10 @@

require 'signed_form'

module SignedForm
PRESTINE_MODULE = self.dup
end

module SignedFormViewHelper
include ActionView::Helpers

Expand Down Expand Up @@ -66,4 +70,11 @@ def get_data_from_form(content)
config.filter_run_excluding action_pack: ->(version) { ActionPack::VERSION::STRING.match(/\d+\.\d+/)[0] !~ version }

config.order = 'random'

config.around(:each) do |example|
prestine_module = SignedForm.dup
example.run
Object.send :remove_const, :SignedForm
SignedForm = prestine_module
end
end

0 comments on commit 2a044f6

Please sign in to comment.