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

feat: add Granular OAuth Scopes #169

Merged
merged 55 commits into from
Sep 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
c0404ba
feat: add scope checking
tassja Aug 10, 2021
70824f8
fix: return array
tassja Aug 10, 2021
9245931
feat: add scopes check to all controllers
tassja Aug 10, 2021
a91b955
feat: add scope read & write functions
tassja Aug 12, 2021
cf345f1
Merge branch 'master' into ouath-scope
tassja Aug 12, 2021
05ed5f6
style: remove useless assignments
tassja Aug 13, 2021
611a67a
refactor: use question methods from enum
tassja Aug 13, 2021
99ae0cd
Merge branch 'master' into ouath-scope
tassja Aug 13, 2021
c9bff6d
style(helper): add default arg
tassja Aug 13, 2021
a9ebd5b
perf: add scopes to auth user
tassja Aug 13, 2021
03250ad
fix: remove double assignment
tassja Aug 13, 2021
18a1ab5
style: rename method calls
tassja Aug 13, 2021
cfd971f
style(helper): improve effciency
tassja Aug 13, 2021
f5ab60c
style(helper): improve user assignment
tassja Aug 13, 2021
d063a52
refactor: update to use new guest check method
tassja Aug 16, 2021
0228ed4
refactor: imrpove scope check for guests
tassja Aug 16, 2021
6595a0d
style: rename changed method names
tassja Aug 16, 2021
8bf4685
bump shards
tassja Aug 16, 2021
39dd7b3
fix: incorrect method arguments
tassja Aug 16, 2021
c3fbd47
fix: method arguments
tassja Aug 16, 2021
02dcded
Merge branch 'master' into ouath-scope
tassja Aug 17, 2021
880c43f
test: add macro to test scope on crud routes
tassja Aug 17, 2021
8353ecc
fix: variable name
tassja Aug 17, 2021
bdd1378
test: call scopes test in each controller
tassja Aug 17, 2021
2658e34
style: remove Useless assignment
tassja Aug 17, 2021
9a6e358
chore: bump version
tassja Aug 17, 2021
25f1d3f
test: add spec for scopes for update route
tassja Aug 18, 2021
faa6e8e
test(metadata): add unique metadata spec
tassja Aug 18, 2021
5092b4c
stlye: clean up format
tassja Aug 18, 2021
9117787
refactor: add abstract def controller scope
tassja Aug 18, 2021
a4c6e5f
style(current-user): add back new lines
tassja Aug 18, 2021
82caeb2
refactor: turn can_guest_read into abstract
tassja Aug 18, 2021
3a273cf
refactor(systems): ensure all routes are scoped
tassja Aug 18, 2021
0d7edb3
fix(systems): method name
tassja Aug 18, 2021
f836fdf
test(systems): add start scope test
tassja Aug 19, 2021
8541aaa
feat: add available scopes endpoint
tassja Aug 19, 2021
bc40fca
feat: autogenerate scope checks
caspiano Aug 19, 2021
4e57b69
refactor(api:application): remove scopes code
caspiano Aug 19, 2021
85909cf
refactor(current-user): use correct method
tassja Aug 19, 2021
607cd86
refactor(utils:scopes): move scopes into module
caspiano Aug 20, 2021
df142de
fix(utils:scopes): check access inclusion rather than equality
caspiano Aug 20, 2021
4fc9f19
fix(scopes): add verbatim
tassja Aug 23, 2021
34de286
fix(scopes): correct access for multiple scopes
tassja Aug 25, 2021
c0cca02
style(scopes): ignore Ameba
tassja Aug 29, 2021
708977a
fix(scopes): correct constant assignment
tassja Aug 31, 2021
d9ad853
fix(scopes): resolve path
tassja Aug 31, 2021
9b5fed1
refactor(scopes): add macro for update spec
tassja Aug 31, 2021
7bb1a46
refactor(scopes): improve macro in spec
tassja Sep 5, 2021
55ee002
fix(root): lazy getter for scopes endpoint
tassja Sep 6, 2021
d0de93d
test(root): improve scope endpoint test
tassja Sep 6, 2021
2d7c80c
Merge branch 'master' into ouath-scope
tassja Sep 6, 2021
44105f0
refactor(scopes): ensure checks called statically
tassja Sep 8, 2021
b936101
docs(scopes): add example
tassja Sep 9, 2021
76c40b9
docs(scopes): update comment
tassja Sep 9, 2021
33ce341
Merge branch 'master' into ouath-scope
tassja Sep 16, 2021
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
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

tassja marked this conversation as resolved.
Show resolved Hide resolved
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