diff --git a/apps/dashboard/app/controllers/files_controller.rb b/apps/dashboard/app/controllers/files_controller.rb index c43be62547..8f8b7a37b0 100644 --- a/apps/dashboard/app/controllers/files_controller.rb +++ b/apps/dashboard/app/controllers/files_controller.rb @@ -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 diff --git a/apps/dashboard/app/javascript/files/data_table.js b/apps/dashboard/app/javascript/files/data_table.js index 5c30607de3..ef9acf7a6d 100644 --- a/apps/dashboard/app/javascript/files/data_table.js +++ b/apps/dashboard/app/javascript/files/data_table.js @@ -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 @@ -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 { diff --git a/apps/dashboard/app/javascript/files/file_ops.js b/apps/dashboard/app/javascript/files/file_ops.js index 082ab2233c..d6207baaa4 100644 --- a/apps/dashboard/app/javascript/files/file_ops.js +++ b/apps/dashboard/app/javascript/files/file_ops.js @@ -27,6 +27,7 @@ const EVENTNAME = { let fileOps = null; +export {fileOps}; jQuery(function() { fileOps = new FileOps(); diff --git a/apps/dashboard/app/javascript/files/uppy_ops.js b/apps/dashboard/app/javascript/files/uppy_ops.js index bd5c1053a2..5a38595a51 100755 --- a/apps/dashboard/app/javascript/files/uppy_ops.js +++ b/apps/dashboard/app/javascript/files/uppy_ops.js @@ -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; @@ -102,6 +103,7 @@ jQuery(function() { uppy.on('complete', (result) => { if(result.successful.length > 0){ + result.successful.forEach(handleUploadSuccess); reloadTable(); } }); @@ -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); + } +} + diff --git a/apps/dashboard/app/models/posix_file.rb b/apps/dashboard/app/models/posix_file.rb index effbf44921..eb9574eff0 100644 --- a/apps/dashboard/app/models/posix_file.rb +++ b/apps/dashboard/app/models/posix_file.rb @@ -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 @@ -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 diff --git a/apps/dashboard/app/models/remote_file.rb b/apps/dashboard/app/models/remote_file.rb index 5c669168c6..383f8edd0e 100644 --- a/apps/dashboard/app/models/remote_file.rb +++ b/apps/dashboard/app/models/remote_file.rb @@ -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? @@ -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 @@ -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 diff --git a/apps/dashboard/app/models/remote_transfer.rb b/apps/dashboard/app/models/remote_transfer.rb index d24b0c2e82..2ad8b1c2eb 100644 --- a/apps/dashboard/app/models/remote_transfer.rb +++ b/apps/dashboard/app/models/remote_transfer.rb @@ -43,7 +43,7 @@ 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 @@ -51,7 +51,7 @@ def transfers 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: @@ -59,8 +59,7 @@ def build(action:, files:, src_remote:, dest_remote:) # 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 @@ -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 diff --git a/apps/dashboard/app/views/files/_file_action_menu.html.erb b/apps/dashboard/app/views/files/_file_action_menu.html.erb index cf386678e7..cc3f7f7275 100755 --- a/apps/dashboard/app/views/files/_file_action_menu.html.erb +++ b/apps/dashboard/app/views/files/_file_action_menu.html.erb @@ -14,7 +14,9 @@ {{/if}}
  • Rename
  • <%- if Configuration.download_enabled? -%> + {{#if data.download_url}}
  • Download
  • + {{/if}} <%- end -%>
  • Delete
  • diff --git a/apps/dashboard/app/views/files/index.json.jbuilder b/apps/dashboard/app/views/files/index.json.jbuilder index 860f84b224..54965c3059 100644 --- a/apps/dashboard/app/views/files/index.json.jbuilder +++ b/apps/dashboard/app/views/files/index.json.jbuilder @@ -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] diff --git a/apps/dashboard/test/models/files_test.rb b/apps/dashboard/test/models/files_test.rb index d16e672d36..4d0a7695cd 100644 --- a/apps/dashboard/test/models/files_test.rb +++ b/apps/dashboard/test/models/files_test.rb @@ -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 diff --git a/apps/dashboard/test/system/files_test.rb b/apps/dashboard/test/system/files_test.rb index 862d6b41bf..e4e73b85cd 100644 --- a/apps/dashboard/test/system/files_test.rb +++ b/apps/dashboard/test/system/files_test.rb @@ -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 diff --git a/mod_ood_proxy/lib/node_proxy.lua b/mod_ood_proxy/lib/node_proxy.lua index 678ae6d4ad..705d6f86d9 100644 --- a/mod_ood_proxy/lib/node_proxy.lua +++ b/mod_ood_proxy/lib/node_proxy.lua @@ -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 diff --git a/mod_ood_proxy/lib/ood/proxy.lua b/mod_ood_proxy/lib/ood/proxy.lua index 08a13ed54c..81ce24c442 100644 --- a/mod_ood_proxy/lib/ood/proxy.lua +++ b/mod_ood_proxy/lib/ood/proxy.lua @@ -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 diff --git a/mod_ood_proxy/lib/pun_proxy.lua b/mod_ood_proxy/lib/pun_proxy.lua index c47dc64657..0062740350 100644 --- a/mod_ood_proxy/lib/pun_proxy.lua +++ b/mod_ood_proxy/lib/pun_proxy.lua @@ -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 diff --git a/ood-portal-generator/spec/fixtures/ood-portal.conf.dex b/ood-portal-generator/spec/fixtures/ood-portal.conf.dex index 57fdbe4e9d..696222f405 100644 --- a/ood-portal-generator/spec/fixtures/ood-portal.conf.dex +++ b/ood-portal-generator/spec/fixtures/ood-portal.conf.dex @@ -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 diff --git a/ood-portal-generator/spec/fixtures/ood-portal.conf.dex-full b/ood-portal-generator/spec/fixtures/ood-portal.conf.dex-full index 195f8c4f82..9b5ece02be 100644 --- a/ood-portal-generator/spec/fixtures/ood-portal.conf.dex-full +++ b/ood-portal-generator/spec/fixtures/ood-portal.conf.dex-full @@ -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 diff --git a/ood-portal-generator/spec/fixtures/ood-portal.conf.dex-ldap b/ood-portal-generator/spec/fixtures/ood-portal.conf.dex-ldap index c02e457631..6fea66475a 100644 --- a/ood-portal-generator/spec/fixtures/ood-portal.conf.dex-ldap +++ b/ood-portal-generator/spec/fixtures/ood-portal.conf.dex-ldap @@ -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 diff --git a/ood-portal-generator/spec/fixtures/ood-portal.dex-full.proxy.conf b/ood-portal-generator/spec/fixtures/ood-portal.dex-full.proxy.conf index 304f84ff29..54a206cdb9 100644 --- a/ood-portal-generator/spec/fixtures/ood-portal.dex-full.proxy.conf +++ b/ood-portal-generator/spec/fixtures/ood-portal.dex-full.proxy.conf @@ -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 diff --git a/ood-portal-generator/templates/ood-portal.conf.erb b/ood-portal-generator/templates/ood-portal.conf.erb index ff4898374b..dfdfb5fab2 100644 --- a/ood-portal-generator/templates/ood-portal.conf.erb +++ b/ood-portal-generator/templates/ood-portal.conf.erb @@ -101,6 +101,9 @@ Listen <%= addr_port %> RewriteCond <%= @public_root %>/maintenance/index.html -f RewriteCond /etc/ood/maintenance.enable -f RewriteCond %{REQUEST_URI} !<%= @public_uri %>/maintenance/.*$ + <%- if @dex_uri && @dex_enabled -%> + RewriteCond %{REQUEST_URI} !<%= @dex_uri %>.*$ + <%- end -%> <%- @maintenance_ip_allowlist.each do |ip| -%> RewriteCond %{REMOTE_ADDR} !^<%= escape_ip(ip) %> <%- end -%> diff --git a/ood_auth_map/lib/ood_auth_map/helpers.rb b/ood_auth_map/lib/ood_auth_map/helpers.rb index f18a0de1e7..42434e2051 100644 --- a/ood_auth_map/lib/ood_auth_map/helpers.rb +++ b/ood_auth_map/lib/ood_auth_map/helpers.rb @@ -9,7 +9,7 @@ class << self # @param auth_user [String] authenticated username # @return [String, nil] mapped user name or {nil} if no match def parse_mapfile(file, auth_user) - parse_file(file, %r[^"#{Regexp.quote auth_user}" (\w+)$]) + parse_file(file, %r[^"#{Regexp.quote auth_user}" ([\w\.-@]+)$]) end # Parse a file using a given regular expression pattern and output the