Host team avatars instead of only linking them

app/org/maproulette/framework/controller/TeamController.scala

@@ -8,8 +8,13 @@ package org.maproulette.framework.controller
 import javax.inject.Inject
 import org.maproulette.data.ActionManager
 import org.maproulette.exception.{MPExceptionUtil, StatusMessage}
-import org.maproulette.framework.mixins.ParentMixin
-import org.maproulette.framework.service.{ServiceManager, TeamImageService, TeamService}
+import org.maproulette.framework.mixins.{ImageUploadMixin, ParentMixin}
+import org.maproulette.framework.service.{
+  ServiceManager,
+  TeamAvatarService,
+  TeamImageService,
+  TeamService
+}
 import org.maproulette.framework.model.{
   Challenge,
   Group,
@@ -20,6 +25,7 @@ import org.maproulette.framework.model.{
 }
 import org.maproulette.framework.psql.{Paging}
 import org.maproulette.session.SessionManager
+import play.api.libs.Files
 import play.api.libs.json._
 import play.api.mvc._
 
@@ -33,13 +39,17 @@ class TeamController @Inject() (
     val serviceManager: ServiceManager,
     teamService: TeamService,
     teamImageService: TeamImageService,
+    teamAvatarService: TeamAvatarService,
     components: ControllerComponents
 ) extends AbstractController(components)
     with MapRouletteController
+    with ImageUploadMixin
     with ParentMixin {
 
   implicit val challengeWrites: Writes[Challenge] = Challenge.writes.challengeWrites
 
+  private val maxUploadBytes = TeamImage.MAX_UPLOAD_BYTES
+
   /**
     * Create a new team
     */
@@ -394,4 +404,56 @@ class TeamController @Inject() (
       }
     }
   }
+
+  /**
+    * Uploads a team's avatar, replacing whatever avatar it had.
+    *
+    * @param teamId The id of the team whose avatar is being set
+    * @return 200 OK with the updated team
+    */
+  def uploadAvatar(teamId: Long): Action[MultipartFormData[Files.TemporaryFile]] =
+    Action.async(parse.multipartFormData(maxLength = maxUploadBytes)) { implicit request =>
+      this.sessionManager.authenticatedRequest { implicit user =>
+        val upload = this.readImageUpload(request)
+        Ok(
+          Json.toJson(
+            this.teamAvatarService.upload(teamId, upload.data, upload.contentType, user)
+          )
+        )
+      }
+    }
+
+  /**
+    * Removes a team's uploaded avatar.
+    *
+    * @param teamId The id of the team whose avatar is being removed
+    * @return 200 OK with the updated team
+    */
+  def deleteAvatar(teamId: Long): Action[AnyContent] = Action.async { implicit request =>
+    this.sessionManager.authenticatedRequest { implicit user =>
+      Ok(Json.toJson(this.teamAvatarService.remove(teamId, user)))
+    }
+  }
+
+  /**
+    * Serves a team's avatar bytes. Anonymous, because the url is consumed by
+    * plain img tags wherever the team is shown.
+    *
+    * @param teamId The id of the team whose avatar to serve
+    * @return 200 OK with the avatar bytes
+    */
+  def getAvatarFile(teamId: Long): Action[AnyContent] = Action.async { implicit request =>
+    this.sessionManager.userAwareRequest { implicit user =>
+      val avatar = this.teamAvatarService.retrieve(teamId)
+      this.serveImageBytes(
+        etagKey = teamId.toString,
+        modified = avatar.modified,
+        contentType = avatar.contentType,
+        data = this.teamAvatarService.retrieveData(teamId).data,
+        // The url carries a `?v=<modified>` stamp, so the bytes behind any one
+        // url never change and a revalidation could only ever answer 304.
+        cacheControl = "public, max-age=31536000, immutable"
+      )
+    }
+  }
 }

app/org/maproulette/framework/controller/TeamImageController.scala

@@ -7,7 +7,8 @@ package org.maproulette.framework.controller
 
 import javax.inject.Inject
 import org.maproulette.data.ActionManager
-import org.maproulette.exception.{InvalidException, NotFoundException, StatusMessage}
+import org.maproulette.exception.{NotFoundException, StatusMessage}
+import org.maproulette.framework.mixins.ImageUploadMixin
 import org.maproulette.framework.model.TeamImage
 import org.maproulette.framework.service.TeamImageService
 import org.maproulette.permissions.Permission
@@ -29,13 +30,10 @@ class TeamImageController @Inject() (
     permission: Permission,
     components: ControllerComponents
 ) extends AbstractController(components)
-    with MapRouletteController {
+    with MapRouletteController
+    with ImageUploadMixin {
 
-  // Give the multipart parser a little room past the limit we enforce, so an
-  // upload that is merely too big still gets the friendly error below rather
-  // than the parser's, while a wildly oversized one is refused before it is
-  // buffered to disk.
-  private val maxUploadBytes = TeamImage.MAX_SIZE_BYTES * 2
+  private val maxUploadBytes = TeamImage.MAX_UPLOAD_BYTES
 
   /**
     * Requests a new image for a team. Any active member can ask; the image is
@@ -49,28 +47,7 @@ class TeamImageController @Inject() (
       this.sessionManager.authenticatedRequest { implicit user =>
         this.teamImageService.requireTeamAccess(teamId, user)
 
-        val upload = request.body
-          .file("image")
-          .getOrElse(throw new InvalidException("No image file provided in the 'image' field"))
-
-        if (upload.fileSize > TeamImage.MAX_SIZE_BYTES) {
-          throw new InvalidException(
-            s"Image is larger than the ${TeamImage.MAX_SIZE_BYTES / (1024 * 1024)}MB limit"
-          )
-        }
-
-        val data = java.nio.file.Files.readAllBytes(upload.ref.path)
-        // The declared content type is caller-supplied, so the leading bytes
-        // are what we actually trust before storing something we will later
-        // serve back from our own origin.
-        val contentType = TeamImage
-          .detectContentType(data)
-          .getOrElse(
-            throw new InvalidException(
-              s"Unsupported image format. Supported formats: ${TeamImage.ALLOWED_CONTENT_TYPES.toList.sorted
-                .mkString(", ")}"
-            )
-          )
+        val upload = this.readImageUpload(request)
 
         val name = request.body.dataParts
           .get("name")
@@ -80,7 +57,11 @@ class TeamImageController @Inject() (
           .orElse(Option(upload.filename).map(_.trim).filter(_.nonEmpty))
           .getOrElse("Untitled image")
 
-        Ok(Json.toJson(this.teamImageService.request(teamId, name, contentType, data, user.id)))
+        Ok(
+          Json.toJson(
+            this.teamImageService.request(teamId, name, upload.contentType, upload.data, user.id)
+          )
+        )
       }
     }
 
@@ -213,29 +194,21 @@ class TeamImageController @Inject() (
   }
 
   /**
-    * Writes an image's bytes out, answering a still-current cached copy with a
-    * 304 instead. The ETag is built from the metadata alone, so revalidating
-    * never reads the blob.
+    * Writes an image's bytes out through the shared serving rules.
     */
   private def serveImage(
       imageId: Long,
       request: Request[AnyContent],
       cacheControl: String
   ): Result = {
-    val file = this.teamImageService.retrieveFile(imageId)
-    val etag = s""""$imageId-${file.modified.getMillis}""""
-
-    if (request.headers.get("If-None-Match").contains(etag)) {
-      NotModified.withHeaders("ETag" -> etag)
-    } else {
-      Ok(this.teamImageService.retrieveData(imageId).data)
-        .as(file.contentType)
-        .withHeaders(
-          "ETag"                   -> etag,
-          "Cache-Control"          -> cacheControl,
-          "X-Content-Type-Options" -> "nosniff",
-          "Content-Disposition"    -> "inline"
-        )
-    }
+    implicit val r: Request[AnyContent] = request
+    val file                            = this.teamImageService.retrieveFile(imageId)
+    this.serveImageBytes(
+      etagKey = imageId.toString,
+      modified = file.modified,
+      contentType = file.contentType,
+      data = this.teamImageService.retrieveData(imageId).data,
+      cacheControl = cacheControl
+    )
   }
 }

app/org/maproulette/framework/mixins/ImageUploadMixin.scala

@@ -0,0 +1,112 @@
+/*
+ * Copyright (C) 2020 MapRoulette contributors (see CONTRIBUTORS.md).
+ * Licensed under the Apache License, Version 2.0 (see LICENSE).
+ */
+package org.maproulette.framework.mixins
+
+import org.joda.time.DateTime
+import org.maproulette.exception.InvalidException
+import org.maproulette.framework.model.TeamImage
+import play.api.libs.Files
+import play.api.mvc.{MultipartFormData, Request, Result, Results}
+
+/**
+  * An uploaded image that passed validation, with the filename it arrived
+  * under so callers can fall back to it when naming the image.
+  */
+case class UploadedImage(data: Array[Byte], contentType: String, filename: String)
+
+/**
+  * Shared mechanics for the endpoints that take an image in and hand one back.
+  *
+  * Both of the images a team can have - the moderated one its challenges use
+  * and the avatar it is shown by - accept the same formats under the same size
+  * limit and are served from our own origin under the same caching and
+  * content-type rules. Those rules live here so the two endpoints cannot
+  * answer differently, which matters most for the two security headers: a
+  * change made in one place and missed in the other is invisible until it is
+  * exploited rather than failing loudly.
+  */
+trait ImageUploadMixin extends Results {
+
+  /**
+    * Reads the uploaded image out of a multipart request, rejecting anything
+    * too large or not an image we serve.
+    *
+    * The size is checked before the bytes are read, so an oversized upload is
+    * turned away rather than pulled into memory first. The declared content
+    * type is caller-supplied and therefore not trusted: the leading bytes
+    * decide the format, since we will later serve this back from our own
+    * origin.
+    *
+    * @param request The multipart request carrying the image
+    * @param field   The form field the image is expected in
+    * @return The validated image
+    */
+  def readImageUpload(
+      request: Request[MultipartFormData[Files.TemporaryFile]],
+      field: String = "image"
+  ): UploadedImage = {
+    val upload = request.body
+      .file(field)
+      .getOrElse(throw new InvalidException(s"No image file provided in the '$field' field"))
+
+    if (upload.fileSize > TeamImage.MAX_SIZE_BYTES) {
+      throw new InvalidException(
+        s"Image is larger than the ${TeamImage.MAX_SIZE_BYTES / (1024 * 1024)}MB limit"
+      )
+    }
+
+    val data = java.nio.file.Files.readAllBytes(upload.ref.path)
+    val contentType = TeamImage
+      .detectContentType(data)
+      .getOrElse(
+        throw new InvalidException(
+          s"Unsupported image format. Supported formats: ${TeamImage.ALLOWED_CONTENT_TYPES.toList.sorted
+            .mkString(", ")}"
+        )
+      )
+
+    UploadedImage(data, contentType, upload.filename)
+  }
+
+  /**
+    * Writes an image's bytes out, answering a still-current cached copy with a
+    * 304 instead.
+    *
+    * The ETag is built from the identity and modification time alone, so the
+    * caller can settle a revalidation without ever loading the blob - hence
+    * `data` being by-name rather than a value.
+    *
+    * @param etagKey      Identifies the image within its own endpoint
+    * @param modified     When the image last changed
+    * @param contentType  The image's format
+    * @param data         The bytes, evaluated only when they are actually sent
+    * @param cacheControl How long, and to whom, the response may be cached
+    */
+  def serveImageBytes(
+      etagKey: String,
+      modified: DateTime,
+      contentType: String,
+      data: => Array[Byte],
+      cacheControl: String
+  )(implicit request: Request[_]): Result = {
+    val etag = s""""$etagKey-${modified.getMillis}""""
+
+    if (request.headers.get("If-None-Match").contains(etag)) {
+      NotModified.withHeaders("ETag" -> etag)
+    } else {
+      Ok(data)
+        .as(contentType)
+        .withHeaders(
+          "ETag"          -> etag,
+          "Cache-Control" -> cacheControl,
+          // Served from our own origin, so the browser must not be talked into
+          // treating these bytes as anything but the image we sniffed them to
+          // be, nor into opening them as a page.
+          "X-Content-Type-Options" -> "nosniff",
+          "Content-Disposition"    -> "inline"
+        )
+    }
+  }
+}

app/org/maproulette/framework/model/TeamAvatar.scala

@@ -0,0 +1,42 @@
+/*
+ * Copyright (C) 2020 MapRoulette contributors (see CONTRIBUTORS.md).
+ * Licensed under the Apache License, Version 2.0 (see LICENSE).
+ */
+package org.maproulette.framework.model
+
+import org.joda.time.DateTime
+
+/**
+  * A team's uploaded avatar. There is at most one per team, so the team id is
+  * the only identity it needs.
+  *
+  * The raw bytes are deliberately not part of this case class - they are only
+  * read when actually serving the avatar.
+  */
+case class TeamAvatar(
+    teamId: Long,
+    contentType: String,
+    size: Long,
+    uploadedBy: Option[Long] = None,
+    created: DateTime,
+    modified: DateTime
+)
+
+// An avatar is a team image like any other, so the format and size rules live
+// on TeamImage and callers read them from there - repeating them here would
+// only give one rule a second name to drift from.
+object TeamAvatar {
+  val TABLE = "team_avatars"
+
+  /**
+    * The url that serves a team's avatar, stored in groups.avatar_url so the
+    * rest of the app keeps treating the avatar as a plain url.
+    *
+    * Unlike an approved team image, an avatar is mutable - the same team id
+    * serves different bytes after a re-upload - so the url carries the upload
+    * timestamp. That lets the response be cached hard while still changing the
+    * moment a new avatar is uploaded.
+    */
+  def urlFor(teamId: Long, version: Long): String =
+    s"/api/v2/team/$teamId/avatar/file?v=$version"
+}

app/org/maproulette/framework/model/TeamImage.scala

@@ -107,6 +107,12 @@ object TeamImage extends CommonField {
   // Images are meant to be small card thumbnails, not full photos.
   val MAX_SIZE_BYTES: Int = 2 * 1024 * 1024
 
+  // What the multipart parsers are given, as opposed to the limit the upload
+  // endpoints enforce themselves. The room past MAX_SIZE_BYTES lets an upload
+  // that is merely too big reach the friendly error rather than the parser's,
+  // while a wildly oversized one is refused before it is buffered to disk.
+  val MAX_UPLOAD_BYTES: Long = MAX_SIZE_BYTES.toLong * 2
+
   /**
     * Determines the image format from the file's leading bytes. The
     * browser-supplied content type is only a hint, so the magic number is what