diff --git a/compose.yaml b/compose.yaml index 8cb21feab..adbcce38c 100644 --- a/compose.yaml +++ b/compose.yaml @@ -16,7 +16,7 @@ services: - "3306:3306" faf-db-migrations: - image: faforever/faf-db-migrations:v140 + image: faforever/faf-db-migrations:v143 command: migrate environment: FLYWAY_URL: jdbc:mysql://faf-db/faf?useSSL=false diff --git a/push-branch-image.sh b/push-branch-image.sh new file mode 100755 index 000000000..7c93284ae --- /dev/null +++ b/push-branch-image.sh @@ -0,0 +1,49 @@ +#!/usr/bin/env sh +# Build the current branch into a Docker image and push it as a tag. +# Refuses to run on develop/main/master or in a detached HEAD state. +# +# Builds for linux/amd64 (x86_64) by default so the image runs on the real +# servers even when building from an arm64 machine (Apple Silicon). +# +# The Dockerfile expects a prebuilt jar in build/libs/, so we run the Gradle +# bootJar task first (skip with SKIP_BUILD=1 if you already built it). +# +# Usage: ./push-branch-image.sh +# Env: IMAGE override the destination repo (default: faforever/faf-java-api) +# PLATFORM override the target platform (default: linux/amd64) +# SKIP_BUILD set to 1 to reuse an existing build/libs jar +set -eu + +IMAGE="${IMAGE:-faforever/faf-java-api}" +PLATFORM="${PLATFORM:-linux/amd64}" + +branch=$(git rev-parse --abbrev-ref HEAD) + +case "$branch" in + develop|main|master|HEAD) + echo "Refusing to push '$branch' as a Docker tag." >&2 + exit 1 + ;; +esac + +# Docker tags must match [A-Za-z0-9_][A-Za-z0-9_.-]{0,127}. +# Replace anything else (most commonly '/' from branch prefixes) with '-'. +tag=$(printf '%s' "$branch" | sed 's/[^A-Za-z0-9_.-]/-/g') + +if [ "${SKIP_BUILD:-0}" != "1" ]; then + echo "Building jar with ./gradlew bootJar" + ./gradlew bootJar -x test +fi + +echo "Building $IMAGE:$tag from branch $branch for $PLATFORM" +# buildx with --push builds and pushes in one step. --provenance=false keeps +# the registry tag pointing at a plain image manifest rather than a manifest +# list, which some deployment tooling expects. +docker buildx build \ + --platform "$PLATFORM" \ + --provenance=false \ + -t "$IMAGE:$tag" \ + --push \ + . + +echo "Done: $IMAGE:$tag" diff --git a/src/inttest/java/com/faforever/api/config/MainDbTestContainers.java b/src/inttest/java/com/faforever/api/config/MainDbTestContainers.java index 335e6bbea..9076fd709 100644 --- a/src/inttest/java/com/faforever/api/config/MainDbTestContainers.java +++ b/src/inttest/java/com/faforever/api/config/MainDbTestContainers.java @@ -21,7 +21,7 @@ @Configuration public class MainDbTestContainers { private static final MariaDBContainer fafDBContainer = new MariaDBContainer<>("mariadb:11.7"); - private static final GenericContainer flywayMigrationsContainer = new GenericContainer<>("faforever/faf-db-migrations:v140"); + private static final GenericContainer flywayMigrationsContainer = new GenericContainer<>("faforever/faf-db-migrations:v143"); private static final Network sharedNetwork = Network.newNetwork(); @Bean diff --git a/src/main/java/com/faforever/api/data/domain/AvatarAssignment.java b/src/main/java/com/faforever/api/data/domain/AvatarAssignment.java index 80944b2a3..912e02c33 100644 --- a/src/main/java/com/faforever/api/data/domain/AvatarAssignment.java +++ b/src/main/java/com/faforever/api/data/domain/AvatarAssignment.java @@ -41,6 +41,12 @@ public class AvatarAssignment extends AbstractEntity implement @Relationship(Avatar.TYPE_NAME) private Avatar avatar; + /** + * @deprecated The selected avatar is now tracked via {@link Player#getCurrentAvatar()} + * (the {@code login.avatar_id} column). Update that relationship instead; this flag is + * kept only for backwards compatibility with older clients and will be removed. + */ + @Deprecated(forRemoval = true) @Column(name = "selected") @UpdatePermission(expression = IsEntityOwner.EXPRESSION) @Audit(action = Action.UPDATE, logStatement = "Avatar ''{0}'' has been selected on player ''{1}''", logExpressions = {"${avatarAssignment.avatar.id}", "${avatarAssignment.player.id}"}) diff --git a/src/main/java/com/faforever/api/data/domain/Player.java b/src/main/java/com/faforever/api/data/domain/Player.java index a34612b9a..362dac280 100644 --- a/src/main/java/com/faforever/api/data/domain/Player.java +++ b/src/main/java/com/faforever/api/data/domain/Player.java @@ -2,16 +2,24 @@ import com.faforever.api.data.checks.IsEntityOwner; import com.faforever.api.data.checks.Prefab; +import com.faforever.api.data.hook.PlayerAvatarUpdateHook; +import com.faforever.api.data.hook.PlayerAvatarValidationHook; import com.faforever.api.security.elide.permission.AdminModerationReportCheck; import com.github.jasminb.jsonapi.annotations.Type; +import com.yahoo.elide.annotation.Audit; +import com.yahoo.elide.annotation.Audit.Action; import com.yahoo.elide.annotation.Include; +import com.yahoo.elide.annotation.LifeCycleHookBinding; import com.yahoo.elide.annotation.ReadPermission; import com.yahoo.elide.annotation.UpdatePermission; import lombok.Setter; import org.hibernate.annotations.BatchSize; import jakarta.persistence.Entity; +import jakarta.persistence.FetchType; +import jakarta.persistence.JoinColumn; import jakarta.persistence.ManyToMany; +import jakarta.persistence.ManyToOne; import jakarta.persistence.OneToMany; import jakarta.persistence.OneToOne; import jakarta.persistence.Table; @@ -29,9 +37,28 @@ public class Player extends Login { private ClanMembership clanMembership; private Set names; private Set avatarAssignments; + private Avatar currentAvatar; private Set reporterOnModerationReports; private Set reportedOnModerationReports; + @UpdatePermission(expression = IsEntityOwner.EXPRESSION) + @LifeCycleHookBinding( + operation = LifeCycleHookBinding.Operation.UPDATE, + phase = LifeCycleHookBinding.TransactionPhase.PRECOMMIT, + hook = PlayerAvatarValidationHook.class + ) + @LifeCycleHookBinding( + operation = LifeCycleHookBinding.Operation.UPDATE, + phase = LifeCycleHookBinding.TransactionPhase.POSTCOMMIT, + hook = PlayerAvatarUpdateHook.class + ) + @Audit(action = Action.UPDATE, logStatement = "Avatar ''{0}'' has been selected on player ''{1}''", logExpressions = {"${player.currentAvatar.id}", "${player.id}"}) + @ManyToOne(fetch = FetchType.LAZY) + @JoinColumn(name = "avatar_id") + public Avatar getCurrentAvatar() { + return currentAvatar; + } + // Permission is managed by ClanMembership class @UpdatePermission(expression = Prefab.ALL) @OneToOne(mappedBy = "player") diff --git a/src/main/java/com/faforever/api/data/hook/PlayerAvatarUpdateHook.java b/src/main/java/com/faforever/api/data/hook/PlayerAvatarUpdateHook.java new file mode 100644 index 000000000..1c62999cd --- /dev/null +++ b/src/main/java/com/faforever/api/data/hook/PlayerAvatarUpdateHook.java @@ -0,0 +1,45 @@ +package com.faforever.api.data.hook; + +import com.faforever.api.config.RabbitConfiguration; +import com.faforever.api.data.domain.Avatar; +import com.faforever.api.data.domain.Player; +import com.yahoo.elide.annotation.LifeCycleHookBinding.Operation; +import com.yahoo.elide.annotation.LifeCycleHookBinding.TransactionPhase; +import com.yahoo.elide.core.lifecycle.LifeCycleHook; +import com.yahoo.elide.core.security.ChangeSpec; +import com.yahoo.elide.core.security.RequestScope; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.springframework.amqp.rabbit.core.RabbitTemplate; +import org.springframework.stereotype.Component; + +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; + +@Slf4j +@Component +@RequiredArgsConstructor +public class PlayerAvatarUpdateHook implements LifeCycleHook { + + public static final String ROUTING_KEY_PLAYER_AVATAR_UPDATE = "success.player_avatar.update"; + + private final RabbitTemplate rabbitTemplate; + + @Override + public void execute(Operation operation, TransactionPhase phase, Player player, RequestScope requestScope, Optional changes) { + final Avatar avatar = player.getCurrentAvatar(); + final Integer avatarId = avatar == null ? null : avatar.getId(); + + Map payload = new HashMap<>(); + payload.put("player_id", player.getId()); + payload.put("avatar_id", avatarId); + + log.debug("Publishing player_avatar update: {}", payload); + rabbitTemplate.convertAndSend( + RabbitConfiguration.EXCHANGE_FAF_LOBBY, + ROUTING_KEY_PLAYER_AVATAR_UPDATE, + payload + ); + } +} diff --git a/src/main/java/com/faforever/api/data/hook/PlayerAvatarValidationHook.java b/src/main/java/com/faforever/api/data/hook/PlayerAvatarValidationHook.java new file mode 100644 index 000000000..20bc4ab92 --- /dev/null +++ b/src/main/java/com/faforever/api/data/hook/PlayerAvatarValidationHook.java @@ -0,0 +1,40 @@ +package com.faforever.api.data.hook; + +import com.faforever.api.avatar.AvatarAssignmentRepository; +import com.faforever.api.data.domain.Avatar; +import com.faforever.api.data.domain.Player; +import com.faforever.api.error.ApiException; +import com.faforever.api.error.Error; +import com.faforever.api.error.ErrorCode; +import com.yahoo.elide.annotation.LifeCycleHookBinding.Operation; +import com.yahoo.elide.annotation.LifeCycleHookBinding.TransactionPhase; +import com.yahoo.elide.core.lifecycle.LifeCycleHook; +import com.yahoo.elide.core.security.ChangeSpec; +import com.yahoo.elide.core.security.RequestScope; +import lombok.RequiredArgsConstructor; +import org.springframework.stereotype.Component; + +import java.util.Optional; + +/** + * Rejects assigning a {@code currentAvatar} that is not granted to the player. The + * {@code IsEntityOwner} update permission only guarantees the caller edits their own Player row; + * it does not verify the avatar belongs to them. Clearing the avatar (null) is always allowed. + */ +@Component +@RequiredArgsConstructor +public class PlayerAvatarValidationHook implements LifeCycleHook { + + private final AvatarAssignmentRepository avatarAssignmentRepository; + + @Override + public void execute(Operation operation, TransactionPhase phase, Player player, RequestScope requestScope, Optional changes) { + final Avatar avatar = player.getCurrentAvatar(); + if (avatar == null) { + return; + } + + avatarAssignmentRepository.findOneByAvatarIdAndPlayerId(avatar.getId(), player.getId()) + .orElseThrow(() -> new ApiException(new Error(ErrorCode.AVATAR_NOT_ASSIGNED, avatar.getId(), player.getId()))); + } +} diff --git a/src/main/java/com/faforever/api/error/ErrorCode.java b/src/main/java/com/faforever/api/error/ErrorCode.java index 30c6331ee..34e1c8c32 100644 --- a/src/main/java/com/faforever/api/error/ErrorCode.java +++ b/src/main/java/com/faforever/api/error/ErrorCode.java @@ -120,6 +120,7 @@ public enum ErrorCode { STEAM_LOGIN_VALIDATION_FAILED(210, "Login via Steam failed", "Invalid OpenID redirect code"), MAP_VERSION_INVALID_RANGE(211, "Invalid map version", "The map version must be a whole number in range {0, number} to {1, number}."), MOD_VERSION_INVALID_RANGE(212, "Invalid mod version", "The mod version must be a whole number in range {0, number} to {1, number}."), + AVATAR_NOT_ASSIGNED(213, "Avatar not assigned", "Avatar ''{0}'' is not assigned to player ''{1}''."), ; diff --git a/src/test/java/com/faforever/api/data/hook/PlayerAvatarValidationHookTest.java b/src/test/java/com/faforever/api/data/hook/PlayerAvatarValidationHookTest.java new file mode 100644 index 000000000..4cf5cebd9 --- /dev/null +++ b/src/test/java/com/faforever/api/data/hook/PlayerAvatarValidationHookTest.java @@ -0,0 +1,77 @@ +package com.faforever.api.data.hook; + +import com.faforever.api.avatar.AvatarAssignmentRepository; +import com.faforever.api.data.domain.Avatar; +import com.faforever.api.data.domain.AvatarAssignment; +import com.faforever.api.data.domain.Player; +import com.faforever.api.error.ApiException; +import com.faforever.api.error.ErrorCode; +import com.yahoo.elide.annotation.LifeCycleHookBinding.Operation; +import com.yahoo.elide.annotation.LifeCycleHookBinding.TransactionPhase; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.util.Optional; + +import static com.faforever.api.error.ApiExceptionMatcher.hasErrorCode; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class PlayerAvatarValidationHookTest { + + private static final int PLAYER_ID = 7; + private static final int AVATAR_ID = 42; + + @Mock + private AvatarAssignmentRepository avatarAssignmentRepository; + + private PlayerAvatarValidationHook instance; + + @BeforeEach + void setUp() { + instance = new PlayerAvatarValidationHook(avatarAssignmentRepository); + } + + private static Player playerWithAvatar(Integer avatarId) { + Avatar avatar = new Avatar(); + avatar.setId(avatarId); + Player player = new Player(); + player.setId(PLAYER_ID); + player.setCurrentAvatar(avatar); + return player; + } + + @Test + void allowsAssignedAvatar() { + when(avatarAssignmentRepository.findOneByAvatarIdAndPlayerId(AVATAR_ID, PLAYER_ID)) + .thenReturn(Optional.of(new AvatarAssignment())); + + instance.execute(Operation.UPDATE, TransactionPhase.PRECOMMIT, playerWithAvatar(AVATAR_ID), null, Optional.empty()); + } + + @Test + void rejectsUnassignedAvatar() { + when(avatarAssignmentRepository.findOneByAvatarIdAndPlayerId(AVATAR_ID, PLAYER_ID)) + .thenReturn(Optional.empty()); + + ApiException result = assertThrows(ApiException.class, () -> + instance.execute(Operation.UPDATE, TransactionPhase.PRECOMMIT, playerWithAvatar(AVATAR_ID), null, Optional.empty())); + assertThat(result, hasErrorCode(ErrorCode.AVATAR_NOT_ASSIGNED)); + } + + @Test + void allowsClearingAvatar() { + Player player = new Player(); + player.setId(PLAYER_ID); + + instance.execute(Operation.UPDATE, TransactionPhase.PRECOMMIT, player, null, Optional.empty()); + + verifyNoInteractions(avatarAssignmentRepository); + } +}