Poprawki bezpieczenstwa: XSS przez zdjecia, darmowe promowanie, ITN
Deploy Polska Lokalnie / deploy (push) Successful in 12s
Deploy Polska Lokalnie / deploy (push) Successful in 12s
- Zdjecia ogloszen tylko jako data URL JPEG/PNG/WebP; podglad OG serwuje wylacznie te typy z nosniff i CSP sandbox, bez przekierowania na dowolny URL. - Zakup promowania bez skonfigurowanej bramki AutoPay zwraca 503; automatyczne potwierdzanie tylko przy APP_PAYMENTS_AUTO_CONFIRM (lokalnie). - ITN uznaje platnosc tylko przy jawnym statusie SUCCESS i zgodnej kwocie. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -27,3 +27,8 @@ APP_JWT_SECRET=local-dev-secret-change-me-please-32chars-min
|
||||
|
||||
# Adres bazowy w linkach z e-maili (aktywacja konta, reset hasla).
|
||||
APP_PUBLIC_BASE_URL=http://localhost
|
||||
|
||||
# --- Platnosci ---
|
||||
# true = bez skonfigurowanej bramki AutoPay zakup promowania jest potwierdzany automatycznie.
|
||||
# Wylacznie lokalnie (docker-compose.yml). Produkcja tej zmiennej nie czyta.
|
||||
APP_PAYMENTS_AUTO_CONFIRM=true
|
||||
|
||||
@@ -327,11 +327,20 @@ public class ListingService {
|
||||
return cleaned;
|
||||
}
|
||||
|
||||
private static List<String> limitPhotos(List<String> photos) {
|
||||
// Zdjecia przyjmujemy wylacznie jako data URL obrazu rastrowego. Inny typ (np. text/html,
|
||||
// image/svg+xml) serwowany potem z domeny serwisu pozwalalby uruchomic skrypt u oglądającego.
|
||||
private static final java.util.regex.Pattern PHOTO_DATA_URL =
|
||||
java.util.regex.Pattern.compile("^data:image/(jpeg|png|webp);base64,[A-Za-z0-9+/=\\s]+$");
|
||||
|
||||
static List<String> limitPhotos(List<String> photos) {
|
||||
List<String> result = new ArrayList<>();
|
||||
if (photos != null) {
|
||||
for (String photo : photos) {
|
||||
if (photo != null && !photo.isBlank()) {
|
||||
if (!PHOTO_DATA_URL.matcher(photo).matches()) {
|
||||
throw new ResponseStatusException(HttpStatus.BAD_REQUEST,
|
||||
"Nieprawidłowy format zdjęcia - dozwolone są obrazy JPEG, PNG i WebP");
|
||||
}
|
||||
result.add(photo);
|
||||
if (result.size() >= MAX_PHOTOS) {
|
||||
break;
|
||||
|
||||
@@ -8,7 +8,6 @@ import java.util.List;
|
||||
import java.util.Locale;
|
||||
import org.springframework.beans.factory.annotation.Value;
|
||||
import org.springframework.http.CacheControl;
|
||||
import org.springframework.http.HttpStatus;
|
||||
import org.springframework.http.MediaType;
|
||||
import org.springframework.http.ResponseEntity;
|
||||
import org.springframework.transaction.annotation.Transactional;
|
||||
@@ -92,17 +91,18 @@ public class OgController {
|
||||
if (listing == null || listing.getStatus() != ListingStatus.APPROVED || !hasImage(listing)) {
|
||||
return ResponseEntity.notFound().build();
|
||||
}
|
||||
String cover = listing.getPhotos().get(0);
|
||||
DecodedImage image = decodeDataUrl(cover);
|
||||
DecodedImage image = decodeDataUrl(listing.getPhotos().get(0));
|
||||
if (image == null) {
|
||||
// Zdjecie zapisane jako zwykly URL (nie data URL) - odsylamy crawlera pod ten adres.
|
||||
return ResponseEntity.status(HttpStatus.FOUND)
|
||||
.header("Location", cover)
|
||||
.build();
|
||||
// Bez przekierowania pod zapisany adres - to byloby otwarte przekierowanie na dowolna strone.
|
||||
return ResponseEntity.notFound().build();
|
||||
}
|
||||
// nosniff + sandbox: nawet gdyby w bazie siedzial spreparowany plik, przegladarka nie wykona
|
||||
// go jako strony w domenie serwisu.
|
||||
return ResponseEntity.ok()
|
||||
.contentType(MediaType.valueOf(image.mimeType()))
|
||||
.cacheControl(CacheControl.maxAge(java.time.Duration.ofHours(1)).cachePublic())
|
||||
.header("X-Content-Type-Options", "nosniff")
|
||||
.header("Content-Security-Policy", "default-src 'none'; sandbox")
|
||||
.body(image.data());
|
||||
}
|
||||
|
||||
@@ -111,9 +111,14 @@ public class OgController {
|
||||
return photos != null && !photos.isEmpty() && photos.get(0) != null && !photos.get(0).isBlank();
|
||||
}
|
||||
|
||||
private record DecodedImage(String mimeType, byte[] data) {}
|
||||
record DecodedImage(String mimeType, byte[] data) {}
|
||||
|
||||
private static DecodedImage decodeDataUrl(String value) {
|
||||
// Tylko obrazy rastrowe - typ MIME pochodzi od uzytkownika i nie moze decydowac o tym,
|
||||
// jako co przegladarka potraktuje odpowiedz (text/html, image/svg+xml = skrypt w domenie serwisu).
|
||||
private static final java.util.Set<String> ALLOWED_IMAGE_TYPES = java.util.Set.of(
|
||||
MediaType.IMAGE_JPEG_VALUE, MediaType.IMAGE_PNG_VALUE, "image/webp");
|
||||
|
||||
static DecodedImage decodeDataUrl(String value) {
|
||||
if (value == null || !value.startsWith("data:")) {
|
||||
return null;
|
||||
}
|
||||
@@ -129,6 +134,10 @@ public class OgController {
|
||||
if (mime.isBlank()) {
|
||||
mime = MediaType.IMAGE_JPEG_VALUE;
|
||||
}
|
||||
mime = mime.trim().toLowerCase(Locale.ROOT);
|
||||
if (!ALLOWED_IMAGE_TYPES.contains(mime)) {
|
||||
return null;
|
||||
}
|
||||
try {
|
||||
byte[] data = Base64.getDecoder().decode(value.substring(comma + 1));
|
||||
return new DecodedImage(mime, data);
|
||||
|
||||
@@ -42,15 +42,16 @@ public class AutoPayItnController {
|
||||
}
|
||||
String orderId = params.get("OrderID");
|
||||
if (orderId != null && isSuccess(params)) {
|
||||
promotionService.markPaidByExternalId(orderId);
|
||||
promotionService.markPaidByExternalId(orderId, params.get("Amount"));
|
||||
}
|
||||
return ResponseEntity.ok("OK");
|
||||
}
|
||||
|
||||
private static boolean isSuccess(Map<String, String> params) {
|
||||
// Oplacone jest tylko zamowienie z jawnym, koncowym statusem SUCCESS. PENDING to platnosc
|
||||
// rozpoczeta, ktora moze jeszcze sie nie udac, a brak statusu nie moze oznaczac sukcesu.
|
||||
static boolean isSuccess(Map<String, String> params) {
|
||||
String status = params.getOrDefault("PaymentStatus",
|
||||
params.getOrDefault("Status", params.getOrDefault("paymentStatus", "SUCCESS")));
|
||||
String s = status == null ? "" : status.trim().toUpperCase();
|
||||
return s.equals("SUCCESS") || s.equals("CONFIRMED") || s.equals("TRUE") || s.equals("PENDING");
|
||||
params.getOrDefault("Status", params.get("paymentStatus")));
|
||||
return status != null && status.trim().equalsIgnoreCase("SUCCESS");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -28,6 +28,7 @@ public class PromotionService {
|
||||
private final PaymentConfigService paymentConfigService;
|
||||
private final AutoPayService autoPayService;
|
||||
private final String baseUrl;
|
||||
private final boolean autoConfirmPayments;
|
||||
|
||||
public PromotionService(PromotionPlanRepository planRepository,
|
||||
PromotionPackageRepository packageRepository,
|
||||
@@ -36,7 +37,8 @@ public class PromotionService {
|
||||
ListingRepository listingRepository,
|
||||
PaymentConfigService paymentConfigService,
|
||||
AutoPayService autoPayService,
|
||||
@Value("${app.public-base-url:http://localhost}") String baseUrl) {
|
||||
@Value("${app.public-base-url:http://localhost}") String baseUrl,
|
||||
@Value("${app.payments.auto-confirm:false}") boolean autoConfirmPayments) {
|
||||
this.planRepository = planRepository;
|
||||
this.packageRepository = packageRepository;
|
||||
this.orderRepository = orderRepository;
|
||||
@@ -45,6 +47,7 @@ public class PromotionService {
|
||||
this.paymentConfigService = paymentConfigService;
|
||||
this.autoPayService = autoPayService;
|
||||
this.baseUrl = baseUrl == null ? "" : baseUrl.replaceAll("/+$", "");
|
||||
this.autoConfirmPayments = autoConfirmPayments;
|
||||
}
|
||||
|
||||
// --- Publiczne (uzytkownik) ---
|
||||
@@ -63,6 +66,14 @@ public class PromotionService {
|
||||
|
||||
@Transactional
|
||||
public CheckoutResponse checkout(String email, CheckoutRequest request) {
|
||||
PaymentConfig cfg = paymentConfigService.get();
|
||||
// Bez skonfigurowanej bramki nie ma komu zaplacic - zamowienie uznane "od reki" oznaczaloby
|
||||
// darmowe promowanie dla kazdego. Symulacja platnosci tylko przy jawnie wlaczonym APP_PAYMENTS_AUTO_CONFIRM (lokalnie).
|
||||
if (!cfg.isLive() && !autoConfirmPayments) {
|
||||
throw new ResponseStatusException(HttpStatus.SERVICE_UNAVAILABLE,
|
||||
"Płatności są chwilowo niedostępne. Spróbuj ponownie później.");
|
||||
}
|
||||
|
||||
PromotionOrder order = new PromotionOrder();
|
||||
order.setUserEmail(email);
|
||||
order.setKind(request.kind());
|
||||
@@ -86,9 +97,8 @@ public class PromotionService {
|
||||
}
|
||||
order = orderRepository.save(order);
|
||||
|
||||
PaymentConfig cfg = paymentConfigService.get();
|
||||
if (!cfg.isLive()) {
|
||||
// Tryb sandbox - platnosc uznajemy od razu (demo dziala bez kluczy AutoPay).
|
||||
// Lokalny tryb bez bramki (app.payments.auto-confirm) - platnosc uznajemy od razu.
|
||||
applyPaid(order);
|
||||
return CheckoutResponse.paid(order.getId());
|
||||
}
|
||||
@@ -123,10 +133,24 @@ public class PromotionService {
|
||||
orderRepository.save(order);
|
||||
}
|
||||
|
||||
// Wywolywane z callbacku ITN AutoPay po zweryfikowaniu podpisu.
|
||||
// Wywolywane z callbacku ITN AutoPay po zweryfikowaniu podpisu. Kwota z powiadomienia musi
|
||||
// zgadzac sie z zamowieniem - inaczej wplata nie pokrywa tego, co mamy wydac.
|
||||
@Transactional
|
||||
public void markPaidByExternalId(String externalId) {
|
||||
orderRepository.findByExternalId(externalId).ifPresent(this::applyPaid);
|
||||
public void markPaidByExternalId(String externalId, String paidAmount) {
|
||||
orderRepository.findByExternalId(externalId)
|
||||
.filter(order -> amountMatches(order.getAmount(), paidAmount))
|
||||
.ifPresent(this::applyPaid);
|
||||
}
|
||||
|
||||
static boolean amountMatches(BigDecimal expected, String paidAmount) {
|
||||
if (expected == null || paidAmount == null || paidAmount.isBlank()) {
|
||||
return false;
|
||||
}
|
||||
try {
|
||||
return expected.compareTo(new BigDecimal(paidAmount.trim())) == 0;
|
||||
} catch (NumberFormatException ex) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
@Transactional
|
||||
|
||||
@@ -25,6 +25,10 @@ app:
|
||||
# W produkcji ustaw APP_JWT_SECRET (min. 32 znaki) przez zmienna srodowiskowa.
|
||||
secret: ${APP_JWT_SECRET:local-dev-secret-change-me-please-32chars-min}
|
||||
expiration-seconds: ${APP_JWT_EXPIRATION_SECONDS:86400}
|
||||
payments:
|
||||
# true = bez skonfigurowanej bramki AutoPay zakup promowania jest od razu uznawany za oplacony.
|
||||
# Tylko do pracy lokalnej - na produkcji oznaczaloby darmowe promowanie dla kazdego.
|
||||
auto-confirm: ${APP_PAYMENTS_AUTO_CONFIRM:false}
|
||||
admin:
|
||||
email: ${APP_ADMIN_EMAIL:admin@softspm.com}
|
||||
password: ${APP_ADMIN_PASSWORD:Admin123!}
|
||||
|
||||
@@ -0,0 +1,33 @@
|
||||
package pl.polskalokalnie.listing;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.assertj.core.api.Assertions.assertThatThrownBy;
|
||||
|
||||
import java.util.List;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.springframework.web.server.ResponseStatusException;
|
||||
|
||||
class ListingServiceTest {
|
||||
|
||||
@Test
|
||||
void acceptsRasterImageDataUrls() {
|
||||
List<String> photos = List.of(
|
||||
"data:image/jpeg;base64,/9j/4AAQSkZJRg==",
|
||||
"data:image/png;base64,iVBORw0KGgo=",
|
||||
"data:image/webp;base64,UklGRg==");
|
||||
assertThat(ListingService.limitPhotos(photos)).containsExactlyElementsOf(photos);
|
||||
}
|
||||
|
||||
@Test
|
||||
void rejectsPhotosThatCouldRunScriptOnSiteOrigin() {
|
||||
// Regresja: data URL z text/html lub SVG serwowany z /api/og/... uruchamial skrypt w domenie serwisu.
|
||||
for (String photo : List.of(
|
||||
"data:text/html;base64,PHNjcmlwdD5hbGVydCgxKTwvc2NyaXB0Pg==",
|
||||
"data:image/svg+xml;base64,PHN2Zz48L3N2Zz4=",
|
||||
"https://evil.example/redirect",
|
||||
"javascript:alert(1)")) {
|
||||
assertThatThrownBy(() -> ListingService.limitPhotos(List.of(photo)))
|
||||
.isInstanceOf(ResponseStatusException.class);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,23 @@
|
||||
package pl.polskalokalnie.og;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
class OgControllerTest {
|
||||
|
||||
@Test
|
||||
void decodesRasterImage() {
|
||||
OgController.DecodedImage image = OgController.decodeDataUrl("data:image/png;base64,iVBORw0KGgo=");
|
||||
assertThat(image).isNotNull();
|
||||
assertThat(image.mimeType()).isEqualTo("image/png");
|
||||
}
|
||||
|
||||
@Test
|
||||
void refusesMimeTypesThatBrowsersExecute() {
|
||||
// Regresja: typ MIME z data URL trafial wprost do Content-Type odpowiedzi.
|
||||
assertThat(OgController.decodeDataUrl("data:text/html;base64,PHNjcmlwdD48L3NjcmlwdD4=")).isNull();
|
||||
assertThat(OgController.decodeDataUrl("data:image/svg+xml;base64,PHN2Zz48L3N2Zz4=")).isNull();
|
||||
assertThat(OgController.decodeDataUrl("https://evil.example/")).isNull();
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,84 @@
|
||||
package pl.polskalokalnie.promotion;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.assertj.core.api.Assertions.assertThatThrownBy;
|
||||
import static org.mockito.ArgumentMatchers.any;
|
||||
import static org.mockito.Mockito.mock;
|
||||
import static org.mockito.Mockito.never;
|
||||
import static org.mockito.Mockito.verify;
|
||||
import static org.mockito.Mockito.when;
|
||||
|
||||
import java.math.BigDecimal;
|
||||
import java.util.Map;
|
||||
import java.util.Optional;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.springframework.http.HttpStatus;
|
||||
import org.springframework.web.server.ResponseStatusException;
|
||||
import pl.polskalokalnie.listing.ListingRepository;
|
||||
import pl.polskalokalnie.payment.AutoPayService;
|
||||
import pl.polskalokalnie.payment.PaymentConfig;
|
||||
import pl.polskalokalnie.payment.PaymentConfigService;
|
||||
import pl.polskalokalnie.user.AppUser;
|
||||
import pl.polskalokalnie.user.UserRepository;
|
||||
|
||||
class PaymentSecurityTest {
|
||||
|
||||
private final PromotionPackageRepository packageRepository = mock(PromotionPackageRepository.class);
|
||||
private final PromotionOrderRepository orderRepository = mock(PromotionOrderRepository.class);
|
||||
private final UserRepository userRepository = mock(UserRepository.class);
|
||||
private final PaymentConfigService paymentConfigService = mock(PaymentConfigService.class);
|
||||
|
||||
private PromotionService service(boolean autoConfirm) {
|
||||
return new PromotionService(mock(PromotionPlanRepository.class), packageRepository, orderRepository,
|
||||
userRepository, mock(ListingRepository.class), paymentConfigService, mock(AutoPayService.class),
|
||||
"http://localhost", autoConfirm);
|
||||
}
|
||||
|
||||
@Test
|
||||
void checkoutWithoutGatewayIsRefusedInsteadOfBeingFree() {
|
||||
// Regresja: bez skonfigurowanej bramki zamowienie bylo od razu oplacone - darmowe kredyty dla kazdego.
|
||||
when(paymentConfigService.get()).thenReturn(new PaymentConfig());
|
||||
|
||||
assertThatThrownBy(() -> service(false).checkout("user@example.com",
|
||||
new CheckoutRequest(PromotionKind.PACKAGE, null, 1L, null, "AutoPay")))
|
||||
.isInstanceOfSatisfying(ResponseStatusException.class,
|
||||
ex -> assertThat(ex.getStatusCode()).isEqualTo(HttpStatus.SERVICE_UNAVAILABLE));
|
||||
verify(orderRepository, never()).save(any());
|
||||
verify(userRepository, never()).save(any());
|
||||
}
|
||||
|
||||
@Test
|
||||
void checkoutWithoutGatewayConfirmsOnlyWhenAutoConfirmIsEnabled() {
|
||||
when(paymentConfigService.get()).thenReturn(new PaymentConfig());
|
||||
PromotionPackage pack = new PromotionPackage();
|
||||
pack.setName("Pakiet");
|
||||
pack.setQuantity(5);
|
||||
pack.setPrice(new BigDecimal("199.99"));
|
||||
when(packageRepository.findById(any())).thenReturn(Optional.of(pack));
|
||||
when(orderRepository.save(any())).thenAnswer(inv -> inv.getArgument(0));
|
||||
AppUser user = new AppUser();
|
||||
when(userRepository.findByEmailIgnoreCase("user@example.com")).thenReturn(Optional.of(user));
|
||||
|
||||
service(true).checkout("user@example.com", new CheckoutRequest(PromotionKind.PACKAGE, null, 1L, null, "AutoPay"));
|
||||
|
||||
assertThat(user.getPromotionCredits()).isEqualTo(5);
|
||||
}
|
||||
|
||||
@Test
|
||||
void itnCountsOnlyExplicitSuccessAsPaid() {
|
||||
// Regresja: PENDING oraz brak statusu byly traktowane jak oplacona platnosc.
|
||||
assertThat(AutoPayItnController.isSuccess(Map.of("PaymentStatus", "SUCCESS"))).isTrue();
|
||||
assertThat(AutoPayItnController.isSuccess(Map.of("PaymentStatus", "PENDING"))).isFalse();
|
||||
assertThat(AutoPayItnController.isSuccess(Map.of("PaymentStatus", "FAILURE"))).isFalse();
|
||||
assertThat(AutoPayItnController.isSuccess(Map.of("OrderID", "PL-1"))).isFalse();
|
||||
}
|
||||
|
||||
@Test
|
||||
void itnAmountMustMatchOrder() {
|
||||
assertThat(PromotionService.amountMatches(new BigDecimal("49.99"), "49.99")).isTrue();
|
||||
assertThat(PromotionService.amountMatches(new BigDecimal("49.99"), "49.990")).isTrue();
|
||||
assertThat(PromotionService.amountMatches(new BigDecimal("49.99"), "0.01")).isFalse();
|
||||
assertThat(PromotionService.amountMatches(new BigDecimal("49.99"), null)).isFalse();
|
||||
assertThat(PromotionService.amountMatches(new BigDecimal("49.99"), "abc")).isFalse();
|
||||
}
|
||||
}
|
||||
@@ -39,6 +39,9 @@ services:
|
||||
# musi byc wlasna, inaczej kazdy moze wystawic sobie token administratora.
|
||||
APP_JWT_SECRET: ${APP_JWT_SECRET:-local-dev-secret-change-me-please-32chars-min}
|
||||
APP_PUBLIC_BASE_URL: ${APP_PUBLIC_BASE_URL:-http://localhost}
|
||||
# Lokalnie promowanie dziala bez kluczy AutoPay (platnosc potwierdzana automatycznie).
|
||||
# docker-compose.prod.yml tej zmiennej nie przekazuje - na produkcji zawsze false.
|
||||
APP_PAYMENTS_AUTO_CONFIRM: ${APP_PAYMENTS_AUTO_CONFIRM:-true}
|
||||
depends_on:
|
||||
db:
|
||||
condition: service_healthy
|
||||
|
||||
@@ -217,7 +217,7 @@ export function AdminPromotionView({ Icon }: { Icon: IconComponent }) {
|
||||
<span className="mkt-config-icon"><Icon name="credit-card" /></span>
|
||||
<h2>Bramka szybkich płatności - AutoPay</h2>
|
||||
</div>
|
||||
<p className="banner-slot-hint"><Icon name="lock" /> Bez danych dostępowych działa tryb sandbox (płatność potwierdzana automatycznie).</p>
|
||||
<p className="banner-slot-hint"><Icon name="lock" /> Bez danych dostępowych i włączonej bramki zakup promowania jest niedostępny.</p>
|
||||
<div className="mkt-config-grid">
|
||||
<label>ServiceID
|
||||
<input value={pay.serviceId} onChange={(e) => setPay({ ...pay, serviceId: e.target.value })} placeholder="np. 123456" />
|
||||
|
||||
Reference in New Issue
Block a user