Skip to content

Commit 72d658b

Browse files
authored
File manager exception fix (#2229)
* Flash error if the check instructor fails * Unit test to check when a user is not authorized * Unit permission tests for file manager methods * Added test file in fixtures * Re routes when user is not instructor of any course * Fixed broken variable naming
1 parent 46fc697 commit 72d658b

File tree

3 files changed

+351
-31
lines changed

3 files changed

+351
-31
lines changed

app/controllers/file_manager_controller.rb

Lines changed: 51 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ def index
2727
end
2828
end
2929
absolute_path = check_path_exist(path)
30-
if (File.directory?(absolute_path) && check_instructor(absolute_path)) || path == ""
30+
if (File.directory?(absolute_path) && check_instructor(absolute_path)) ||
31+
(path == "" && is_instructor_of_any_course)
3132
populate_directory(absolute_path, new_url)
3233
render 'file_manager/index'
3334
elsif File.file?(absolute_path) && check_instructor(absolute_path)
@@ -42,6 +43,9 @@ def index
4243
filename: File.basename(absolute_path),
4344
disposition: 'attachment')
4445
end
46+
else
47+
flash[:error] = "You are not authorized to view this path"
48+
redirect_to root_path
4549
end
4650
end
4751

@@ -51,12 +55,15 @@ def upload
5155

5256
def delete
5357
absolute_path = check_path_exist(params[:path])
54-
return unless check_instructor(absolute_path)
55-
56-
parent = absolute_path.parent
57-
raise "Unable to delete courses in the root directory." if parent == BASE_DIRECTORY
58+
if check_instructor(absolute_path)
59+
parent = absolute_path.parent
60+
raise "Unable to delete courses in the root directory." if parent == BASE_DIRECTORY
5861

59-
FileUtils.rm_rf(absolute_path)
62+
FileUtils.rm_rf(absolute_path)
63+
else
64+
flash[:error] = "You are not authorized to delete this"
65+
redirect_to root_path
66+
end
6067
end
6168

6269
def rename
@@ -86,6 +93,9 @@ def rename
8693
FileUtils.mv(absolute_path, new_path)
8794
flash[:success] = "Successfully renamed file to #{params[:new_name]}"
8895
end
96+
else
97+
flash[:error] = "You are not authorized to rename this path"
98+
redirect_to root_path
8999
end
90100
rescue ArgumentError => e
91101
flash[:error] = e.message
@@ -95,33 +105,36 @@ def download_tar
95105
path = params[:path]&.split("/")&.drop(2)&.join("/")
96106
path = CGI.unescape(path)
97107
absolute_path = check_path_exist(path)
98-
return unless check_instructor(absolute_path)
99-
100-
if File.directory?(absolute_path)
101-
tar_stream = StringIO.new("")
102-
Gem::Package::TarWriter.new(tar_stream) do |tar|
103-
Dir[File.join(absolute_path.to_s, '**', '**')].each do |file|
104-
mode = File.stat(file).mode
105-
relative_path = file.sub(%r{^#{Regexp.escape(absolute_path.to_s)}/?}, '')
106-
if File.directory?(file)
107-
tar.mkdir relative_path, mode
108-
else
109-
tar.add_file relative_path, mode do |tar_file|
110-
File.open(file, "rb") { |f| tar_file.write f.read }
108+
if check_instructor(absolute_path)
109+
if File.directory?(absolute_path)
110+
tar_stream = StringIO.new("")
111+
Gem::Package::TarWriter.new(tar_stream) do |tar|
112+
Dir[File.join(absolute_path.to_s, '**', '**')].each do |file|
113+
mode = File.stat(file).mode
114+
relative_path = file.sub(%r{^#{Regexp.escape(absolute_path.to_s)}/?}, '')
115+
if File.directory?(file)
116+
tar.mkdir relative_path, mode
117+
else
118+
tar.add_file relative_path, mode do |tar_file|
119+
File.open(file, "rb") { |f| tar_file.write f.read }
120+
end
111121
end
112122
end
113123
end
124+
tar_stream.rewind
125+
tar_stream.close
126+
send_data tar_stream.string.force_encoding("binary"),
127+
filename: "file_manager.tar",
128+
type: "application/x-tar",
129+
disposition: "attachment"
130+
else
131+
send_file(absolute_path,
132+
filename: File.basename(absolute_path),
133+
disposition: 'attachment')
114134
end
115-
tar_stream.rewind
116-
tar_stream.close
117-
send_data tar_stream.string.force_encoding("binary"),
118-
filename: "file_manager.tar",
119-
type: "application/x-tar",
120-
disposition: "attachment"
121135
else
122-
send_file(absolute_path,
123-
filename: File.basename(absolute_path),
124-
disposition: 'attachment')
136+
flash[:error] = "You are not authorized to download attachments at this path"
137+
redirect_to root_path
125138
end
126139
end
127140

@@ -159,6 +172,9 @@ def upload_file(path)
159172
end
160173
end
161174
end
175+
else
176+
flash[:error] = "You are not authorized to upload files at this path"
177+
redirect_to root_path
162178
end
163179
end
164180
end
@@ -223,6 +239,13 @@ def check_path_exist(path)
223239
@absolute_path
224240
end
225241

242+
def is_instructor_of_any_course
243+
current_user_id = current_user.id
244+
cuds = CourseUserDatum.where(user_id: current_user_id, instructor: true)
245+
courses = Course.where(id: cuds.map(&:course_id))
246+
!courses.empty?
247+
end
248+
226249
def check_instructor(path)
227250
current_user_id = current_user.id
228251
cuds = CourseUserDatum.where(user_id: current_user_id, instructor: true)

0 commit comments

Comments
 (0)