Skip to content

Commit fe993ec

Browse files
CollinBeczakCollinBeczak
committed
Host team avatars instead of only linking them
A team's avatar could only be a url pointing somewhere else on the internet. POST and DELETE /team/:teamId/avatar now store and clear an avatar we host (evolution 125), pointing the team's avatar_url at bytes of our own so the rest of the app keeps treating an avatar as a plain url. GET /team/:teamId/avatar/file serves them anonymously with an ETag, since that url feeds plain img tags. Unlike team images these are not moderated: a team admin could already point avatar_url at any image on the internet, so requiring review for the uploaded case would gate the safer of the two paths. The size and content-type limits are shared with team images. Stored keyed by team, so a team has at most one avatar and uploading a new one replaces the bytes rather than accumulating them. The bytes and the avatar_url update commit together -- updateTeam and updateGroup now accept the caller's connection -- so a failure cannot leave the team pointing at an avatar that was never stored.
1 parent d37a1ea commit fe993ec

11 files changed

Lines changed: 558 additions & 4 deletions

File tree

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

Lines changed: 139 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,23 @@
55

66
package org.maproulette.framework.controller
77

8+
import java.sql.Connection
89
import javax.inject.Inject
910
import org.maproulette.data.ActionManager
10-
import org.maproulette.exception.{MPExceptionUtil, StatusMessage}
11+
import org.maproulette.exception.{
12+
InvalidException,
13+
MPExceptionUtil,
14+
NotFoundException,
15+
StatusMessage
16+
}
1117
import org.maproulette.framework.service.TeamService
12-
import org.maproulette.framework.model.{User, MemberObject, Group}
18+
import org.maproulette.framework.model.{User, MemberObject, Group, TeamAvatar}
19+
import org.maproulette.framework.repository.TeamAvatarRepository
1320
import org.maproulette.framework.psql.{Paging}
21+
import org.maproulette.permissions.Permission
1422
import org.maproulette.session.SessionManager
23+
import play.api.db.Database
24+
import play.api.libs.Files
1525
import play.api.libs.json._
1626
import play.api.mvc._
1727

@@ -23,6 +33,9 @@ class TeamController @Inject() (
2333
override val actionManager: ActionManager,
2434
override val bodyParsers: PlayBodyParsers,
2535
teamService: TeamService,
36+
teamAvatarRepository: TeamAvatarRepository,
37+
permission: Permission,
38+
db: Database,
2639
components: ControllerComponents
2740
) extends AbstractController(components)
2841
with MapRouletteController {
@@ -310,4 +323,128 @@ class TeamController @Inject() (
310323
}
311324
}
312325
}
326+
327+
/**
328+
* Fetches a team, or fails with a 404.
329+
*/
330+
private def team(teamId: Long, user: User): Group =
331+
this.teamService
332+
.retrieve(teamId, user)
333+
.getOrElse(throw new NotFoundException(s"No team found with id $teamId"))
334+
335+
/**
336+
* Uploads a team's avatar, replacing whatever avatar it had. The bytes are
337+
* stored by us and the team's avatar url is pointed at them, so the rest of
338+
* the app keeps treating the avatar as a plain url.
339+
*
340+
* Unlike a team's challenge images this needs no review: a team admin could
341+
* already point the avatar url at any image on the internet, so gating only
342+
* the uploaded case would be stricter about the safer of the two paths.
343+
*
344+
* @param teamId The id of the team whose avatar is being set
345+
* @return 200 OK with the updated team
346+
*/
347+
def uploadAvatar(teamId: Long): Action[MultipartFormData[Files.TemporaryFile]] =
348+
Action.async(parse.multipartFormData) { implicit request =>
349+
this.sessionManager.authenticatedRequest { implicit user =>
350+
val existing = this.team(teamId, user)
351+
// Checked before anything is stored, so a non-admin can't write bytes
352+
// and only be turned away afterwards
353+
this.permission.hasObjectAdminAccess(existing, user)
354+
355+
request.body.file("image") match {
356+
case Some(upload) =>
357+
if (upload.fileSize > TeamAvatar.MAX_SIZE_BYTES) {
358+
throw new InvalidException(
359+
s"Image is larger than the ${TeamAvatar.MAX_SIZE_BYTES / (1024 * 1024)}MB limit"
360+
)
361+
}
362+
363+
val data = java.nio.file.Files.readAllBytes(upload.ref.path)
364+
// The declared content type is caller-supplied, so the leading
365+
// bytes are what we actually trust before storing something we
366+
// will later serve back from our own origin.
367+
val contentType = TeamAvatar.detectContentType(data) match {
368+
case Some(detected) => detected
369+
case None =>
370+
throw new InvalidException(
371+
s"Unsupported image format. Supported formats: ${TeamAvatar.ALLOWED_CONTENT_TYPES.toList.sorted
372+
.mkString(", ")}"
373+
)
374+
}
375+
376+
// Storing the bytes and pointing the team's avatar url at them
377+
// are two writes describing one fact, so they commit together. Left
378+
// apart, a failure between them strands the bytes with the url
379+
// still on the team's previous avatar, and the url carries a
380+
// `?v=<modified>` stamp that would then be stale in browser caches
381+
// until the next upload.
382+
val updated = this.db.withTransaction { connection =>
383+
implicit val c: Option[Connection] = Some(connection)
384+
val modified =
385+
this.teamAvatarRepository.upsert(teamId, contentType, data, user.id)
386+
this.teamService.updateTeam(
387+
existing.copy(avatarURL = Some(TeamAvatar.urlFor(teamId, modified.getMillis))),
388+
user
389+
)
390+
}
391+
392+
Ok(Json.toJson(updated.get))
393+
case None =>
394+
throw new InvalidException("No image file provided in the 'image' field")
395+
}
396+
}
397+
}
398+
399+
/**
400+
* Removes a team's uploaded avatar. An avatar url the team pasted in
401+
* themselves is left alone - there are no bytes of ours behind it, and
402+
* clearing it would be deleting something this endpoint never set.
403+
*
404+
* @param teamId The id of the team whose avatar is being removed
405+
* @return 200 OK with the updated team
406+
*/
407+
def deleteAvatar(teamId: Long): Action[AnyContent] = Action.async { implicit request =>
408+
this.sessionManager.authenticatedRequest { implicit user =>
409+
val existing = this.team(teamId, user)
410+
this.permission.hasObjectAdminAccess(existing, user)
411+
412+
this.teamAvatarRepository.delete(teamId)
413+
val remainingURL = existing.avatarURL.filterNot(TeamAvatar.isStoredAvatarUrl(_, teamId))
414+
415+
Ok(
416+
Json.toJson(this.teamService.updateTeam(existing.copy(avatarURL = remainingURL), user).get)
417+
)
418+
}
419+
}
420+
421+
/**
422+
* Serves a team's avatar bytes. Anonymous, because the url is consumed by
423+
* plain img tags wherever the team is shown.
424+
*
425+
* @param teamId The id of the team whose avatar to serve
426+
* @return 200 OK with the avatar bytes
427+
*/
428+
def getAvatarFile(teamId: Long): Action[AnyContent] = Action.async { implicit request =>
429+
this.sessionManager.userAwareRequest { implicit user =>
430+
this.teamAvatarRepository.retrieveData(teamId) match {
431+
case Some(avatar) =>
432+
val etag = "\"" + s"$teamId-${avatar.modified.getMillis}" + "\""
433+
if (request.headers.get("If-None-Match").contains(etag)) {
434+
NotModified.withHeaders("ETag" -> etag)
435+
} else {
436+
Ok(avatar.data)
437+
.as(avatar.contentType)
438+
.withHeaders(
439+
"ETag" -> etag,
440+
"Cache-Control" -> "public, max-age=86400",
441+
"X-Content-Type-Options" -> "nosniff",
442+
"Content-Disposition" -> "inline"
443+
)
444+
}
445+
case None =>
446+
throw new NotFoundException(s"No avatar found for team $teamId")
447+
}
448+
}
449+
}
313450
}

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/*
2+
* Copyright (C) 2020 MapRoulette contributors (see CONTRIBUTORS.md).
3+
* Licensed under the Apache License, Version 2.0 (see LICENSE).
4+
*/
5+
package org.maproulette.framework.model
6+
7+
import org.joda.time.DateTime
8+
9+
/**
10+
* A team's uploaded avatar. There is at most one per team, so the team id is
11+
* the only identity it needs.
12+
*
13+
* The raw bytes are deliberately not part of this case class - they are only
14+
* read when actually serving the avatar.
15+
*/
16+
case class TeamAvatar(
17+
teamId: Long,
18+
contentType: String,
19+
size: Long,
20+
uploadedBy: Option[Long] = None,
21+
created: DateTime,
22+
modified: DateTime
23+
)
24+
25+
object TeamAvatar {
26+
val TABLE = "team_avatars"
27+
28+
// An avatar is a team image like any other, so it is held to the same format
29+
// and size rules rather than a parallel set that could drift from them.
30+
val ALLOWED_CONTENT_TYPES: Set[String] = TeamImage.ALLOWED_CONTENT_TYPES
31+
val MAX_SIZE_BYTES: Int = TeamImage.MAX_SIZE_BYTES
32+
33+
def detectContentType(data: Array[Byte]): Option[String] = TeamImage.detectContentType(data)
34+
35+
/**
36+
* The url that serves a team's avatar, stored in groups.avatar_url so the
37+
* rest of the app keeps treating the avatar as a plain url.
38+
*
39+
* Unlike an approved team image, an avatar is mutable - the same team id
40+
* serves different bytes after a re-upload - so the url carries the upload
41+
* timestamp. That lets the response be cached hard while still changing the
42+
* moment a new avatar is uploaded.
43+
*/
44+
def urlFor(teamId: Long, version: Long): String = s"/api/v2/team/$teamId/avatar/file?v=$version"
45+
46+
/**
47+
* Whether a stored avatar url is one of ours for the given team, as opposed
48+
* to an external url the team pasted in.
49+
*/
50+
def isStoredAvatarUrl(url: String, teamId: Long): Boolean =
51+
url.startsWith(s"/api/v2/team/$teamId/avatar/file")
52+
}

‎app/org/maproulette/framework/repository/TeamAvatarRepository.scala‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
/*
2+
* Copyright (C) 2020 MapRoulette contributors (see CONTRIBUTORS.md).
3+
* Licensed under the Apache License, Version 2.0 (see LICENSE).
4+
*/
5+
package org.maproulette.framework.repository
6+
7+
import anorm.SqlParser._
8+
import anorm._
9+
import java.sql.Connection
10+
import javax.inject.{Inject, Singleton}
11+
import org.joda.time.DateTime
12+
import org.maproulette.framework.model.{TeamAvatar, TeamImageData}
13+
import play.api.db.Database
14+
15+
/**
16+
* Repository for team avatars. A team has at most one, so every operation is
17+
* keyed by team id rather than by a row id.
18+
*/
19+
@Singleton
20+
class TeamAvatarRepository @Inject() (override val db: Database) extends RepositoryMixin {
21+
implicit val baseTable: String = TeamAvatar.TABLE
22+
23+
// The bytes are excluded on purpose; only `retrieveData` pulls them.
24+
private val parser: RowParser[TeamAvatar] = {
25+
get[Long]("team_id") ~
26+
get[String]("content_type") ~
27+
get[Long]("size") ~
28+
get[Option[Long]]("uploaded_by") ~
29+
get[DateTime]("created") ~
30+
get[DateTime]("modified") map {
31+
case teamId ~ contentType ~ size ~ uploadedBy ~ created ~ modified =>
32+
TeamAvatar(teamId, contentType, size, uploadedBy, created, modified)
33+
}
34+
}
35+
36+
/**
37+
* Retrieves a team's avatar metadata.
38+
*/
39+
def retrieve(teamId: Long): Option[TeamAvatar] = {
40+
this.withMRConnection { implicit c =>
41+
SQL"""SELECT team_id, content_type, octet_length(data) AS size, uploaded_by, created, modified
42+
FROM team_avatars WHERE team_id = $teamId"""
43+
.as(this.parser.singleOpt)
44+
}
45+
}
46+
47+
/**
48+
* Retrieves the bytes of a team's avatar, for serving it.
49+
*/
50+
def retrieveData(teamId: Long): Option[TeamImageData] = {
51+
this.withMRConnection { implicit c =>
52+
SQL"SELECT content_type, data, modified FROM team_avatars WHERE team_id = $teamId"
53+
.as(
54+
(get[String]("content_type") ~ get[Array[Byte]]("data") ~ get[DateTime]("modified") map {
55+
case contentType ~ data ~ modified => TeamImageData(contentType, data, modified)
56+
}).singleOpt
57+
)
58+
}
59+
}
60+
61+
/**
62+
* Stores a team's avatar, replacing any avatar it already had. Returns the
63+
* upload time, which callers use to version the avatar's url so a re-upload
64+
* is not masked by a cached response.
65+
*
66+
* Accepts a caller-supplied connection so storing the bytes and pointing the
67+
* team's avatar url at them can commit as one unit.
68+
*
69+
* @return The modified timestamp of the stored avatar
70+
*/
71+
def upsert(
72+
teamId: Long,
73+
contentType: String,
74+
data: Array[Byte],
75+
uploadedBy: Long
76+
)(implicit c: Option[Connection] = None): DateTime = {
77+
this.withMRTransaction { implicit c =>
78+
SQL"""INSERT INTO team_avatars (team_id, content_type, data, uploaded_by)
79+
VALUES ($teamId, $contentType, $data, $uploadedBy)
80+
ON CONFLICT (team_id) DO UPDATE
81+
SET content_type = EXCLUDED.content_type, data = EXCLUDED.data,
82+
uploaded_by = EXCLUDED.uploaded_by, modified = NOW()
83+
RETURNING modified"""
84+
.as(get[DateTime]("modified").single)
85+
}
86+
}
87+
88+
/**
89+
* Deletes a team's stored avatar.
90+
*
91+
* @return true if the team had a stored avatar to remove
92+
*/
93+
def delete(teamId: Long): Boolean = {
94+
this.withMRTransaction { implicit c =>
95+
SQL"DELETE FROM team_avatars WHERE team_id = $teamId".executeUpdate() > 0
96+
}
97+
}
98+
}

‎app/org/maproulette/framework/service/GroupService.scala‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
package org.maproulette.framework.service
77

8+
import java.sql.Connection
89
import javax.inject.{Inject, Singleton}
910
import org.maproulette.framework.model.{Group, GroupMember, MemberObject}
1011
import org.maproulette.data.{ItemType}
@@ -244,7 +245,8 @@ class GroupService @Inject() (
244245
*
245246
* @param group The latest group data
246247
*/
247-
def updateGroup(group: Group): Option[Group] = this.repository.update(group)
248+
def updateGroup(group: Group)(implicit c: Option[Connection] = None): Option[Group] =
249+
this.repository.update(group)
248250

249251
/**
250252
* Delete a group from the database

‎app/org/maproulette/framework/service/TeamService.scala‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
package org.maproulette.framework.service
77

8+
import java.sql.Connection
89
import javax.inject.{Inject, Singleton}
910
import org.maproulette.exception.{InvalidException, NotFoundException}
1011
import org.maproulette.framework.model._
@@ -675,7 +676,9 @@ class TeamService @Inject() (
675676
* @param team The latest team data
676677
* @param user The user updating the team
677678
*/
678-
def updateTeam(team: Group, user: User): Option[Group] = {
679+
def updateTeam(team: Group, user: User)(
680+
implicit c: Option[Connection] = None
681+
): Option[Group] = {
679682
// Only a team admin can update a team
680683
this.ensureTeam(team)
681684
this.permission.hasObjectAdminAccess(team, user)

‎conf/evolutions/default/124.sql‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
# --- MapRoulette Scheme
2+
3+
# --- !Ups
4+
5+
-- A team's own avatar, uploaded rather than linked. Unlike team_images these
6+
-- are not moderated: a team admin could already point avatar_url at any image
7+
-- on the internet, so requiring review only for the uploaded case would gate
8+
-- the safer of the two paths.
9+
--
10+
-- Keyed by team, so a team has at most one stored avatar and uploading a new
11+
-- one replaces the old bytes rather than accumulating them.
12+
CREATE TABLE IF NOT EXISTS team_avatars
13+
(
14+
team_id integer NOT NULL PRIMARY KEY,
15+
content_type character varying NOT NULL,
16+
data bytea NOT NULL,
17+
uploaded_by integer,
18+
created timestamp without time zone DEFAULT NOW(),
19+
modified timestamp without time zone DEFAULT NOW(),
20+
CONSTRAINT team_avatars_team_id_fkey FOREIGN KEY (team_id)
21+
REFERENCES groups (id) MATCH SIMPLE
22+
ON UPDATE CASCADE ON DELETE CASCADE,
23+
CONSTRAINT team_avatars_uploaded_by_fkey FOREIGN KEY (uploaded_by)
24+
REFERENCES users (id) MATCH SIMPLE
25+
ON UPDATE CASCADE ON DELETE SET NULL
26+
);;
27+
28+
# --- !Downs
29+
30+
DROP TABLE IF EXISTS team_avatars;;

0 commit comments

Comments
 (0)