diff --git a/devdocs/todo.txt b/devdocs/todo.txt
index 70e16496..097c1ff3 100644
--- a/devdocs/todo.txt
+++ b/devdocs/todo.txt
@@ -10,7 +10,13 @@ todo: check attachment NOTES property is actually supported
//todo: search tables in schema, I think there is a missing index here, need to look at the search query section again as it was changed several times from the original schema creation
-
+todo: log failed
+ - Download attempts with wrong key
+ - Add delay for failed download
+ - authentication attempts (or would this cause a possible DOS?)
+todo: some kind of ops specific event log or system that triggers alerts to ops people, I guess notifications would be the word I'm looking for...
+ - Once that exists a lot of backfilling needs to be done for example attachment files missing notification etc etc etc
+
todo: can a user be locked out from the server end even though they posess a valid token?
- and prevent download of images etc?
diff --git a/server/AyaNova/Controllers/AttachmentController.cs b/server/AyaNova/Controllers/AttachmentController.cs
index 46d2580b..2985f7db 100644
--- a/server/AyaNova/Controllers/AttachmentController.cs
+++ b/server/AyaNova/Controllers/AttachmentController.cs
@@ -36,7 +36,6 @@ namespace AyaNova.Api.Controllers
[ApiVersion("8.0")]
[Route("api/v{version:apiVersion}/[controller]")]
[Produces("application/json")]
- [Authorize]
public class AttachmentController : ControllerBase
{
private readonly AyContext ct;
@@ -118,6 +117,7 @@ namespace AyaNova.Api.Controllers
///
///
/// NameValue list of filenames and attachment id's
+ [Authorize]
[HttpPost]
[DisableFormValueModelBinding]
[RequestSizeLimit(10737418241)]//10737418240 = 10gb https://github.com/aspnet/Announcements/issues/267
@@ -269,6 +269,7 @@ namespace AyaNova.Api.Controllers
///
///
/// Ok
+ [Authorize]
[HttpDelete("{id}")]
public async Task DeleteAttachmentAsync([FromRoute] long id)
{
@@ -313,11 +314,14 @@ namespace AyaNova.Api.Controllers
/// Download a file attachment
///
///
- ///
+ /// download token
///
[HttpGet("download/{id}")]
- public async Task DownloadAsync([FromRoute] long id, [FromQuery] string dlkey)
+ public async Task DownloadAsync([FromRoute] long id, [FromQuery] string t)
{
+ int nFailedAuthDelay = 3000;//should be just long enough to make brute force a hassle but short enough to not annoy people who just mistyped their creds to login
+
+ //NOTE this is the only unauthorized route as it needs to work with wiki url links and relies on the dlkey to work
//copied from Rockfish
//https://dotnetcoretutorials.com/2017/03/12/uploading-files-asp-net-core/
//https://stackoverflow.com/questions/45763149/asp-net-core-jwt-in-uri-query-parameter/45811270#45811270
@@ -325,47 +329,57 @@ namespace AyaNova.Api.Controllers
return StatusCode(503, new ApiErrorResponse(serverState.ApiErrorCode, null, serverState.Reason));
- if (string.IsNullOrWhiteSpace(dlkey))
+ //NOTE: this is a potentially dangerous route since it's not Authorized so we need to treat it like Auth route and not leak any
+ //useful information to bad actors and also ensure a delay to avoid brute force or DOS attacks
+
+ if (string.IsNullOrWhiteSpace(t))
{
- return NotFound();
+ await Task.Delay(nFailedAuthDelay);//DOS protection
+ return StatusCode(401, new ApiErrorResponse(ApiErrorCode.AUTHENTICATION_FAILED));
}
+
+
+
//get user by key, if not found then reject
//If user dlkeyexp has not expired then return file
- var dlkeyUser = await ct.User.SingleOrDefaultAsync(m => m.DlKey == dlkey);
+ var dlkeyUser = await ct.User.SingleOrDefaultAsync(m => m.DlKey == t && m.Active == true);
if (dlkeyUser == null)
{
- //don't want to leak information so just say not found
- //return BadRequest(new ApiErrorResponse(ApiErrorCode.NOT_AUTHORIZED, "dlkey", "Download token not valid"));
+ await Task.Delay(nFailedAuthDelay);//DOS protection
return StatusCode(401, new ApiErrorResponse(ApiErrorCode.AUTHENTICATION_FAILED));
}
//Make sure the token provided is for the current user
+ //bugbug: Is this not a bug, there's no way to set this properly is there?
long UserId = UserIdFromContext.Id(HttpContext.Items);
if (UserId != dlkeyUser.Id)
{
- // return BadRequest(new ApiErrorResponse(ApiErrorCode.NOT_AUTHORIZED, "dlkey", "Download token not valid"));
- return StatusCode(401, new ApiErrorResponse(ApiErrorCode.AUTHENTICATION_FAILED));
+ await Task.Delay(nFailedAuthDelay);//DOS protection
+ return StatusCode(401, new ApiErrorResponse(ApiErrorCode.AUTHENTICATION_FAILED));
}
var utcNow = new DateTimeOffset(DateTime.Now.ToUniversalTime(), TimeSpan.Zero);
if (dlkeyUser.DlKeyExpire < utcNow.DateTime)
{
- // return BadRequest(new ApiErrorResponse(ApiErrorCode.NOT_AUTHORIZED, "dlkey", "Download token has expired"));
- return StatusCode(401, new ApiErrorResponse(ApiErrorCode.AUTHENTICATION_FAILED));
+
+ await Task.Delay(nFailedAuthDelay);//DOS protection
+ return StatusCode(401, new ApiErrorResponse(ApiErrorCode.AUTHENTICATION_FAILED));
}
//Ok, user has a valid download key and it's not expired yet so get the attachment record
var dbObj = await ct.FileAttachment.SingleOrDefaultAsync(m => m.Id == id);
if (dbObj == null)
{
+ await Task.Delay(nFailedAuthDelay);//fishing protection
return NotFound(new ApiErrorResponse(ApiErrorCode.NOT_FOUND));
}
//is this allowed?
if (!Authorized.HasReadFullRole(HttpContext.Items, dbObj.AttachToObjectType))
{
+ await Task.Delay(nFailedAuthDelay);//DOS protection
return StatusCode(403, new ApiNotAuthorizedResponse());
}
@@ -376,7 +390,11 @@ namespace AyaNova.Api.Controllers
{
//TODO: this should trigger some kind of notification to the ops people
//and a red light on the dashboard
- return NotFound(new ApiErrorResponse(ApiErrorCode.NOT_FOUND, null, $"Physical file {dbObj.StoredFileName} not found despite attachment record, this file is missing"));
+
+ var errText = $"Physical file {dbObj.StoredFileName} not found despite attachment record, this file is missing";
+ log.LogError(errText);
+
+ return NotFound(new ApiErrorResponse(ApiErrorCode.NOT_FOUND, null, errText));
}
//Log
diff --git a/server/AyaNova/Controllers/AuthController.cs b/server/AyaNova/Controllers/AuthController.cs
index f45229ae..6e1ed7df 100644
--- a/server/AyaNova/Controllers/AuthController.cs
+++ b/server/AyaNova/Controllers/AuthController.cs
@@ -177,16 +177,6 @@ namespace AyaNova.Api.Controllers
}
- // //If the user is inactive they may not login
- // if (!u.Active)
- // {
- // //This is leaking information, instead just act like bad creds
- // //return StatusCode(401, new ApiErrorResponse(ApiErrorCode.NOT_AUTHORIZED, null, "User deactivated"));
- // return StatusCode(401, new ApiErrorResponse(ApiErrorCode.AUTHENTICATION_FAILED));
- // }
-
-
-
//build the key (JWT set in startup.cs)
byte[] secretKey = System.Text.Encoding.ASCII.GetBytes(ServerBootConfig.AYANOVA_JWT_SECRET);