Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Private Devices #148

Merged
merged 15 commits into from
Sep 3, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions app/controllers/v0/devices_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@ def show

def index
if params[:with_tags]
@q = Device.with_user_tags(params[:with_tags])
@q = policy_scope(Device)
.with_user_tags(params[:with_tags])
.includes(:owner,:tags, kit: [:sensors, :components])
.ransack(params[:q])
else
@q = Device.includes(:owner, :tags, kit: [:components, :sensors])
@q = policy_scope(Device)
.includes(:owner, :tags, kit: [:components, :sensors])
.ransack(params[:q])
end

Expand Down Expand Up @@ -82,6 +84,7 @@ def fresh_world_map
longitude: device.longitude,
city: device.city,
country_code: device.country_code,
is_private: device.is_private,
kit_id: device.kit_id,
state: device.state,
system_tags: device.system_tags,
Expand Down Expand Up @@ -124,7 +127,7 @@ def world_map
private

def device_params
params.permit(
params_to_permit = [
:name,
:description,
:mac_address,
Expand All @@ -138,6 +141,15 @@ def device_params
:meta,
:kit_id,
:user_tags
]

# Researchers + Admins can update is_private
if current_user.role_mask >= 2
params_to_permit.push(:is_private)
end

params.permit(
params_to_permit
)
end

Expand Down
1 change: 1 addition & 0 deletions app/controllers/v0/static_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ def metrics
render json: {
devices: {
total: Device.count,
private: Device.where(is_private: true).count,
online: {
now: Device.where('last_recorded_at > ?', 10.minutes.ago).count,
last_hour: Device.where('last_recorded_at > ?', 1.hour.ago).count,
Expand Down
1 change: 1 addition & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ def is_admin?
end

def role
return 'researcher' if role_mask == 2
role_mask < 5 ? 'citizen' : 'admin'
end

Expand Down
29 changes: 28 additions & 1 deletion app/policies/device_policy.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,34 @@
class DevicePolicy < ApplicationPolicy
class Scope
attr_reader :user, :scope

def initialize(user, scope)
@user = user
@scope = scope
end

def resolve
if user
if user.is_admin?
# Admins get everything
scope
else
# Non admins should get all non_private + the Devices they own
scope.where(is_private: false).or(scope.where(owner_id: user.id))
end
else
# not logged in
scope.where(is_private: false)
end
end
end

def show?
true
if record.is_private?
update?
else
true
end
end

def update?
Expand Down
1 change: 1 addition & 0 deletions app/views/v0/devices/_device.jbuilder
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ json.(device,
:hardware_info,
:system_tags,
:user_tags,
:is_private,
:notify_low_battery,
:notify_stopped_publishing,
:last_reading_at,
Expand Down
5 changes: 5 additions & 0 deletions db/migrate/20190819084816_add_is_private_to_devices.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddIsPrivateToDevices < ActiveRecord::Migration[5.2]
def change
add_column :devices, :is_private, :boolean, default: false
end
end
3 changes: 2 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 2019_02_22_130041) do
ActiveRecord::Schema.define(version: 2019_08_19_084816) do

# These are extensions that must be enabled in order to support this database
enable_extension "adminpack"
Expand Down Expand Up @@ -93,6 +93,7 @@
t.datetime "notify_low_battery_timestamp", default: "2019-01-16 16:19:35"
t.boolean "notify_low_battery", default: false
t.boolean "notify_stopped_publishing", default: false
t.boolean "is_private", default: false
t.index ["device_token"], name: "index_devices_on_device_token", unique: true
t.index ["geohash"], name: "index_devices_on_geohash"
t.index ["kit_id"], name: "index_devices_on_kit_id"
Expand Down
12 changes: 8 additions & 4 deletions db/seeds.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
3.times do
Kit.create(
name: "Kit #{Faker::Educator.campus}",
description: Faker::Lorem.sentence(5),
description: Faker::Lorem.sentence,
slug: 'sck:1,1',
sensor_map: {"temp": 12, "hum": 13, "light": 14}
)
Expand Down Expand Up @@ -89,10 +89,11 @@

#device has many sensors through components
#has_many :components, as: :board
5.times do

10.times do
Device.create(
{
owner: User.first,
owner: User.all.sample,
name: Faker::Address.city,
city: Faker::Address.city,
country_code: Faker::Address.country_code,
Expand All @@ -102,6 +103,7 @@
latitude: 42.385,
longitude: 2.173,
device_token: Faker::Crypto.sha1[0,6],
is_private: [true, false].sample,
notify_low_battery: [true, false].sample,
notify_low_battery_timestamp: Time.now,
notify_stopped_publishing: [true, false].sample,
Expand All @@ -128,6 +130,7 @@
# belongs_to :sensor
# Kit and Device have many Components, as: :board


Component.create(
board: Kit.first, sensor: Sensor.find(12)
)
Expand Down Expand Up @@ -181,7 +184,8 @@
report: {"random_property":"random_result"},
)

Device.find(1).update_attributes(
d = Device.first
d.update!(
hardware_info: {
"id": 1,
"uuid": "7d45fead-defd-4482-bc6a-a1b711879e2d",
Expand Down
74 changes: 72 additions & 2 deletions spec/requests/v0/devices_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

let(:application) { create :application }
let(:user) { create :user }
let(:user2) { create :user }
let(:token) { create :access_token, application: application, resource_owner_id: user.id }
let(:device) { create(:device) }
let(:admin) { create :admin }
Expand All @@ -24,7 +25,49 @@
# expect(json[0]['name']).to eq(first.name)
# expect(json[1]['name']).to eq(second.name)
expect(json[0].keys).to eq(%w(id uuid name description state
hardware_info system_tags user_tags notify_low_battery notify_stopped_publishing last_reading_at added_at updated_at mac_address owner data kit))
hardware_info system_tags user_tags is_private notify_low_battery notify_stopped_publishing last_reading_at added_at updated_at mac_address owner data kit))
end

describe "when not logged in" do
it 'does not show private devices' do
device = create(:device, owner: user, is_private: false)
device1 = create(:device, owner: user, is_private: true)
device2 = create(:device, owner: user, is_private: true)

expect(Device.count).to eq(3)
j = api_get "devices/"
expect(j.count).to eq(1)
expect(response.status).to eq(200)
expect(j[0]['id']).to eq(device.id)
end
end

describe "when logged in as a normal user" do
it 'shows the user his devices, even though they are private' do
device1 = create(:device, owner: user, is_private: false)
device2 = create(:device, owner: user, is_private: true)
device3 = create(:device, owner: user2, is_private: true)

expect(Device.count).to eq(3)
j = api_get "devices/", { access_token: token.token }
expect(j[0]['id']).to eq(device1.id)
expect(response.status).to eq(200)
expect(j.count).to eq(2)
end
end

describe "when logged in as an admin" do
it 'allows admin to see ALL devices' do
device1 = create(:device, owner: user, is_private: false)
device2 = create(:device, owner: user, is_private: true)
device3 = create(:device, owner: user2, is_private: true)

expect(Device.count).to eq(3)
j = api_get "devices/", {access_token: admin_token.token}
expect(response.status).to eq(200)
expect(j[0]['id']).to eq(device1.id)
expect(j.count).to eq(3)
end
end

describe "world map" do
Expand Down Expand Up @@ -87,6 +130,21 @@
expect(response.status).to eq(404)
end

it 'does not show a private device' do
device = create(:device, owner: user, is_private: true)
j = api_get "devices/#{device.id}"
expect(j['id']).to eq("forbidden")
expect(response.status).to eq(403)
end

it 'shows a non_private device' do
device = create(:device, owner: user, is_private: false)
j = api_get "devices/#{device.id}"
expect(j['id']).to eq(device.id)
expect(response.status).to eq(200)
end


describe "mac_address" do

it "filters mac address from guests" do
Expand Down Expand Up @@ -118,6 +176,19 @@

let!(:device) { create :device, owner: user }

it "cannot update a device is_private attribute" do
api_put "devices/#{device.id}", { is_private: true, access_token: token.token }
expect(response.status).to eq(200)
expect(Device.find(device.id).is_private).to eq(false)
end

it "can update a device is_private attribute when user has role" do
user.update role_mask: 3
api_put "devices/#{device.id}", { is_private: true, access_token: token.token }
expect(response.status).to eq(200)
expect(Device.find(device.id).is_private).to eq(true)
end

it "updates a device" do
api_put "devices/#{device.id}", { name: 'new name', access_token: token.token }
expect(response.status).to eq(200)
Expand All @@ -141,7 +212,6 @@
end

describe "POST /devices" do

it "creates a device" do
api_post 'devices', {
access_token: token.token,
Expand Down