Skip to content

Commit

Permalink
feat: add Granular OAuth Scopes (#169)
Browse files Browse the repository at this point in the history
* feat: add scope checking

* fix: return array

* feat: add scopes check to all controllers

* feat: add scope read & write functions

* style: remove useless assignments

* refactor: use question methods from enum

* style(helper): add default arg

* perf: add scopes to auth user

* fix: remove double assignment

* style: rename method calls

* style(helper): improve effciency

* style(helper): improve  user assignment

* refactor: update to use new guest check method

* refactor: imrpove scope check for guests

* style: rename changed method names

* bump shards

* fix: incorrect method arguments

* fix: method arguments

* test: add macro to test scope on crud routes

* fix: variable name

* test: call scopes test in each controller

* style: remove Useless assignment

* chore: bump version

* test: add spec for scopes for update route

* test(metadata): add unique metadata spec

* stlye: clean up format

* refactor: add abstract def controller scope

* style(current-user): add back new lines

* refactor: turn can_guest_read into abstract

* refactor(systems): ensure all routes are scoped

* fix(systems): method name

* test(systems): add start scope test

* feat: add available scopes endpoint

* feat: autogenerate scope checks

* refactor(api:application): remove scopes code

* refactor(current-user): use correct method

* refactor(utils:scopes): move scopes into module

- Fixed compilation issues with scopes
- Seperated callbacks and scope checks in each controller
- Removed a few redundant includes of `Utils::CurrentUser`

* fix(utils:scopes): check access inclusion rather than equality

* fix(scopes): add verbatim

* fix(scopes): correct access for multiple scopes

* style(scopes): ignore Ameba

* fix(scopes): correct constant assignment

* fix(scopes): resolve path

* refactor(scopes): add macro for update spec

* refactor(scopes): improve macro in spec

* fix(root): lazy getter for scopes endpoint

* test(root): improve scope endpoint test

* refactor(scopes): ensure  checks called statically

* docs(scopes): add example

* docs(scopes): update comment

Co-authored-by: Caspian Baska <email@caspian.computer>
  • Loading branch information
tassja and caspiano committed Sep 16, 2021
1 parent 586b71b commit b1e5698
Show file tree
Hide file tree
Showing 40 changed files with 818 additions and 22 deletions.
1 change: 0 additions & 1 deletion shard.lock
Original file line number Diff line number Diff line change
Expand Up @@ -228,4 +228,3 @@ shards:
yaml_mapping:
git: https://github.com/crystal-lang/yaml_mapping.cr.git
version: 0.1.1

2 changes: 1 addition & 1 deletion shard.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: placeos-rest-api
version: 1.29.3
version: 1.30.0
crystal: ~> 1

targets:
Expand Down
5 changes: 5 additions & 0 deletions spec/api_key_spec.cr
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require "./helper"
require "./scope_helper"

module PlaceOS::Api
describe ApiKeys do
Expand All @@ -16,6 +17,10 @@ module PlaceOS::Api
describe "CRUD operations", tags: "crud" do
test_crd(Model::ApiKey, ApiKeys)
end

describe "scopes" do
test_controller_scope(ApiKeys)
end
end
end
end
6 changes: 6 additions & 0 deletions spec/brokers_spec.cr
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require "./helper"
require "./scope_helper"

module PlaceOS::Api
describe Brokers do
Expand Down Expand Up @@ -36,6 +37,11 @@ module PlaceOS::Api
updated.name.should_not eq original_name
end
end

describe "scopes" do
test_update_write_scope(Brokers)
test_controller_scope(Brokers)
end
end
end
end
5 changes: 5 additions & 0 deletions spec/drivers_spec.cr
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require "./helper"
require "./scope_helper"

module PlaceOS::Api
describe Drivers do
Expand Down Expand Up @@ -78,6 +79,10 @@ module PlaceOS::Api
end
end
end

describe "scopes" do
test_controller_scope(Drivers)
end
end
end
end
5 changes: 5 additions & 0 deletions spec/edges_spec.cr
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require "./helper"
require "./scope_helper"

module PlaceOS::Api
describe Edges do
Expand All @@ -16,6 +17,10 @@ module PlaceOS::Api
describe "CRUD operations", tags: "crud" do
test_crd(Model::Edge, Edges)
end

describe "scopes" do
test_controller_scope(Edges)
end
end
end
end
13 changes: 13 additions & 0 deletions spec/helper.cr
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,19 @@ def authentication(sys_admin : Bool = true, support : Bool = true, scope = [Plac
end
end

def generate_auth_user(sys_admin, support, scopes)
scope_list = scopes.try &.join('-', &.to_s)
test_user_email = "test-#{"admin-" if sys_admin}#{"supp" if support}-scope-#{scope_list}-rest-api@place.tech"
existing = PlaceOS::Model::User.where(email: test_user_email).first?

existing || PlaceOS::Model::Generator.user.tap do |user|
user.email = test_user_email
user.sys_admin = sys_admin
user.support = support
user.save!
end
end

# Check application responds with 404 when model not present
def test_404(base, model_name, headers)
it "404s if #{model_name} isn't present in database", tags: "search" do
Expand Down
142 changes: 142 additions & 0 deletions spec/metadata_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,148 @@ module PlaceOS::Api
metadata.size.should eq 1
end
end

describe "scopes" do
context "read" do
scope_name = "metadata"

it "allows access to show" do
_, authorization_header = authentication(scope: [PlaceOS::Model::UserJWT::Scope.new(scope_name, :read)])

parent = Model::Generator.zone.save!
parent_id = parent.id.as(String)

3.times do
child = Model::Generator.zone
child.parent_id = parent_id
child.save!
Model::Generator.metadata(parent: child.id).save!
end

result = curl(
method: "GET",
path: "#{base}/#{parent_id}/children",
headers: authorization_header,
)
result.status_code.should eq 200
Array(NamedTuple(zone: JSON::Any, metadata: Hash(String, Model::Metadata::Interface)))
.from_json(result.body)
.tap { |m| m.size.should eq(3) }
.count(&.[:metadata].empty?.!)
.should eq 3

parent.destroy
end

it "should not allow access to delete" do
_, authorization_header = authentication(scope: [PlaceOS::Model::UserJWT::Scope.new(scope_name, :read)])

parent = Model::Generator.zone.save!
parent_id = parent.id.as(String)

3.times do
child = Model::Generator.zone
child.parent_id = parent_id
child.save!
Model::Generator.metadata(parent: child.id).save!
end

id = parent.id.as(String)

result = curl(
method: "DELETE",
path: "#{base}/#{id}",
headers: authorization_header,
)
result.status_code.should eq 403
end
end
context "write" do
scope_name = "metadata"

it "should allow access to update" do
_, authorization_header = authentication(scope: [PlaceOS::Model::UserJWT::Scope.new(scope_name, :write)])

parent = Model::Generator.zone.save!
meta = Model::Metadata::Interface.new(
name: "test",
description: "",
details: JSON.parse(%({"hello":"world","bye":"friends"})),
parent_id: nil,
editors: Set(String).new,
)

parent_id = parent.id.as(String)
path = "#{base}/#{parent_id}"

result = curl(
method: "PUT",
path: path,
body: meta.to_json,
headers: authorization_header.merge({"Content-Type" => "application/json"}),
)

new_metadata = Model::Metadata::Interface.from_json(result.body)
found = Model::Metadata.for(parent_id, meta.name).first
found.name.should eq new_metadata.name
end

it "should not allow access to show" do
_, authorization_header = authentication(scope: [PlaceOS::Model::UserJWT::Scope.new(scope_name, :write)])

parent = Model::Generator.zone.save!
parent_id = parent.id.as(String)

3.times do
child = Model::Generator.zone
child.parent_id = parent_id
child.save!
Model::Generator.metadata(parent: child.id).save!
end

result = curl(
method: "GET",
path: "#{base}/#{parent_id}/children",
headers: authorization_header,
)
result.status_code.should eq 403
end
end

it "checks that guests can read metadata" do
_, authorization_header = authentication(scope: [PlaceOS::Model::UserJWT::Scope.new("guest", PlaceOS::Model::UserJWT::Scope::Access::Read)])

zone = Model::Generator.zone.save!
zone_id = zone.id.as(String)
meta = Model::Generator.metadata(name: "special", parent: zone_id).save!

result = curl(
method: "GET",
path: "#{base}/#{zone_id}",
headers: authorization_header,
)

result.status_code.should eq 200
metadata = Hash(String, Model::Metadata::Interface).from_json(result.body)
metadata.size.should eq 1
metadata.values.first.parent_id.should eq zone_id
metadata.values.first.name.should eq meta.name
end

it "checks that guests cannot write metadata" do
_, authorization_header = authentication(scope: [PlaceOS::Model::UserJWT::Scope.new("guest", PlaceOS::Model::UserJWT::Scope::Access::Read)])

zone = Model::Generator.zone.save!
zone_id = zone.id.as(String)

result = curl(
method: "POST",
path: "#{base}/#{zone_id}",
headers: authorization_header,
)
result.success?.should be_false
end
end
end
end
end
30 changes: 30 additions & 0 deletions spec/modules_spec.cr
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require "./helper"
require "./scope_helper"
require "timecop"

module PlaceOS
Expand Down Expand Up @@ -315,6 +316,35 @@ module PlaceOS::Api
result.success?.should be_true
body["pingable"].should be_true
end

describe "scopes" do
test_controller_scope(Modules)

it "checks scope on update" do
_, authorization_header = authentication(scope: [PlaceOS::Model::UserJWT::Scope.new("modules", PlaceOS::Model::UserJWT::Scope::Access::Write)])
driver = Model::Generator.driver(role: Model::Driver::Role::Service).save!
mod = Model::Generator.module(driver: driver).save!

connected = mod.connected
mod.connected = !connected

id = mod.id.as(String)
path = base + id

result = update_route(path, mod, authorization_header)

result.status_code.should eq 200
updated = Model::Module.from_trusted_json(result.body)
updated.id.should eq mod.id
updated.connected.should eq !connected

_, authorization_header = authentication(scope: [PlaceOS::Model::UserJWT::Scope.new("modules", PlaceOS::Model::UserJWT::Scope::Access::Read)])
result = update_route(path, mod, authorization_header)

result.success?.should be_false
result.status_code.should eq 403
end
end
end
end
end
6 changes: 6 additions & 0 deletions spec/repositories_spec.cr
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require "./helper"
require "./scope_helper"

module PlaceOS::Api
describe Repositories do
Expand Down Expand Up @@ -99,6 +100,11 @@ module PlaceOS::Api
end
end
end

describe "scopes" do
test_controller_scope(Repositories)
test_update_write_scope(Repositories)
end
end
end
end
6 changes: 6 additions & 0 deletions spec/root_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ module PlaceOS::Api
response.commit.should eq BUILD_COMMIT
end

it "gets scope names" do
result = curl("GET", File.join(base, "scopes"), headers: authorization_header)
scopes = Array(String).from_json(result.body)
scopes.size.should eq(Root.scopes.size)
end

it "constructs service versions" do
WebMock.allow_net_connect = false

Expand Down
Loading

0 comments on commit b1e5698

Please sign in to comment.