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

3.1.8 #3779

Merged
merged 8 commits into from
Sep 17, 2024
Merged

3.1.8 #3779

Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 9 additions & 2 deletions apps/dashboard/app/controllers/files_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,16 @@ def upload
parse_path(upload_path)
validate_path!

@path.handle_upload(params[:file].tempfile)
# Need to remove the tempfile from list of Rack tempfiles to prevent it
# being cleaned up once request completes since Rclone uses the files.
request.env[Rack::RACK_TEMPFILES].reject! { |f| f.path == params[:file].tempfile.path } unless posix_file?

render json: {}
@transfer = @path.handle_upload(params[:file].tempfile)
if @transfer
render "transfers/show"
else
render json: {}
end
rescue AllowlistPolicy::Forbidden => e
render json: { error_message: e.message }, status: :forbidden
rescue Errno::EACCES => e
Expand Down
11 changes: 10 additions & 1 deletion apps/dashboard/app/javascript/files/data_table.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,15 @@ class DataTable {
return this._table;
}

toHumanSize(number) {
if(number === null) {
return '-';
} else {
const unitIndex = number == 0 ? 0 : Math.floor(Math.log(number) / Math.log(1000));
return `${((number / Math.pow(1000, unitIndex)).toFixed(2))} ${['B', 'kB', 'MB', 'GB', 'TB', 'PB'][unitIndex]}`;
}
}

loadDataTable() {
this._table = $(CONTENTID).on('xhr.dt', function (e, settings, json, xhr) {
// new ajax request for new data so update date/time
Expand Down Expand Up @@ -219,7 +228,7 @@ class DataTable {
{
data: 'size',
render: (data, type, row, meta) => {
return type == "display" ? row.human_size : data;
return type == "display" ? this.toHumanSize(row.size) : data;
}
}, // human_size
{
Expand Down
1 change: 1 addition & 0 deletions apps/dashboard/app/javascript/files/file_ops.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const EVENTNAME = {


let fileOps = null;
export {fileOps};

jQuery(function() {
fileOps = new FileOps();
Expand Down
14 changes: 14 additions & 0 deletions apps/dashboard/app/javascript/files/uppy_ops.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import XHRUpload from '@uppy/xhr-upload'
import _ from 'lodash';
import {CONTENTID, EVENTNAME as DATATABLE_EVENTNAME} from './data_table.js';
import { maxFileSize, csrfToken, uppyLocale } from '../config.js';
import { fileOps } from './file_ops.js';

let uppy = null;

Expand Down Expand Up @@ -102,6 +103,7 @@ jQuery(function() {

uppy.on('complete', (result) => {
if(result.successful.length > 0){
result.successful.forEach(handleUploadSuccess);
reloadTable();
}
});
Expand Down Expand Up @@ -179,3 +181,15 @@ function reloadTable() {
$(CONTENTID).trigger(DATATABLE_EVENTNAME.reloadTable,{});
}

// Uploads may return the status of a transfer for remote uploads.
function handleUploadSuccess(result) {
// These extra checks might not be needed.
const body = result?.response?.body;
if (!body || !(typeof body === "object" && !Array.isArray(body) && body !== null)) {
return;
}
if (Object.keys(body).length > 0 && !body.completed) {
fileOps.reportTransfer(body);
}
}

25 changes: 15 additions & 10 deletions apps/dashboard/app/models/posix_file.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ class PosixFile

attr_reader :path, :stat

delegate :basename, :descend, :parent, :join, :to_s, :read, :write, :mkdir, :directory?, :realpath, to: :path
delegate :basename, :descend, :parent, :join, :to_s, :read, :write, :mkdir, to: :path
delegate :directory?, :realpath, :readable?, :file?, to: :path

# include to give us number_to_human_size
include ActionView::Helpers::NumberHelper
Expand Down Expand Up @@ -53,18 +54,22 @@ def to_h
return { name: basename } if stat.nil?

{
id: "dev-#{stat.dev}-inode-#{stat.ino}",
name: basename,
size: directory? ? nil : stat.size,
human_size: human_size,
directory: directory?,
date: stat.mtime.to_i,
owner: PosixFile.username_from_cache(stat.uid),
mode: stat.mode,
dev: stat.dev
id: "dev-#{stat.dev}-inode-#{stat.ino}",
name: basename,
size: directory? ? nil : stat.size,
directory: directory?,
date: stat.mtime.to_i,
owner: PosixFile.username_from_cache(stat.uid),
mode: stat.mode,
dev: stat.dev,
downloadable: downloadable?
}
end

def downloadable?
(directory? || file?) && readable?
end

def human_size
directory? ? '-' : number_to_human_size(stat.size, precision: 3)
end
Expand Down
37 changes: 18 additions & 19 deletions apps/dashboard/app/models/remote_file.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,15 @@ def ls
# owner, mode and dev are not defined for remote files, leaving them empty
files.map do |file|
{
id: file['Path'],
name: file['Name'],
size: file['IsDir'] ? nil : file['Size'],
human_size: human_size(file),
directory: file['IsDir'],
date: DateTime.parse(file['ModTime']).to_time.to_i,
owner: '',
mode: '',
dev: 0
id: file['Path'],
name: file['Name'],
size: file['IsDir'] ? nil : file['Size'],
directory: file['IsDir'],
date: DateTime.parse(file['ModTime']).to_time.to_i,
owner: '',
mode: '',
dev: 0,
downloadable: true
}
end.select do |stats|
valid_encoding = stats[:name].to_s.valid_encoding?
Expand All @@ -48,14 +48,6 @@ def ls
end.sort_by { |p| p[:directory] ? 0 : 1 }
end

def human_size(file)
if file['IsDir']
'-'
else
::ApplicationController.helpers.number_to_human_size(file['Size'], precision: 3)
end
end

def can_download_as_zip?(timeout: Configuration.file_download_dir_timeout, download_directory_size_limit: Configuration.file_download_dir_max)
[false, 'Downloading remote files as zip is currently not supported']
end
Expand Down Expand Up @@ -85,8 +77,15 @@ def write(content)
end

def handle_upload(tempfile)
# FIXME: upload to the remote asynchronously
RcloneUtil.moveto(remote, path, tempfile.path)
# Start a transfer that moves the Rack tempfile to the remote.
transfer = RemoteTransfer.build(
action: "mv",
files: { tempfile.path => path.to_s },
src_remote: RcloneUtil::LOCAL_FS_NAME,
dest_remote: remote,
tempfile: tempfile
)
transfer.tap(&:perform_later)
end

def mime_type
Expand Down
8 changes: 4 additions & 4 deletions apps/dashboard/app/models/remote_transfer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,24 +43,23 @@ class RemoteTransfer < Transfer
end
end

attr_accessor :src_remote, :dest_remote, :filesizes, :transferred
attr_accessor :src_remote, :dest_remote, :tempfile, :filesizes, :transferred

class << self
def transfers
# all transfers stored in the Transfer class
Transfer.transfers
end

def build(action:, files:, src_remote:, dest_remote:)
def build(action:, files:, src_remote:, dest_remote:, tempfile: nil)
if files.is_a?(Array)
# rm action will want to provide an array of files
# so if it is an Array we convert it to a hash:
#
# convert [a1, a2, a3] to {a1 => nil, a2 => nil, a3 => nil}
files = Hash[files.map { |f| [f, nil] }].with_indifferent_access
end

self.new(action: action, files: files, src_remote: src_remote, dest_remote: dest_remote)
self.new(action: action, files: files, src_remote: src_remote, dest_remote: dest_remote, tempfile: tempfile)
end
end

Expand Down Expand Up @@ -133,6 +132,7 @@ def perform
errors.add :base, e.message
ensure
self.status = OodCore::Job::Status.new(state: :completed)
tempfile&.close(true)
end

def from
Expand Down
2 changes: 2 additions & 0 deletions apps/dashboard/app/views/files/_file_action_menu.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
{{/if}}
<li><a href="#" class="rename-file dropdown-item" data-row-index="{{row_index}}"><i class="fas fa-font" aria-hidden="true"></i> Rename</a></li>
<%- if Configuration.download_enabled? -%>
{{#if data.download_url}}
<li><a href="{{data.download_url}}" class="download-file dropdown-item" data-row-index="{{row_index}}"><i class="fas fa-download" aria-hidden="true"></i> Download</a></li>
{{/if}}
<%- end -%>
<li class="dropdown-divider mt-4"></li>
<li><a href="#" class="delete-file dropdown-item text-danger" data-row-index="{{row_index}}"><i class="fas fa-trash" aria-hidden="true"></i> Delete</a></li>
Expand Down
2 changes: 1 addition & 1 deletion apps/dashboard/app/views/files/index.json.jbuilder
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ json.files @files do |f|
json.name f[:name]

json.url files_path(@filesystem, @path.join(f[:name]).to_s)
json.download_url files_path(@filesystem, @path.join(f[:name]).to_s, download: '1') # FIXME: should change for directory
json.download_url files_path(@filesystem, @path.join(f[:name]).to_s, download: '1') if f[:downloadable]
json.edit_url OodAppkit.editor.edit(path: @path.join(f[:name]).to_s, fs: @filesystem).to_s

json.size f[:size]
Expand Down
40 changes: 40 additions & 0 deletions apps/dashboard/test/models/files_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -117,4 +117,44 @@ class FilesTest < ActiveSupport::TestCase
end
end
end

test 'unreadable files are not downloadable' do
Dir.mktmpdir do |dir|
file = File.join(dir, 'foo.text')
FileUtils.touch(file)
# make sure file is not readable, a condition of PosixFile#downloadable?
FileUtils.chmod(0o0333, file)

refute(PosixFile.new(file).downloadable?)
end
end

test 'fifo pipes are not downloadable' do
Dir.mktmpdir do |dir|
file = "#{dir}/test.fifo"
`mkfifo #{file}`

refute(PosixFile.new(file).downloadable?)
end
end

test 'block devices are not downloadable' do
block_dev = Dir.children('/dev').map do |p|
Pathname.new("/dev/#{p}")
end.select(&:blockdev?).first

assert(block_dev.exist?)
assert(block_dev.blockdev?)
refute(PosixFile.new(block_dev.to_s).downloadable?)
end

test 'character devices are not downloadable' do
char_dev = Dir.children('/dev').map do |p|
Pathname.new("/dev/#{p}")
end.select(&:chardev?).first

assert(char_dev.exist?)
assert(char_dev.chardev?)
refute(PosixFile.new(char_dev.to_s).downloadable?)
end
end
41 changes: 41 additions & 0 deletions apps/dashboard/test/system/files_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -593,4 +593,45 @@ def setup
find('tbody a', exact_text: 'good_file.txt')
end
end

test 'unreadable files and fifos are not downloadable' do
Dir.mktmpdir do |dir|
cant_read = 'cant_read.txt'
fifo = 'fifo'

`touch #{dir}/#{cant_read}`
`chmod 000 #{dir}/#{cant_read}`
`mkfifo #{dir}/#{fifo}`

visit files_url(dir)

fifo_row = find('tbody a', exact_text: fifo).ancestor('tr')
cant_read_row = find('tbody a', exact_text: cant_read).ancestor('tr')

fifo_row.find('button.dropdown-toggle').click
fifo_links = fifo_row.all('td > div.btn-group > ul > li > a').map(&:text)

cant_read_row.find('button.dropdown-toggle').click
cant_read_links = cant_read_row.all('td > div.btn-group > ul > li > a').map(&:text)

# NOTE: download is not an expected link.
expected_links = ['View', 'Edit', 'Rename', 'Delete']

assert_equal(expected_links, fifo_links)
assert_equal(expected_links, cant_read_links)
end
end

test 'block devices are not downloadable' do
visit files_url('/dev')

null_row = find('tbody a', exact_text: 'null').ancestor('tr')
null_row.find('button.dropdown-toggle').click
null_links = null_row.all('td > div.btn-group > ul > li > a').map(&:text)

# NOTE: download is not an expected link.
expected_links = ['View', 'Edit', 'Rename', 'Delete']

assert_equal(expected_links, null_links)
end
end
2 changes: 1 addition & 1 deletion mod_ood_proxy/lib/node_proxy.lua
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ function node_proxy_handler(r)
local conn = {}
conn.user = user
conn.server = host .. ":" .. port
conn.uri = uri and (r.args and (uri .. "?" .. r.args) or uri) or r.unparsed_uri
conn.uri = uri and (r.args and (uri .. "?" .. r.args) or uri) or r.uri

-- last ditch effort to ensure that the uri is at least something
-- because the request-line of an HTTP request _has_ to have something for a URL
Expand Down
9 changes: 4 additions & 5 deletions mod_ood_proxy/lib/ood/proxy.lua
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,16 @@ function set_reverse_proxy(r, conn)
-- find protocol used by parsing the request headers
local protocol = (r.headers_in['Upgrade'] and "ws://" or "http://")

-- setup request to use mod_proxy for the reverse proxy
r.handler = "proxy-server"
r.proxyreq = apache2.PROXYREQ_REVERSE

-- define reverse proxy destination using connection object
if conn.socket then
r.filename = "proxy:unix:" .. conn.socket .. "|" .. protocol .. "localhost" .. conn.uri
r.handler = "proxy:unix:" .. conn.socket .. "|" .. protocol .. "localhost"
else
r.filename = "proxy:" .. protocol .. conn.server .. conn.uri
r.handler = "proxy:" .. protocol .. conn.server
end

r.filename = conn.uri

-- include useful information for the backend server

-- provide the protocol used
Expand Down
2 changes: 1 addition & 1 deletion mod_ood_proxy/lib/pun_proxy.lua
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function pun_proxy_handler(r)
local conn = {}
conn.user = user
conn.socket = pun_socket_root .. "/" .. user .. "/passenger.sock"
conn.uri = r.unparsed_uri
conn.uri = r.uri

-- start up PUN if socket doesn't exist
local err = nil
Expand Down
1 change: 1 addition & 0 deletions ood-portal-generator/spec/fixtures/ood-portal.conf.dex
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
RewriteCond /var/www/ood/public/maintenance/index.html -f
RewriteCond /etc/ood/maintenance.enable -f
RewriteCond %{REQUEST_URI} !/public/maintenance/.*$
RewriteCond %{REQUEST_URI} !/dex.*$
RewriteRule ^.*$ /public/maintenance/index.html [R=302,L]

TraceEnable off
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
RewriteCond /var/www/ood/public/maintenance/index.html -f
RewriteCond /etc/ood/maintenance.enable -f
RewriteCond %{REQUEST_URI} !/public/maintenance/.*$
RewriteCond %{REQUEST_URI} !/dex.*$
RewriteRule ^.*$ /public/maintenance/index.html [R=302,L]

TraceEnable off
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
RewriteCond /var/www/ood/public/maintenance/index.html -f
RewriteCond /etc/ood/maintenance.enable -f
RewriteCond %{REQUEST_URI} !/public/maintenance/.*$
RewriteCond %{REQUEST_URI} !/dex.*$
RewriteRule ^.*$ /public/maintenance/index.html [R=302,L]

TraceEnable off
Expand Down
Loading
Loading