Skip to content

Commit

Permalink
fixes #5235: filter sensitive attributes from ui and exports
Browse files Browse the repository at this point in the history
- adds a config filter_attributes that will remove definied attributes from a resource's default_attributes
- by default those attributes include encrypted_password (devise!) as well as password and password_confirmation as a good guess
- removes filtered attributes from default forms
- adds feature specs for filtered attributes except JSON export
- uses except option over Formtastics buggy skipped_columns config
  • Loading branch information
chrp authored and chrp committed Oct 18, 2018
1 parent 787c5d7 commit f5f84af
Show file tree
Hide file tree
Showing 11 changed files with 92 additions and 3 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@

### Enhancements

### Security Fixes

* Prevent leaking hashed passwords via user CSV export and adds a config option for sensitive attributes [#5486][] by [@chrp][]

#### Minor

* Allow proc label in datepicker input [#5408][] by [@tiagotex][]
Expand Down Expand Up @@ -310,6 +314,7 @@ Please check [0-6-stable][] for previous changes.
[#5501]: https://github.com/activeadmin/activeadmin/pull/5501
[#5408]: https://github.com/activeadmin/activeadmin/pull/5408
[#5516]: https://github.com/activeadmin/activeadmin/pull/5516
[#5486]: https://github.com/activeadmin/activeadmin/pull/5486

[@5t111111]: https://github.com/5t111111
[@aarek]: https://github.com/aarek
Expand Down Expand Up @@ -350,3 +355,4 @@ Please check [0-6-stable][] for previous changes.
[@wasifhossain]: https://github.com/wasifhossain
[@wspurgin]: https://github.com/wspurgin
[@zorab47]: https://github.com/zorab47
[@chrp]: https://github.com/chrp
46 changes: 46 additions & 0 deletions features/filter_attributes.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
Feature: Filter Attributes

Filtering sensitive attributes

Background:
Given a configuration of:
"""
ActiveAdmin.register User
"""
Given I am logged in
And a user named "John Doe" exists
And I am on the index page for users

Scenario: Default index page
Then I should not see "Encrypted"
But I should see "Age"

Scenario: Default new form
Given I follow "New User"
Then I should not see "Encrypted"
But I should see "Age"

Scenario: Default edit form
Given I follow "Edit"
Then I should not see "Encrypted"
But I should see "Age"

Scenario: Default show page
Given I follow "View"
Then I should not see "Encrypted"
But I should see "Age"

Scenario: Default CSV export
Given I follow "CSV"
Then I should not see "Encrypted"
But I should see "Age"

# TODO: JSON
# Scenario: Default JSON
# Given I follow "JSON"
# Then I should not see "encrypted"
# But I should see "age"

Scenario: Default XML
Given I follow "XML"
Then I should not see "encrypted"
2 changes: 2 additions & 0 deletions features/support/paths.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ def path_to(page_name)
"/admin"
when /the new post page/
"/admin/posts/new"
when /the new user page/
"/admin/users/new"
when /the login page/
"/admin/login"
when /the first post custom status page/
Expand Down
3 changes: 3 additions & 0 deletions lib/active_admin/application_settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,8 @@ class ApplicationSettings < SettingsNode

# To make debugging easier, by default don't stream in development
register :disable_streaming_in, ['development']

# Remove sensitive attributes from being displayed, made editable, or exported by default
register :filter_attributes, [:encrypted_password, :password, :password_confirmation]
end
end
5 changes: 4 additions & 1 deletion lib/active_admin/resource/attributes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def foreign_methods
end

def reject_col?(c)
primary_col?(c) || sti_col?(c) || counter_cache_col?(c)
primary_col?(c) || sti_col?(c) || counter_cache_col?(c) || filtered_col?(c)
end

def primary_col?(c)
Expand All @@ -39,6 +39,9 @@ def counter_cache_col?(c)
c.name.end_with?('_count')
end

def filtered_col?(c)
ActiveAdmin.application.filter_attributes.include?(c.name.to_sym)
end
end
end
end
6 changes: 6 additions & 0 deletions lib/active_admin/views/components/active_admin_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@ def inputs(*args, &block)
end
insert_tag(SemanticInputsProxy, form_builder, *args, &wrapped_block)
else
# Set except option to prevent sensitive fields from being shown in forms by default.
opts = args.extract_options!
opts[:except] ||= []
ActiveAdmin.application.filter_attributes.each { |e| opts[:except] << e }
args << opts

proxy_call_to_form(:inputs, *args, &block)
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,13 @@ ActiveAdmin.setup do |config|
#
# config.before_action :do_something_awesome

# == Attribute Filters
#
# You can exclude possibly sensitive model attributes from being displayed,
# added to forms, or exported by default by ActiveAdmin
#
config.filter_attributes = [:encrypted_password, :password, :password_confirmation]

# == Localize Date/Time Format
#
# Set the localize format to display dates and times.
Expand Down
2 changes: 1 addition & 1 deletion spec/support/rails_template.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class Blog::Post < ActiveRecord::Base

generate :model, 'profile user_id:integer bio:text'

generate :model, 'user type:string first_name:string last_name:string username:string age:integer'
generate :model, 'user type:string first_name:string last_name:string username:string age:integer encrypted_password:string'
create_file 'app/models/user.rb', <<-RUBY.strip_heredoc, force: true
class User < ActiveRecord::Base
class VIP < self
Expand Down
3 changes: 2 additions & 1 deletion spec/support/rails_template_with_data.rb
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,8 @@
User.create! first_name: first,
last_name: last,
username: [first,last].join('-').downcase,
age: rand(80)
age: rand(80),
encrypted_password: SecureRandom.hex
end
categories = ["Rock", "Pop Rock", "Alt-Country", "Blues", "Dub-Step"].collect do |name|
Expand Down
8 changes: 8 additions & 0 deletions spec/unit/csv_builder_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,14 @@
expect(csv_builder.columns[title_index].name).to eq localized_name
end
end

context 'for models having sensitive attributes' do
let(:resource){ ActiveAdmin::Resource.new(namespace, User, {}) }

it 'omits sensitive fields' do
expect(csv_builder.columns.map(&:data)).to_not include :encrypted_password
end
end
end

context 'when empty' do
Expand Down
7 changes: 7 additions & 0 deletions spec/unit/resource/attributes_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,13 @@ module ActiveAdmin
title: :title,
updated_at: :updated_at)
end

it 'does not return sensitive attributes' do
keep = ActiveAdmin.application.filter_attributes
ActiveAdmin.application.filter_attributes = [:published_date]
expect(subject).to_not include :published_date
ActiveAdmin.application.filter_attributes = keep
end
end

describe "#association_columns" do
Expand Down

0 comments on commit f5f84af

Please sign in to comment.