Commit 72b87ff3606fd058d168ef1e4c82a3962705056a
1 parent
ca9b4c1d
Exists in
colab
and in
4 other branches
Repository ownership checking uses RepositoryAttributes instead of Project
Showing
2 changed files
with
62 additions
and
2 deletions
Show diff stats
app/controllers/concerns/ownership_authentication.rb
... | ... | @@ -14,7 +14,7 @@ module OwnershipAuthentication |
14 | 14 | end |
15 | 15 | |
16 | 16 | def repository_owner? |
17 | - check_project_ownership(params[:project_id]) | |
17 | + check_repository_ownership(params[:id]) | |
18 | 18 | end |
19 | 19 | |
20 | 20 | def reading_group_owner? |
... | ... | @@ -48,9 +48,20 @@ module OwnershipAuthentication |
48 | 48 | check_kalibro_configuration_ownership(params[:kalibro_configuration_id]) |
49 | 49 | end |
50 | 50 | |
51 | - | |
52 | 51 | private |
53 | 52 | |
53 | + def check_repository_ownership(id) | |
54 | + if current_user.repository_attributes.find_by_repository_id(id).nil? | |
55 | + respond_to do |format| | |
56 | + format.html { redirect_to projects_url, notice: t('not_allowed') } | |
57 | + format.json { head :no_content } | |
58 | + end | |
59 | + end | |
60 | + | |
61 | + return true | |
62 | + end | |
63 | + | |
64 | + | |
54 | 65 | def check_project_ownership(id) |
55 | 66 | if current_user.project_attributes.find_by_project_id(id).nil? |
56 | 67 | respond_to do |format| | ... | ... |
spec/controllers/concerns/ownership_authentication_spec.rb
... | ... | @@ -140,4 +140,53 @@ describe OwnershipAuthentication, type: :controller do |
140 | 140 | end |
141 | 141 | end |
142 | 142 | end |
143 | + | |
144 | + describe 'repository_owner?' do | |
145 | + let(:repository) { FactoryGirl.build(:repository) } | |
146 | + | |
147 | + context 'within RepositoriesController' do | |
148 | + let! (:repositories_controller) { RepositoriesController.new } | |
149 | + | |
150 | + before do | |
151 | + repositories_controller.params = {} | |
152 | + repositories_controller.params[:id] = repository.id | |
153 | + end | |
154 | + | |
155 | + context 'with a user logged in' do | |
156 | + let! (:current_user) { FactoryGirl.build(:user) } | |
157 | + | |
158 | + before do | |
159 | + repositories_controller.expects(:current_user).returns(current_user) | |
160 | + end | |
161 | + | |
162 | + context 'when the user owns the Repository' do | |
163 | + let!(:repository_attributes) { FactoryGirl.build(:repository_attributes, {user_id: current_user.id, repository_id: repository.id}) } | |
164 | + | |
165 | + before do | |
166 | + repository_attrs = mock('repository_attributes') | |
167 | + repository_attrs.expects(:find_by_repository_id).with(repository.id).returns(repository_attributes) | |
168 | + current_user.expects(:repository_attributes).returns(repository_attrs) | |
169 | + end | |
170 | + | |
171 | + it 'should return true' do | |
172 | + expect(repositories_controller.repository_owner?).to be_truthy | |
173 | + end | |
174 | + end | |
175 | + | |
176 | + context 'when the user does not own the Repository' do | |
177 | + before do | |
178 | + repository_attrs = mock('repository_attributes') | |
179 | + repository_attrs.expects(:find_by_repository_id).with(repository.id).returns(nil) | |
180 | + current_user.expects(:repository_attributes).returns(repository_attrs) | |
181 | + end | |
182 | + | |
183 | + it 'should respond' do # FIXME: this is not the best test, but it it's the closest we can do I think | |
184 | + repositories_controller.expects(:respond_to) | |
185 | + | |
186 | + repositories_controller.repository_owner? | |
187 | + end | |
188 | + end | |
189 | + end | |
190 | + end | |
191 | + end | |
143 | 192 | end | ... | ... |