diff options
author | Rémy Coutable <remy@rymai.me> | 2016-06-24 13:01:48 +0300 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2016-06-24 13:01:48 +0300 |
commit | aec3475df94bc9681a723c344f3df05972ebe68c (patch) | |
tree | 3ed0b3d284fc4c0657f06489a5403f1c30126b30 /spec/models/user_spec.rb | |
parent | 4477dc249e563e60e126d4f5ad2692297a9584c1 (diff) |
Fix an information disclosure when requesting access to a group containing private projects
The issue was with the `User#groups` and `User#projects` associations
which goes through the `User#group_members` and `User#project_members`.
Initially I chose to use a secure approach by storing the requester's
user ID in `Member#created_by_id` instead of `Member#user_id` because I
was aware that there was a security risk since I didn't know the
codebase well enough.
Then during the review, we decided to change that and directly store the
requester's user ID into `Member#user_id` (for the sake of simplifying
the code I believe), meaning that every `group_members` / `project_members`
association would include the requesters by default...
My bad for not checking that all the `group_members` / `project_members`
associations and the ones that go through them (e.g. `Group#users` and
`Project#users`) were made safe with the `where(requested_at: nil)` /
`where(members: { requested_at: nil })` scopes.
Now they are all secure.
Signed-off-by: Rémy Coutable <remy@rymai.me>
Diffstat (limited to 'spec/models/user_spec.rb')
-rw-r--r-- | spec/models/user_spec.rb | 20 |
1 files changed, 20 insertions, 0 deletions
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 73bee535fe3..328254ed56b 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -31,6 +31,26 @@ describe User, models: true do it { is_expected.to have_many(:spam_logs).dependent(:destroy) } it { is_expected.to have_many(:todos).dependent(:destroy) } it { is_expected.to have_many(:award_emoji).dependent(:destroy) } + + describe '#group_members' do + it 'does not include group memberships for which user is a requester' do + user = create(:user) + group = create(:group, :public) + group.request_access(user) + + expect(user.group_members).to be_empty + end + end + + describe '#project_members' do + it 'does not include project memberships for which user is a requester' do + user = create(:user) + project = create(:project, :public) + project.request_access(user) + + expect(user.project_members).to be_empty + end + end end describe 'validations' do |