From b11c0f0ed3da965adb8dabe9dc4af0d13a1f831e Mon Sep 17 00:00:00 2001 From: gcampo88 Date: Mon, 30 Jan 2017 17:30:28 -0500 Subject: [PATCH 01/10] Use the list of account IDs on the token to determine user access to accounts. --- .../api/auth/accounts_controller.rb | 6 ++++- app/models/user.rb | 16 ++++---------- .../api/auth/accounts_controller_test.rb | 22 +++++++++++++++---- test/models/user_test.rb | 10 +++++---- 4 files changed, 33 insertions(+), 21 deletions(-) diff --git a/app/controllers/api/auth/accounts_controller.rb b/app/controllers/api/auth/accounts_controller.rb index caaaefe3..e6730454 100644 --- a/app/controllers/api/auth/accounts_controller.rb +++ b/app/controllers/api/auth/accounts_controller.rb @@ -14,6 +14,10 @@ def resources end def resources_base - @accounts ||= current_user.approved_accounts + @accounts ||= token_accounts + end + + def token_accounts + current_user.approved_accounts = Account.where({ id: prx_auth_token.authorized_resources.keys }) end end diff --git a/app/models/user.rb b/app/models/user.rb index b74a7493..c8b86687 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -15,6 +15,8 @@ class User < BaseModel after_commit :create_individual_account, on: [:create] + attr_accessor :approved_accounts + def individual_account accounts.where('type = \'IndividualAccount\'').first end @@ -45,25 +47,15 @@ def name "#{first_name} #{last_name}" end - def approved_accounts - Account. - joins('LEFT OUTER JOIN `memberships` ON `memberships`.`account_id` = `accounts`.`id`'). - where(['memberships.user_id = ? and memberships.approved is true', id]) - end - def approved_active_accounts approved_accounts.active end def approved_account_stories - Story. - joins('LEFT OUTER JOIN `memberships` ON `memberships`.`account_id` = `pieces`.`account_id`'). - where(['memberships.user_id = ? and memberships.approved is true', id]) + Story.where({ account_id: [approved_accounts] }) end def approved_account_series - Series. - joins('LEFT OUTER JOIN `memberships` ON `memberships`.`account_id` = `series`.`account_id`'). - where(['memberships.user_id = ? and memberships.approved is true', id]) + Series.where({ account_id: [approved_accounts] }) end end diff --git a/test/controllers/api/auth/accounts_controller_test.rb b/test/controllers/api/auth/accounts_controller_test.rb index accd08ad..cb8ea897 100644 --- a/test/controllers/api/auth/accounts_controller_test.rb +++ b/test/controllers/api/auth/accounts_controller_test.rb @@ -3,12 +3,20 @@ describe Api::Auth::AccountsController do let (:user) { create(:user_with_accounts, group_accounts: 2) } - let (:token) { OpenStruct.new.tap { |t| t.user_id = user.id } } + let (:individual_account) { user.individual_account } - let (:member_account) { user.accounts.member.first } - let (:unapproved_account) { user.accounts.member.last } let (:random_account) { create(:account) } - before { unapproved_account.memberships.first.update!(approved: false) } + let (:member_account) { create(:account) } + let (:unapproved_account) { create(:account)} + + let (:token) { OpenStruct.new.tap { |t| + t.authorized_resources = { + member_account.id => "member", + individual_account.id => "admin" + } + t.user_id = user.id + } + } describe 'with a valid token' do @@ -49,6 +57,12 @@ ids.wont_include random_account.id end + it 'gets list of approved accounts for a user from users token' do + get(:index, api_version: 'v1') + assert_response :success + user.reload.approved_accounts.wont_be_nil + end + end describe 'with no token' do diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 44c69fcf..b3709960 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -3,6 +3,8 @@ describe User do let (:user) { create(:user) } let (:network) { create(:network, account: user.individual_account) } + let (:story) { create(:story, account: user.individual_account) } + let (:series) { create(:series, account: user.individual_account) } it 'has a table defined' do User.table_name.must_equal 'users' @@ -39,9 +41,9 @@ user.networks.must_include network end - it 'has a list of stories for approved accounts' do - start_count = user.individual_account.stories.count - create(:account, user: user, stories_count: 2) - user.approved_account_stories.count.must_equal start_count + 2 + it 'has a list of stories and series for accounts approved on token' do + user.approved_account_stories.must_include story + user.approved_account_series.must_include series + end end From 862f874e1dbb46a96f21e5c2d27be11aa61ad61e Mon Sep 17 00:00:00 2001 From: gcampo88 Date: Tue, 31 Jan 2017 14:14:42 -0500 Subject: [PATCH 02/10] Get token-authorized accounts in ApiAuth concern, update tests --- app/controllers/api/auth/accounts_controller.rb | 6 +----- app/controllers/concerns/api_authenticated.rb | 8 +++++++- app/models/user.rb | 6 +++--- .../api/auth/accounts_controller_test.rb | 17 +++++------------ .../api/authorizations_controller_test.rb | 2 +- test/models/user_test.rb | 5 ++++- .../api/authorization_representer_test.rb | 4 ++++ test/test_helper.rb | 4 +++- 8 files changed, 28 insertions(+), 24 deletions(-) diff --git a/app/controllers/api/auth/accounts_controller.rb b/app/controllers/api/auth/accounts_controller.rb index e6730454..0fb636dc 100644 --- a/app/controllers/api/auth/accounts_controller.rb +++ b/app/controllers/api/auth/accounts_controller.rb @@ -14,10 +14,6 @@ def resources end def resources_base - @accounts ||= token_accounts - end - - def token_accounts - current_user.approved_accounts = Account.where({ id: prx_auth_token.authorized_resources.keys }) + current_user.approved_accounts end end diff --git a/app/controllers/concerns/api_authenticated.rb b/app/controllers/concerns/api_authenticated.rb index 633f8076..5398c7e7 100644 --- a/app/controllers/concerns/api_authenticated.rb +++ b/app/controllers/concerns/api_authenticated.rb @@ -7,13 +7,19 @@ module ApiAuthenticated extend ActiveSupport::Concern included do - before_filter :authenticate_user! + before_filter :authenticate_user!, :get_authorized_resources end def authenticate_user! user_not_authorized unless current_user end + def get_authorized_resources + if prx_auth_token.authorized_resources + current_user.approved_accounts ||= Account.where({ id: prx_auth_token.authorized_resources.keys }) + end + end + def cache_show? false end diff --git a/app/models/user.rb b/app/models/user.rb index c8b86687..c47d023f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -48,14 +48,14 @@ def name end def approved_active_accounts - approved_accounts.active + approved_accounts.try(:active) end def approved_account_stories - Story.where({ account_id: [approved_accounts] }) + !approved_accounts.nil? ? Story.where({ account_id: approved_accounts.try(:ids) }) : [] end def approved_account_series - Series.where({ account_id: [approved_accounts] }) + !approved_accounts.nil? ? Series.where({ account_id: approved_accounts.try(:ids) }) : [] end end diff --git a/test/controllers/api/auth/accounts_controller_test.rb b/test/controllers/api/auth/accounts_controller_test.rb index cb8ea897..4e512417 100644 --- a/test/controllers/api/auth/accounts_controller_test.rb +++ b/test/controllers/api/auth/accounts_controller_test.rb @@ -9,12 +9,11 @@ let (:member_account) { create(:account) } let (:unapproved_account) { create(:account)} - let (:token) { OpenStruct.new.tap { |t| - t.authorized_resources = { - member_account.id => "member", - individual_account.id => "admin" - } - t.user_id = user.id + let (:token) { StubToken.new(nil, nil, user.id).tap { |t| + t.authorized_resources = { + member_account.id => "member", + individual_account.id => "admin" + } } } @@ -57,12 +56,6 @@ ids.wont_include random_account.id end - it 'gets list of approved accounts for a user from users token' do - get(:index, api_version: 'v1') - assert_response :success - user.reload.approved_accounts.wont_be_nil - end - end describe 'with no token' do diff --git a/test/controllers/api/authorizations_controller_test.rb b/test/controllers/api/authorizations_controller_test.rb index b29c9420..7c868056 100644 --- a/test/controllers/api/authorizations_controller_test.rb +++ b/test/controllers/api/authorizations_controller_test.rb @@ -3,7 +3,7 @@ describe Api::AuthorizationsController do let (:user) { create(:user) } - let (:token) { OpenStruct.new.tap { |t| t.user_id = user.id } } + let (:token) { StubToken.new(user.individual_account.id, "admin", user.id) } it 'shows the user with a valid token' do @controller.stub(:prx_auth_token, token) do diff --git a/test/models/user_test.rb b/test/models/user_test.rb index b3709960..1e86b43b 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -6,6 +6,10 @@ let (:story) { create(:story, account: user.individual_account) } let (:series) { create(:series, account: user.individual_account) } + before do + user.approved_accounts = Account.where(id: [story.account_id, series.account_id]) + end + it 'has a table defined' do User.table_name.must_equal 'users' end @@ -44,6 +48,5 @@ it 'has a list of stories and series for accounts approved on token' do user.approved_account_stories.must_include story user.approved_account_series.must_include series - end end diff --git a/test/representers/api/authorization_representer_test.rb b/test/representers/api/authorization_representer_test.rb index 2e0e9848..4e9af3a8 100644 --- a/test/representers/api/authorization_representer_test.rb +++ b/test/representers/api/authorization_representer_test.rb @@ -7,6 +7,10 @@ let(:representer) { Api::AuthorizationRepresenter.new(user) } let(:json) { JSON.parse(representer.to_json) } + before do + user.approved_accounts = Account.where(id: account.id) + end + def get_link(rel, key = 'href') json['_links'][rel] ? json['_links'][rel][key] : nil end diff --git a/test/test_helper.rb b/test/test_helper.rb index 3b0a4a89..1c195e8f 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -63,9 +63,11 @@ def extract_filename(uri) StubToken = Struct.new(:resource, :scopes, :user_id) class StubToken + attr_accessor :authorized_resources @@fake_user_id = 0 def initialize(res, scopes, explicit_user_id = nil) + @authorized_resources = { res => scopes } if explicit_user_id super(res.to_s, scopes, explicit_user_id) else @@ -74,7 +76,7 @@ def initialize(res, scopes, explicit_user_id = nil) end def authorized?(r, s = nil) - resource == r.to_s && (s.nil? || scopes.include?(s.to_s)) + authorized_resources.keys.include?(r) && (s.nil? || authorized_resources.values.flatten.include?(s.to_s)) end end From cde199811736072a7df0df98baa7cf27e1846174 Mon Sep 17 00:00:00 2001 From: gcampo88 Date: Tue, 31 Jan 2017 14:48:42 -0500 Subject: [PATCH 03/10] hound --- app/controllers/concerns/api_authenticated.rb | 5 +++-- app/models/user.rb | 4 ++-- .../api/auth/accounts_controller_test.rb | 14 +++++++------- .../api/authorizations_controller_test.rb | 2 +- test/test_helper.rb | 5 ++++- 5 files changed, 17 insertions(+), 13 deletions(-) diff --git a/app/controllers/concerns/api_authenticated.rb b/app/controllers/concerns/api_authenticated.rb index 5398c7e7..d4c2c486 100644 --- a/app/controllers/concerns/api_authenticated.rb +++ b/app/controllers/concerns/api_authenticated.rb @@ -15,8 +15,9 @@ def authenticate_user! end def get_authorized_resources - if prx_auth_token.authorized_resources - current_user.approved_accounts ||= Account.where({ id: prx_auth_token.authorized_resources.keys }) + token_accounts = prx_auth_token.authorized_resources.try(:keys) + if token_accounts + current_user.approved_accounts ||= Account.where(id: token_accounts) end end diff --git a/app/models/user.rb b/app/models/user.rb index c47d023f..5af07e28 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -52,10 +52,10 @@ def approved_active_accounts end def approved_account_stories - !approved_accounts.nil? ? Story.where({ account_id: approved_accounts.try(:ids) }) : [] + !approved_accounts.nil? ? Story.where(account_id: approved_accounts.try(:ids)) : [] end def approved_account_series - !approved_accounts.nil? ? Series.where({ account_id: approved_accounts.try(:ids) }) : [] + !approved_accounts.nil? ? Series.where(account_id: approved_accounts.try(:ids)) : [] end end diff --git a/test/controllers/api/auth/accounts_controller_test.rb b/test/controllers/api/auth/accounts_controller_test.rb index 4e512417..b92acae7 100644 --- a/test/controllers/api/auth/accounts_controller_test.rb +++ b/test/controllers/api/auth/accounts_controller_test.rb @@ -7,15 +7,15 @@ let (:individual_account) { user.individual_account } let (:random_account) { create(:account) } let (:member_account) { create(:account) } - let (:unapproved_account) { create(:account)} + let (:unapproved_account) { create(:account) } + let (:token) { StubToken.new(nil, nil, user.id) } - let (:token) { StubToken.new(nil, nil, user.id).tap { |t| - t.authorized_resources = { - member_account.id => "member", - individual_account.id => "admin" - } + before do + token.authorized_resources = { + member_account.id => 'member', + individual_account.id => 'admin' } - } + end describe 'with a valid token' do diff --git a/test/controllers/api/authorizations_controller_test.rb b/test/controllers/api/authorizations_controller_test.rb index 7c868056..59081d78 100644 --- a/test/controllers/api/authorizations_controller_test.rb +++ b/test/controllers/api/authorizations_controller_test.rb @@ -3,7 +3,7 @@ describe Api::AuthorizationsController do let (:user) { create(:user) } - let (:token) { StubToken.new(user.individual_account.id, "admin", user.id) } + let (:token) { StubToken.new(user.individual_account.id, 'admin', user.id) } it 'shows the user with a valid token' do @controller.stub(:prx_auth_token, token) do diff --git a/test/test_helper.rb b/test/test_helper.rb index 1c195e8f..679e6046 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -76,7 +76,10 @@ def initialize(res, scopes, explicit_user_id = nil) end def authorized?(r, s = nil) - authorized_resources.keys.include?(r) && (s.nil? || authorized_resources.values.flatten.include?(s.to_s)) + res = authorized_resources.keys + roles = authorized_resources.values.flatten + + res.include?(r) && (s.nil? || roles.include?(s.to_s)) end end From 5cb4e8b02a283e300ff8bb93af96de07636b923e Mon Sep 17 00:00:00 2001 From: gcampo88 Date: Tue, 31 Jan 2017 15:01:56 -0500 Subject: [PATCH 04/10] Remove redundant test --- test/controllers/api/auth/accounts_controller_test.rb | 8 -------- 1 file changed, 8 deletions(-) diff --git a/test/controllers/api/auth/accounts_controller_test.rb b/test/controllers/api/auth/accounts_controller_test.rb index b92acae7..a987baa4 100644 --- a/test/controllers/api/auth/accounts_controller_test.rb +++ b/test/controllers/api/auth/accounts_controller_test.rb @@ -3,9 +3,7 @@ describe Api::Auth::AccountsController do let (:user) { create(:user_with_accounts, group_accounts: 2) } - let (:individual_account) { user.individual_account } - let (:random_account) { create(:account) } let (:member_account) { create(:account) } let (:unapproved_account) { create(:account) } let (:token) { StubToken.new(nil, nil, user.id) } @@ -38,11 +36,6 @@ assert_response :not_found end - it 'does not show non-member accounts' do - get(:show, api_version: 'v1', id: random_account.id) - assert_response :not_found - end - it 'indexes only member accounts' do get(:index, api_version: 'v1') assert_response :success @@ -53,7 +46,6 @@ ids.must_include individual_account.id ids.must_include member_account.id ids.wont_include unapproved_account.id - ids.wont_include random_account.id end end From a1d1bb6167c032f467095d04ffdabe73163d4392 Mon Sep 17 00:00:00 2001 From: gcampo88 Date: Wed, 1 Feb 2017 13:26:19 -0500 Subject: [PATCH 05/10] Create authorization model to query accounts, series, and stories based on token account IDs --- .../api/auth/accounts_controller.rb | 2 +- app/controllers/api/auth/series_controller.rb | 2 +- .../api/auth/stories_controller.rb | 4 +-- app/controllers/concerns/api_authenticated.rb | 9 +------ app/models/authorization.rb | 27 +++++++++++++++++++ app/models/user.rb | 19 +++++++++---- test/models/authorization_test.rb | 20 ++++++++++++++ test/models/user_test.rb | 6 +---- .../api/authorization_representer_test.rb | 4 --- 9 files changed, 67 insertions(+), 26 deletions(-) create mode 100644 app/models/authorization.rb create mode 100644 test/models/authorization_test.rb diff --git a/app/controllers/api/auth/accounts_controller.rb b/app/controllers/api/auth/accounts_controller.rb index 0fb636dc..29c8e7db 100644 --- a/app/controllers/api/auth/accounts_controller.rb +++ b/app/controllers/api/auth/accounts_controller.rb @@ -14,6 +14,6 @@ def resources end def resources_base - current_user.approved_accounts + Authorization.new(prx_auth_token).token_auth_accounts end end diff --git a/app/controllers/api/auth/series_controller.rb b/app/controllers/api/auth/series_controller.rb index 33879014..273d3416 100644 --- a/app/controllers/api/auth/series_controller.rb +++ b/app/controllers/api/auth/series_controller.rb @@ -10,6 +10,6 @@ class Api::Auth::SeriesController < Api::SeriesController represent_with Api::Auth::SeriesRepresenter def resources_base - @series ||= current_user.approved_account_series + @series ||= Authorization.new(prx_auth_token).token_auth_series end end diff --git a/app/controllers/api/auth/stories_controller.rb b/app/controllers/api/auth/stories_controller.rb index 35f8e9fc..b7939318 100644 --- a/app/controllers/api/auth/stories_controller.rb +++ b/app/controllers/api/auth/stories_controller.rb @@ -38,7 +38,7 @@ def resources_base @stories ||= if params[:network_id] super.published else - current_user.approved_account_stories + Authorization.new(prx_auth_token).token_auth_stories end end @@ -47,7 +47,7 @@ def create_resource story.creator_id = current_user.id story.account_id ||= story.series.try(:account_id) story.account_id ||= current_user.account_id - story.account_id ||= current_user.approved_accounts.first.try(:id) + story.account_id ||= current_user.approved_accounts.first.try(:id) # not sure if I should change this to look at token end end end diff --git a/app/controllers/concerns/api_authenticated.rb b/app/controllers/concerns/api_authenticated.rb index d4c2c486..633f8076 100644 --- a/app/controllers/concerns/api_authenticated.rb +++ b/app/controllers/concerns/api_authenticated.rb @@ -7,20 +7,13 @@ module ApiAuthenticated extend ActiveSupport::Concern included do - before_filter :authenticate_user!, :get_authorized_resources + before_filter :authenticate_user! end def authenticate_user! user_not_authorized unless current_user end - def get_authorized_resources - token_accounts = prx_auth_token.authorized_resources.try(:keys) - if token_accounts - current_user.approved_accounts ||= Account.where(id: token_accounts) - end - end - def cache_show? false end diff --git a/app/models/authorization.rb b/app/models/authorization.rb new file mode 100644 index 00000000..0113606e --- /dev/null +++ b/app/models/authorization.rb @@ -0,0 +1,27 @@ +# encoding: utf-8 + +class Authorization + attr_accessor :token + attr_reader :token_auth_accounts + + def initialize(token) + @token = token + get_authorized_resources + end + + def get_authorized_resources + end + + def token_auth_accounts + token_ids = token.authorized_resources.try(:keys) + @token_auth_accounts = Account.where(id: token_ids) if token_ids + end + + def token_auth_stories + Story.where(account_id: token_auth_accounts.try(:ids)) unless token_auth_accounts.nil? + end + + def token_auth_series + Series.where(account_id: token_auth_accounts.try(:ids)) unless token_auth_accounts.nil? + end +end diff --git a/app/models/user.rb b/app/models/user.rb index 5af07e28..beb0eaf8 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -15,8 +15,6 @@ class User < BaseModel after_commit :create_individual_account, on: [:create] - attr_accessor :approved_accounts - def individual_account accounts.where('type = \'IndividualAccount\'').first end @@ -47,15 +45,26 @@ def name "#{first_name} #{last_name}" end + def approved_accounts + Account. + joins('LEFT OUTER JOIN `memberships` ON `memberships`.`account_id` = `accounts`.`id`'). + where(['memberships.user_id = ? and memberships.approved is true', id]) + end + def approved_active_accounts - approved_accounts.try(:active) + approved_accounts.active end def approved_account_stories - !approved_accounts.nil? ? Story.where(account_id: approved_accounts.try(:ids)) : [] + Story. + joins('LEFT OUTER JOIN `memberships` ON `memberships`.`account_id` = `pieces`.`account_id`'). + where(['memberships.user_id = ? and memberships.approved is true', id]) end def approved_account_series - !approved_accounts.nil? ? Series.where(account_id: approved_accounts.try(:ids)) : [] + Series. + joins('LEFT OUTER JOIN `memberships` ON `memberships`.`account_id` = `series`.`account_id`'). + where(['memberships.user_id = ? and memberships.approved is true', id]) end + end diff --git a/test/models/authorization_test.rb b/test/models/authorization_test.rb new file mode 100644 index 00000000..4c92f8ce --- /dev/null +++ b/test/models/authorization_test.rb @@ -0,0 +1,20 @@ +require 'test_helper' + +describe Authorization do + let(:account) { create(:account) } + let(:token) { StubToken.new(account.id, ['member'], 456) } + let(:authorization) { Authorization.new(token) } + + it 'has a token' do + authorization.token.wont_be_nil + end + + it 'has a list of accounts authorized on token' do + authorization.token_auth_accounts.must_include account + end + + it 'has lists of stories and series belonging to accounts authorized on token' do + authorization.token_auth_stories.must_equal account.stories + authorization.token_auth_series.must_equal account.series + end +end diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 1e86b43b..a27a91b1 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -6,10 +6,6 @@ let (:story) { create(:story, account: user.individual_account) } let (:series) { create(:series, account: user.individual_account) } - before do - user.approved_accounts = Account.where(id: [story.account_id, series.account_id]) - end - it 'has a table defined' do User.table_name.must_equal 'users' end @@ -45,7 +41,7 @@ user.networks.must_include network end - it 'has a list of stories and series for accounts approved on token' do + it 'has a list of stories and series' do user.approved_account_stories.must_include story user.approved_account_series.must_include series end diff --git a/test/representers/api/authorization_representer_test.rb b/test/representers/api/authorization_representer_test.rb index 4e9af3a8..2e0e9848 100644 --- a/test/representers/api/authorization_representer_test.rb +++ b/test/representers/api/authorization_representer_test.rb @@ -7,10 +7,6 @@ let(:representer) { Api::AuthorizationRepresenter.new(user) } let(:json) { JSON.parse(representer.to_json) } - before do - user.approved_accounts = Account.where(id: account.id) - end - def get_link(rel, key = 'href') json['_links'][rel] ? json['_links'][rel][key] : nil end From ad87223929dbc84a4592f419dc65164c10555116 Mon Sep 17 00:00:00 2001 From: gcampo88 Date: Wed, 1 Feb 2017 13:32:01 -0500 Subject: [PATCH 06/10] Oops --- app/models/authorization.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/models/authorization.rb b/app/models/authorization.rb index 0113606e..6c1069ea 100644 --- a/app/models/authorization.rb +++ b/app/models/authorization.rb @@ -6,10 +6,6 @@ class Authorization def initialize(token) @token = token - get_authorized_resources - end - - def get_authorized_resources end def token_auth_accounts From c9ade9bb08e56f3273173dab73f9c5925f57eddf Mon Sep 17 00:00:00 2001 From: gcampo88 Date: Wed, 1 Feb 2017 13:36:00 -0500 Subject: [PATCH 07/10] Remove newlines --- app/models/user.rb | 1 - test/test_helper.rb | 1 - 2 files changed, 2 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index beb0eaf8..b74a7493 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -66,5 +66,4 @@ def approved_account_series joins('LEFT OUTER JOIN `memberships` ON `memberships`.`account_id` = `series`.`account_id`'). where(['memberships.user_id = ? and memberships.approved is true', id]) end - end diff --git a/test/test_helper.rb b/test/test_helper.rb index 679e6046..62b991aa 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -78,7 +78,6 @@ def initialize(res, scopes, explicit_user_id = nil) def authorized?(r, s = nil) res = authorized_resources.keys roles = authorized_resources.values.flatten - res.include?(r) && (s.nil? || roles.include?(s.to_s)) end end From 3dcd64ab31c44c40360e7f0acf49f7f517fedb99 Mon Sep 17 00:00:00 2001 From: gcampo88 Date: Fri, 3 Feb 2017 16:38:44 -0500 Subject: [PATCH 08/10] Nix user model methods and put everything in authorization --- .../api/auth/accounts_controller.rb | 2 +- app/controllers/api/auth/series_controller.rb | 2 +- .../api/auth/stories_controller.rb | 6 ++--- .../api/authorizations_controller.rb | 2 +- app/controllers/concerns/api_authenticated.rb | 4 ++++ app/models/authorization.rb | 12 ++++++++++ app/models/user.rb | 22 ------------------- .../api/authorization_representer.rb | 11 +++++----- .../api/authorizations_controller_test.rb | 7 ------ .../concerns/api_authenticated_test.rb | 11 ++++++++++ test/models/authorization_test.rb | 6 +++++ test/models/user_test.rb | 5 ----- .../api/authorization_representer_test.rb | 4 +++- 13 files changed, 47 insertions(+), 47 deletions(-) diff --git a/app/controllers/api/auth/accounts_controller.rb b/app/controllers/api/auth/accounts_controller.rb index 29c8e7db..4e74eae0 100644 --- a/app/controllers/api/auth/accounts_controller.rb +++ b/app/controllers/api/auth/accounts_controller.rb @@ -14,6 +14,6 @@ def resources end def resources_base - Authorization.new(prx_auth_token).token_auth_accounts + authorization.token_auth_accounts end end diff --git a/app/controllers/api/auth/series_controller.rb b/app/controllers/api/auth/series_controller.rb index 273d3416..6925ceab 100644 --- a/app/controllers/api/auth/series_controller.rb +++ b/app/controllers/api/auth/series_controller.rb @@ -10,6 +10,6 @@ class Api::Auth::SeriesController < Api::SeriesController represent_with Api::Auth::SeriesRepresenter def resources_base - @series ||= Authorization.new(prx_auth_token).token_auth_series + @series ||= authorization.token_auth_series end end diff --git a/app/controllers/api/auth/stories_controller.rb b/app/controllers/api/auth/stories_controller.rb index b7939318..26414461 100644 --- a/app/controllers/api/auth/stories_controller.rb +++ b/app/controllers/api/auth/stories_controller.rb @@ -38,7 +38,7 @@ def resources_base @stories ||= if params[:network_id] super.published else - Authorization.new(prx_auth_token).token_auth_stories + authorization.token_auth_stories end end @@ -46,8 +46,8 @@ def create_resource super.tap do |story| story.creator_id = current_user.id story.account_id ||= story.series.try(:account_id) - story.account_id ||= current_user.account_id - story.account_id ||= current_user.approved_accounts.first.try(:id) # not sure if I should change this to look at token + story.account_id ||= current_user.account_id if authorization.authorized?(current_user.default_account) + story.account_id ||= authorization.token_auth_accounts.first.try(:id) end end end diff --git a/app/controllers/api/authorizations_controller.rb b/app/controllers/api/authorizations_controller.rb index 1e5e24c0..34da795e 100644 --- a/app/controllers/api/authorizations_controller.rb +++ b/app/controllers/api/authorizations_controller.rb @@ -10,6 +10,6 @@ class Api::AuthorizationsController < Api::BaseController private def resource - current_user + authorization end end diff --git a/app/controllers/concerns/api_authenticated.rb b/app/controllers/concerns/api_authenticated.rb index 633f8076..c7383e13 100644 --- a/app/controllers/concerns/api_authenticated.rb +++ b/app/controllers/concerns/api_authenticated.rb @@ -14,6 +14,10 @@ def authenticate_user! user_not_authorized unless current_user end + def authorization + Authorization.new(prx_auth_token) + end + def cache_show? false end diff --git a/app/models/authorization.rb b/app/models/authorization.rb index 6c1069ea..6eb47b8b 100644 --- a/app/models/authorization.rb +++ b/app/models/authorization.rb @@ -8,6 +8,14 @@ def initialize(token) @token = token end + def id + default_account.id + end + + def default_account + User.find(token.user_id).default_account + end + def token_auth_accounts token_ids = token.authorized_resources.try(:keys) @token_auth_accounts = Account.where(id: token_ids) if token_ids @@ -20,4 +28,8 @@ def token_auth_stories def token_auth_series Series.where(account_id: token_auth_accounts.try(:ids)) unless token_auth_accounts.nil? end + + def authorized?(account) + token_auth_accounts.include?(account) + end end diff --git a/app/models/user.rb b/app/models/user.rb index b74a7493..93131d4e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -44,26 +44,4 @@ def networks def name "#{first_name} #{last_name}" end - - def approved_accounts - Account. - joins('LEFT OUTER JOIN `memberships` ON `memberships`.`account_id` = `accounts`.`id`'). - where(['memberships.user_id = ? and memberships.approved is true', id]) - end - - def approved_active_accounts - approved_accounts.active - end - - def approved_account_stories - Story. - joins('LEFT OUTER JOIN `memberships` ON `memberships`.`account_id` = `pieces`.`account_id`'). - where(['memberships.user_id = ? and memberships.approved is true', id]) - end - - def approved_account_series - Series. - joins('LEFT OUTER JOIN `memberships` ON `memberships`.`account_id` = `series`.`account_id`'). - where(['memberships.user_id = ? and memberships.approved is true', id]) - end end diff --git a/app/representers/api/authorization_representer.rb b/app/representers/api/authorization_representer.rb index 2d52f68c..ae52b77d 100644 --- a/app/representers/api/authorization_representer.rb +++ b/app/representers/api/authorization_representer.rb @@ -2,7 +2,6 @@ class Api::AuthorizationRepresenter < Api::BaseRepresenter property :id, writeable: false - property :name link :default_account do { @@ -15,10 +14,10 @@ class Api::AuthorizationRepresenter < Api::BaseRepresenter { href: "#{api_authorization_accounts_path}#{index_url_params}", templated: true, - count: represented.accounts.count + count: represented.token_auth_accounts.count } end - embed :approved_active_accounts, as: :accounts, paged: true, per: :all, item_class: Account, + embed :token_auth_accounts, as: :accounts, paged: true, per: :all, item_class: Account, item_decorator: Api::Auth::AccountMinRepresenter, url: ->(_r) { api_authorization_accounts_path } @@ -27,12 +26,12 @@ class Api::AuthorizationRepresenter < Api::BaseRepresenter { href: "#{api_authorization_series_path_template(id: '{id}')}#{show_url_params}", templated: true, - count: represented.approved_account_series.count + count: represented.token_auth_series.count }, { href: "#{api_authorization_series_index_path}#{index_url_params}", templated: true, - count: represented.approved_account_series.count + count: represented.token_auth_series.count } ] end @@ -41,7 +40,7 @@ class Api::AuthorizationRepresenter < Api::BaseRepresenter { href: "#{api_authorization_stories_path}#{index_url_params}", templated: true, - count: represented.approved_account_stories.count + count: represented.token_auth_stories.count } end diff --git a/test/controllers/api/authorizations_controller_test.rb b/test/controllers/api/authorizations_controller_test.rb index 59081d78..a1cd400b 100644 --- a/test/controllers/api/authorizations_controller_test.rb +++ b/test/controllers/api/authorizations_controller_test.rb @@ -5,13 +5,6 @@ let (:user) { create(:user) } let (:token) { StubToken.new(user.individual_account.id, 'admin', user.id) } - it 'shows the user with a valid token' do - @controller.stub(:prx_auth_token, token) do - get(:show, api_version: 'v1') - assert_response :success - end - end - it 'returns unauthorized with invalid token' do @controller.stub(:prx_auth_token, OpenStruct.new) do get(:show, api_version: 'v1') diff --git a/test/controllers/concerns/api_authenticated_test.rb b/test/controllers/concerns/api_authenticated_test.rb index cbbf739d..5b60bc1b 100644 --- a/test/controllers/concerns/api_authenticated_test.rb +++ b/test/controllers/concerns/api_authenticated_test.rb @@ -12,6 +12,9 @@ def current_user def user_not_authorized raise 'user_not_authorized' end + + def prx_auth_token + end end let(:controller) { ApiAuthenticatedTestController.new } @@ -33,4 +36,12 @@ def user_not_authorized err.message.must_match /user_not_authorized/ end + it 'builds an authorization from token' do + controller.stub(:current_user, true) do + controller.stub(:prx_auth_token, StubToken.new(123, 'admin', nil)) do + controller.authorization.wont_be_nil + end + end + end + end diff --git a/test/models/authorization_test.rb b/test/models/authorization_test.rb index 4c92f8ce..dc31c2ee 100644 --- a/test/models/authorization_test.rb +++ b/test/models/authorization_test.rb @@ -4,6 +4,7 @@ let(:account) { create(:account) } let(:token) { StubToken.new(account.id, ['member'], 456) } let(:authorization) { Authorization.new(token) } + let(:unauth_account) { create(:account) } it 'has a token' do authorization.token.wont_be_nil @@ -17,4 +18,9 @@ authorization.token_auth_stories.must_equal account.stories authorization.token_auth_series.must_equal account.series end + + it 'checks against token to see if accounts are authorized' do + authorization.authorized?(account).must_equal true + authorization.authorized?(unauth_account).must_equal false + end end diff --git a/test/models/user_test.rb b/test/models/user_test.rb index a27a91b1..d48b0173 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -40,9 +40,4 @@ it 'has a list of networks' do user.networks.must_include network end - - it 'has a list of stories and series' do - user.approved_account_stories.must_include story - user.approved_account_series.must_include series - end end diff --git a/test/representers/api/authorization_representer_test.rb b/test/representers/api/authorization_representer_test.rb index 2e0e9848..1421f71c 100644 --- a/test/representers/api/authorization_representer_test.rb +++ b/test/representers/api/authorization_representer_test.rb @@ -4,7 +4,9 @@ let(:user) { create(:user) } let(:account) { user.default_account } - let(:representer) { Api::AuthorizationRepresenter.new(user) } + let(:token) { StubToken.new(account.id, 'admin', user.id)} + let(:authorization) { Authorization.new(token) } + let(:representer) { Api::AuthorizationRepresenter.new(authorization) } let(:json) { JSON.parse(representer.to_json) } def get_link(rel, key = 'href') From dded0e40d1d1e86be76030455a9315d39e6bc09d Mon Sep 17 00:00:00 2001 From: gcampo88 Date: Fri, 3 Feb 2017 16:40:55 -0500 Subject: [PATCH 09/10] hound --- test/controllers/concerns/api_authenticated_test.rb | 3 +-- test/representers/api/authorization_representer_test.rb | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/test/controllers/concerns/api_authenticated_test.rb b/test/controllers/concerns/api_authenticated_test.rb index 5b60bc1b..73e95a06 100644 --- a/test/controllers/concerns/api_authenticated_test.rb +++ b/test/controllers/concerns/api_authenticated_test.rb @@ -13,8 +13,7 @@ def user_not_authorized raise 'user_not_authorized' end - def prx_auth_token - end + def prx_auth_token; end end let(:controller) { ApiAuthenticatedTestController.new } diff --git a/test/representers/api/authorization_representer_test.rb b/test/representers/api/authorization_representer_test.rb index 1421f71c..e2fc36e0 100644 --- a/test/representers/api/authorization_representer_test.rb +++ b/test/representers/api/authorization_representer_test.rb @@ -4,7 +4,7 @@ let(:user) { create(:user) } let(:account) { user.default_account } - let(:token) { StubToken.new(account.id, 'admin', user.id)} + let(:token) { StubToken.new(account.id, 'admin', user.id) } let(:authorization) { Authorization.new(token) } let(:representer) { Api::AuthorizationRepresenter.new(authorization) } let(:json) { JSON.parse(representer.to_json) } From af9cafb06b80a539990a675aa0f3884df9a2d65f Mon Sep 17 00:00:00 2001 From: gcampo88 Date: Fri, 3 Feb 2017 16:45:29 -0500 Subject: [PATCH 10/10] unused test objects --- test/models/user_test.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/models/user_test.rb b/test/models/user_test.rb index d48b0173..87f34664 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -3,8 +3,6 @@ describe User do let (:user) { create(:user) } let (:network) { create(:network, account: user.individual_account) } - let (:story) { create(:story, account: user.individual_account) } - let (:series) { create(:series, account: user.individual_account) } it 'has a table defined' do User.table_name.must_equal 'users'