Improve validation and error handling in Controller methods

This commit is contained in:
Shaun Walker
2021-06-07 15:29:08 -04:00
parent 54cd360bb5
commit 82c05a841f
38 changed files with 922 additions and 435 deletions

View File

@ -30,14 +30,16 @@ namespace Oqtane.Controllers
private readonly IFolderRepository _folders;
private readonly IUserPermissions _userPermissions;
private readonly ILogManager _logger;
private readonly Alias _alias;
public FileController(IWebHostEnvironment environment, IFileRepository files, IFolderRepository folders, IUserPermissions userPermissions, ILogManager logger)
public FileController(IWebHostEnvironment environment, IFileRepository files, IFolderRepository folders, IUserPermissions userPermissions, ILogManager logger, ITenantManager tenantManager)
{
_environment = environment;
_files = files;
_folders = folders;
_userPermissions = userPermissions;
_logger = logger;
_alias = tenantManager.GetAlias();
}
// GET: api/<controller>?folder=x
@ -48,11 +50,17 @@ namespace Oqtane.Controllers
int folderid;
if (int.TryParse(folder, out folderid))
{
Folder f = _folders.GetFolder(folderid);
if (f != null && _userPermissions.IsAuthorized(User, PermissionNames.Browse, f.Permissions))
Folder Folder = _folders.GetFolder(folderid);
if (Folder != null && Folder.SiteId == _alias.SiteId && _userPermissions.IsAuthorized(User, PermissionNames.Browse, Folder.Permissions))
{
files = _files.GetFiles(folderid).ToList();
}
else
{
_logger.Log(LogLevel.Error, this, LogFunction.Security, "Unauthorized File Get Attempt {FolderId}", folder);
HttpContext.Response.StatusCode = (int)HttpStatusCode.Forbidden;
files = null;
}
}
else
{
@ -67,6 +75,12 @@ namespace Oqtane.Controllers
}
}
}
else
{
_logger.Log(LogLevel.Error, this, LogFunction.Security, "Unauthorized File Get Attempt {Folder}", folder);
HttpContext.Response.StatusCode = (int)HttpStatusCode.Forbidden;
files = null;
}
}
return files;
@ -76,27 +90,18 @@ namespace Oqtane.Controllers
[HttpGet("{siteId}/{path}")]
public IEnumerable<Models.File> Get(int siteId, string path)
{
var folderPath = WebUtility.UrlDecode(path);
Folder folder = _folders.GetFolder(siteId, folderPath);
List<Models.File> files;
if (folder != null)
Folder folder = _folders.GetFolder(siteId, WebUtility.UrlDecode(path));
if (folder != null && folder.SiteId == _alias.SiteId && _userPermissions.IsAuthorized(User, PermissionNames.Browse, folder.Permissions))
{
if (_userPermissions.IsAuthorized(User, PermissionNames.Browse, folder.Permissions))
{
files = _files.GetFiles(folder.FolderId).ToList();
}
else
{
_logger.Log(LogLevel.Error, this, LogFunction.Read, "User Not Authorized To Access Folder {folder}", folder);
HttpContext.Response.StatusCode = 401;
return null;
}
files = _files.GetFiles(folder.FolderId).ToList();
}
else
{
_logger.Log(LogLevel.Error, this, LogFunction.Read, "Folder Not Found {SiteId} {Path}", siteId, path);
HttpContext.Response.StatusCode = 404;
return null;
_logger.Log(LogLevel.Error, this, LogFunction.Security, "Unauthorized File Get Attempt {SiteId} {Path}", siteId, path);
HttpContext.Response.StatusCode = (int)HttpStatusCode.Forbidden;
files = null;
}
return files;
@ -107,23 +112,14 @@ namespace Oqtane.Controllers
public Models.File Get(int id)
{
Models.File file = _files.GetFile(id);
if (file != null)
if (file != null && file.Folder.SiteId == _alias.SiteId && _userPermissions.IsAuthorized(User, PermissionNames.View, file.Folder.Permissions))
{
if (_userPermissions.IsAuthorized(User, PermissionNames.View, file.Folder.Permissions))
{
return file;
}
else
{
_logger.Log(LogLevel.Error, this, LogFunction.Read, "User Not Authorized To Access File {File}", file);
HttpContext.Response.StatusCode = 401;
return null;
}
return file;
}
else
{
_logger.Log(LogLevel.Error, this, LogFunction.Read, "File Not Found {FileId}", id);
HttpContext.Response.StatusCode = 404;
_logger.Log(LogLevel.Error, this, LogFunction.Security, "Unauthorized File Get Attempt {FileId}", id);
HttpContext.Response.StatusCode = (int)HttpStatusCode.Forbidden;
return null;
}
}
@ -133,19 +129,17 @@ namespace Oqtane.Controllers
[Authorize(Roles = RoleNames.Registered)]
public Models.File Put(int id, [FromBody] Models.File file)
{
if (ModelState.IsValid && _userPermissions.IsAuthorized(User, EntityNames.Folder, file.FolderId, PermissionNames.Edit))
var File = _files.GetFile(file.FileId, false);
if (ModelState.IsValid && File != null && File.Folder.SiteId == _alias.SiteId && _userPermissions.IsAuthorized(User, EntityNames.Folder, file.FolderId, PermissionNames.Edit))
{
file.Folder = _folders.GetFolder(file.FolderId);
Models.File _file = _files.GetFile(id, false);
if (_file.Name != file.Name || _file.FolderId != file.FolderId)
if (File.Name != file.Name || File.FolderId != file.FolderId)
{
string folderpath = _folders.GetFolderPath(file.Folder);
if (!Directory.Exists(folderpath))
{
Directory.CreateDirectory(folderpath);
}
System.IO.File.Move(_files.GetFilePath(_file), Path.Combine(folderpath, file.Name));
System.IO.File.Move(_files.GetFilePath(File), Path.Combine(folderpath, file.Name));
}
file.Extension = Path.GetExtension(file.Name).ToLower().Replace(".", "");
@ -154,8 +148,8 @@ namespace Oqtane.Controllers
}
else
{
_logger.Log(LogLevel.Error, this, LogFunction.Update, "User Not Authorized To Update File {File}", file);
HttpContext.Response.StatusCode = 401;
_logger.Log(LogLevel.Error, this, LogFunction.Security, "Unauthorized File Put Attempt {File}", file);
HttpContext.Response.StatusCode = (int)HttpStatusCode.Forbidden;
file = null;
}
@ -168,30 +162,22 @@ namespace Oqtane.Controllers
public void Delete(int id)
{
Models.File file = _files.GetFile(id);
if (file != null)
if (file != null && file.Folder.SiteId == _alias.SiteId && _userPermissions.IsAuthorized(User, EntityNames.Folder, file.Folder.FolderId, PermissionNames.Edit))
{
if (_userPermissions.IsAuthorized(User, EntityNames.Folder, file.Folder.FolderId, PermissionNames.Edit))
{
_files.DeleteFile(id);
_files.DeleteFile(id);
string filepath = _files.GetFilePath(file);
if (System.IO.File.Exists(filepath))
{
System.IO.File.Delete(filepath);
}
_logger.Log(LogLevel.Information, this, LogFunction.Delete, "File Deleted {File}", file);
}
else
string filepath = _files.GetFilePath(file);
if (System.IO.File.Exists(filepath))
{
_logger.Log(LogLevel.Error, this, LogFunction.Delete, "User Not Authorized To Delete File {FileId}", id);
HttpContext.Response.StatusCode = 401;
System.IO.File.Delete(filepath);
}
_logger.Log(LogLevel.Information, this, LogFunction.Delete, "File Deleted {File}", file);
}
else
{
_logger.Log(LogLevel.Error, this, LogFunction.Delete, "File Not Found {FileId}", id);
HttpContext.Response.StatusCode = 404;
_logger.Log(LogLevel.Error, this, LogFunction.Security, "Unauthorized File Delete Attempt {FileId}", id);
HttpContext.Response.StatusCode = (int)HttpStatusCode.Forbidden;
}
}
@ -200,55 +186,57 @@ namespace Oqtane.Controllers
public Models.File UploadFile(string url, string folderid)
{
Models.File file = null;
Folder folder = _folders.GetFolder(int.Parse(folderid));
if (folder == null || !_userPermissions.IsAuthorized(User, PermissionNames.Edit, folder.Permissions))
Folder folder = null;
int FolderId;
if (int.TryParse(folderid, out FolderId))
{
_logger.Log(LogLevel.Error, this, LogFunction.Create,
"User Not Authorized To Download File {Url} {FolderId}", url, folderid);
HttpContext.Response.StatusCode = 401;
return file;
folder = _folders.GetFolder(FolderId);
}
string folderPath = _folders.GetFolderPath(folder);
CreateDirectory(folderPath);
string filename = url.Substring(url.LastIndexOf("/", StringComparison.Ordinal) + 1);
// check for allowable file extensions
if (!Constants.UploadableFiles.Split(',')
.Contains(Path.GetExtension(filename).ToLower().Replace(".", "")))
if (folder != null && folder.SiteId == _alias.SiteId && _userPermissions.IsAuthorized(User, PermissionNames.Edit, folder.Permissions))
{
_logger.Log(LogLevel.Error, this, LogFunction.Create,
"File Could Not Be Downloaded From Url Due To Its File Extension {Url}", url);
HttpContext.Response.StatusCode = (int) HttpStatusCode.Conflict;
return file;
}
string folderPath = _folders.GetFolderPath(folder);
CreateDirectory(folderPath);
if (!filename.IsPathOrFileValid())
{
_logger.Log(LogLevel.Error, this, LogFunction.Create,
$"File Could Not Be Downloaded From Url Due To Its File Name Not Allowed {url}");
HttpContext.Response.StatusCode = (int) HttpStatusCode.Conflict;
return file;
}
try
{
var client = new WebClient();
string targetPath = Path.Combine(folderPath, filename);
// remove file if it already exists
if (System.IO.File.Exists(targetPath))
string filename = url.Substring(url.LastIndexOf("/", StringComparison.Ordinal) + 1);
// check for allowable file extensions
if (!Constants.UploadableFiles.Split(',').Contains(Path.GetExtension(filename).ToLower().Replace(".", "")))
{
System.IO.File.Delete(targetPath);
_logger.Log(LogLevel.Error, this, LogFunction.Create, "File Could Not Be Downloaded From Url Due To Its File Extension {Url}", url);
HttpContext.Response.StatusCode = (int)HttpStatusCode.Conflict;
return file;
}
client.DownloadFile(url, targetPath);
file = _files.AddFile(CreateFile(filename, folder.FolderId, targetPath));
if (!filename.IsPathOrFileValid())
{
_logger.Log(LogLevel.Error, this, LogFunction.Create, $"File Could Not Be Downloaded From Url Due To Its File Name Not Allowed {url}");
HttpContext.Response.StatusCode = (int)HttpStatusCode.Conflict;
return file;
}
try
{
var client = new WebClient();
string targetPath = Path.Combine(folderPath, filename);
// remove file if it already exists
if (System.IO.File.Exists(targetPath))
{
System.IO.File.Delete(targetPath);
}
client.DownloadFile(url, targetPath);
file = _files.AddFile(CreateFile(filename, folder.FolderId, targetPath));
}
catch
{
_logger.Log(LogLevel.Error, this, LogFunction.Create, "File Could Not Be Downloaded From Url {Url}", url);
}
}
catch
else
{
_logger.Log(LogLevel.Error, this, LogFunction.Create,
"File Could Not Be Downloaded From Url {Url}", url);
_logger.Log(LogLevel.Error, this, LogFunction.Security, "Unauthorized File Upload Attempt {FolderId} {Url}", folderid, url);
HttpContext.Response.StatusCode = (int)HttpStatusCode.Forbidden;
}
return file;
@ -271,25 +259,25 @@ namespace Oqtane.Controllers
string folderPath = "";
if (int.TryParse(folder, out int folderId))
int FolderId;
if (int.TryParse(folder, out FolderId))
{
Folder virtualFolder = _folders.GetFolder(folderId);
if (virtualFolder != null &&
_userPermissions.IsAuthorized(User, PermissionNames.Edit, virtualFolder.Permissions))
Folder Folder = _folders.GetFolder(FolderId);
if (Folder != null && Folder.SiteId == _alias.SiteId && _userPermissions.IsAuthorized(User, PermissionNames.Edit, Folder.Permissions))
{
folderPath = _folders.GetFolderPath(virtualFolder);
folderPath = _folders.GetFolderPath(Folder);
}
}
else
{
FolderId = -1;
if (User.IsInRole(RoleNames.Host))
{
folderPath = GetFolderPath(folder);
folderId = -1;
}
}
if (!String.IsNullOrEmpty(folderPath))
if (!string.IsNullOrEmpty(folderPath))
{
CreateDirectory(folderPath);
using (var stream = new FileStream(Path.Combine(folderPath, file.FileName), FileMode.Create))
@ -298,16 +286,15 @@ namespace Oqtane.Controllers
}
string upload = await MergeFile(folderPath, file.FileName);
if (upload != "" && folderId != -1)
if (upload != "" && FolderId != -1)
{
_files.AddFile(CreateFile(upload, folderId, Path.Combine(folderPath, upload)));
_files.AddFile(CreateFile(upload, FolderId, Path.Combine(folderPath, upload)));
}
}
else
{
_logger.Log(LogLevel.Error, this, LogFunction.Create,
"User Not Authorized To Upload File {Folder} {File}", folder, file);
HttpContext.Response.StatusCode = 401;
_logger.Log(LogLevel.Error, this, LogFunction.Security, "Unauthorized File Upload Attempt {Folder} {File}", folder, file);
HttpContext.Response.StatusCode = (int)HttpStatusCode.Forbidden;
}
}
@ -466,32 +453,26 @@ namespace Oqtane.Controllers
private IActionResult Download(int id, bool asAttachment)
{
var file = _files.GetFile(id);
if (file != null)
if (file != null && file.Folder.SiteId == _alias.SiteId && _userPermissions.IsAuthorized(User, PermissionNames.View, file.Folder.Permissions))
{
if (_userPermissions.IsAuthorized(User, PermissionNames.View, file.Folder.Permissions))
var filepath = _files.GetFilePath(file);
if (System.IO.File.Exists(filepath))
{
var filepath = _files.GetFilePath(file);
if (System.IO.File.Exists(filepath))
{
var result = asAttachment
? PhysicalFile(filepath, file.GetMimeType(), file.Name)
: PhysicalFile(filepath, file.GetMimeType());
return result;
}
_logger.Log(LogLevel.Error, this, LogFunction.Read, "File Does Not Exist {FileId} {FilePath}", id, filepath);
HttpContext.Response.StatusCode = 404;
var result = asAttachment
? PhysicalFile(filepath, file.GetMimeType(), file.Name)
: PhysicalFile(filepath, file.GetMimeType());
return result;
}
else
{
_logger.Log(LogLevel.Error, this, LogFunction.Read, "User Not Authorized To Access File {FileId}", id);
HttpContext.Response.StatusCode = 401;
_logger.Log(LogLevel.Error, this, LogFunction.Read, "File Does Not Exist {FileId} {FilePath}", id, filepath);
HttpContext.Response.StatusCode = (int)HttpStatusCode.NotFound;
}
}
else
{
_logger.Log(LogLevel.Error, this, LogFunction.Read, "File Not Found {FileId}", id);
HttpContext.Response.StatusCode = 404;
_logger.Log(LogLevel.Error, this, LogFunction.Security, "Unauthorized File Access Attempt {FileId}", id);
HttpContext.Response.StatusCode = (int)HttpStatusCode.Forbidden;
}
string errorPath = Path.Combine(GetFolderPath("images"), "error.png");