diff --git a/CHANGELOG.md b/CHANGELOG.md index f3281630..fee93cf1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * `geolocation-country-code`: Allows to perform redirections based on the ISO 3166-1 alpha-2 two-letter country code resolved while geolocating the visitor. * `geolocation-city-name`: Allows to perform redirections based on the city name resolved while geolocating the visitor. +* [#2032](https://github.com/shlinkio/shlink/issues/2032) Save the URL to which a visitor is redirected when a visit is tracked. + + The value is exposed in the API as a new `redirectUrl` field for visit objects. + + This is useful to know where a visitor was redirected for a short URL with dynamic redirect rules, for special redirects, or simply in case the long URL was changed over time, and you still want to know where visitors were redirected originally. + + Some visits may not have a redirect URL if a redirect didn't happen, like for orphan visits when no special redirects are configured, or when a visit is tracked as part of the pixel action. + ### Changed * [#2193](https://github.com/shlinkio/shlink/issues/2193) API keys are now hashed using SHA256, instead of being saved in plain text. diff --git a/config/constants.php b/config/constants.php index d6bb9621..09df0e60 100644 --- a/config/constants.php +++ b/config/constants.php @@ -22,3 +22,4 @@ const DEFAULT_QR_CODE_ENABLED_FOR_DISABLED_SHORT_URLS = true; const DEFAULT_QR_CODE_COLOR = '#000000'; // Black const DEFAULT_QR_CODE_BG_COLOR = '#ffffff'; // White const IP_ADDRESS_REQUEST_ATTRIBUTE = 'remote_address'; +const REDIRECT_URL_REQUEST_ATTRIBUTE = 'redirect_url'; diff --git a/docs/async-api/async-api.json b/docs/async-api/async-api.json index b2da154b..2d69084b 100644 --- a/docs/async-api/async-api.json +++ b/docs/async-api/async-api.json @@ -247,6 +247,11 @@ "type": "string", "nullable": true, "description": "The originally visited URL that triggered the tracking of this visit" + }, + "redirectUrl": { + "type": "string", + "nullable": true, + "description": "The URL to which the visitor was redirected, or null if a redirect did not occur, like for 404 requests or pixel tracking" } }, "example": { diff --git a/docs/swagger/definitions/Visit.json b/docs/swagger/definitions/Visit.json index c4589bb1..2ccdfd23 100644 --- a/docs/swagger/definitions/Visit.json +++ b/docs/swagger/definitions/Visit.json @@ -25,6 +25,10 @@ "visitedUrl": { "type": ["string", "null"], "description": "The originally visited URL that triggered the tracking of this visit" + }, + "redirectUrl": { + "type": ["string", "null"], + "description": "The URL to which the visitor was redirected, or null if a redirect did not occur, like for 404 requests or pixel tracking" } } } diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Visit.Entity.Visit.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Visit.Entity.Visit.php index 7d402384..34d98572 100644 --- a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Visit.Entity.Visit.php +++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Visit.Entity.Visit.php @@ -75,4 +75,10 @@ return static function (ClassMetadata $metadata, array $emConfig): void { ->columnName('potential_bot') ->option('default', false) ->build(); + + fieldWithUtf8Charset($builder->createField('redirectUrl', Types::STRING), $emConfig) + ->columnName('redirect_url') + ->length(Visitor::REDIRECT_URL_MAX_LENGTH) + ->nullable() + ->build(); }; diff --git a/module/Core/migrations/Version20241124112257.php b/module/Core/migrations/Version20241124112257.php new file mode 100644 index 00000000..c11cbe2b --- /dev/null +++ b/module/Core/migrations/Version20241124112257.php @@ -0,0 +1,39 @@ +getTable('visits'); + $this->skipIf($visits->hasColumn(self::COLUMN_NAME)); + + $visits->addColumn(self::COLUMN_NAME, Types::STRING, [ + 'length' => 2048, + 'notnull' => false, + 'default' => null, + ]); + } + + public function down(Schema $schema): void + { + $visits = $schema->getTable('visits'); + $this->skipIf(! $visits->hasColumn(self::COLUMN_NAME)); + $visits->dropColumn(self::COLUMN_NAME); + } + + public function isTransactional(): bool + { + return ! ($this->connection->getDatabasePlatform() instanceof MySQLPlatform); + } +} diff --git a/module/Core/src/Action/AbstractTrackingAction.php b/module/Core/src/Action/AbstractTrackingAction.php index 78eebc05..ff35828f 100644 --- a/module/Core/src/Action/AbstractTrackingAction.php +++ b/module/Core/src/Action/AbstractTrackingAction.php @@ -16,6 +16,8 @@ use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\Visit\RequestTrackerInterface; +use const Shlinkio\Shlink\REDIRECT_URL_REQUEST_ATTRIBUTE; + abstract class AbstractTrackingAction implements MiddlewareInterface, RequestMethodInterface { public function __construct( @@ -30,9 +32,13 @@ abstract class AbstractTrackingAction implements MiddlewareInterface, RequestMet try { $shortUrl = $this->urlResolver->resolveEnabledShortUrl($identifier); - $this->requestTracker->trackIfApplicable($shortUrl, $request); + $response = $this->createSuccessResp($shortUrl, $request); + $this->requestTracker->trackIfApplicable($shortUrl, $request->withAttribute( + REDIRECT_URL_REQUEST_ATTRIBUTE, + $response->hasHeader('Location') ? $response->getHeaderLine('Location') : null, + )); - return $this->createSuccessResp($shortUrl, $request); + return $response; } catch (ShortUrlNotFoundException) { return $this->createErrorResp($request, $handler); } diff --git a/module/Core/src/ErrorHandler/NotFoundTrackerMiddleware.php b/module/Core/src/ErrorHandler/NotFoundTrackerMiddleware.php index f3342c5a..633d83db 100644 --- a/module/Core/src/ErrorHandler/NotFoundTrackerMiddleware.php +++ b/module/Core/src/ErrorHandler/NotFoundTrackerMiddleware.php @@ -10,7 +10,9 @@ use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Core\Visit\RequestTrackerInterface; -class NotFoundTrackerMiddleware implements MiddlewareInterface +use const Shlinkio\Shlink\REDIRECT_URL_REQUEST_ATTRIBUTE; + +readonly class NotFoundTrackerMiddleware implements MiddlewareInterface { public function __construct(private RequestTrackerInterface $requestTracker) { @@ -18,7 +20,12 @@ class NotFoundTrackerMiddleware implements MiddlewareInterface public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface { - $this->requestTracker->trackNotFoundIfApplicable($request); - return $handler->handle($request); + $response = $handler->handle($request); + $this->requestTracker->trackNotFoundIfApplicable($request->withAttribute( + REDIRECT_URL_REQUEST_ATTRIBUTE, + $response->hasHeader('Location') ? $response->getHeaderLine('Location') : null, + )); + + return $response; } } diff --git a/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php b/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php index 4a02f6e9..4b013b33 100644 --- a/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php +++ b/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php @@ -25,6 +25,8 @@ use function implode; use function sprintf; use function trim; +use const Shlinkio\Shlink\REDIRECT_URL_REQUEST_ATTRIBUTE; + readonly class ExtraPathRedirectMiddleware implements MiddlewareInterface { public function __construct( @@ -73,9 +75,12 @@ readonly class ExtraPathRedirectMiddleware implements MiddlewareInterface try { $shortUrl = $this->resolver->resolveEnabledShortUrl($identifier); - $this->requestTracker->trackIfApplicable($shortUrl, $request); - $longUrl = $this->redirectionBuilder->buildShortUrlRedirect($shortUrl, $request, $extraPath); + $this->requestTracker->trackIfApplicable( + $shortUrl, + $request->withAttribute(REDIRECT_URL_REQUEST_ATTRIBUTE, $longUrl), + ); + return $this->redirectResponseHelper->buildRedirectResponse($longUrl); } catch (ShortUrlNotFoundException) { if ($extraPath === null || ! $this->urlShortenerOptions->multiSegmentSlugsEnabled) { diff --git a/module/Core/src/Visit/Entity/Visit.php b/module/Core/src/Visit/Entity/Visit.php index 9e8540bc..70733593 100644 --- a/module/Core/src/Visit/Entity/Visit.php +++ b/module/Core/src/Visit/Entity/Visit.php @@ -28,6 +28,7 @@ class Visit extends AbstractEntity implements JsonSerializable public readonly bool $potentialBot, public readonly string|null $remoteAddr = null, public readonly string|null $visitedUrl = null, + public readonly string|null $redirectUrl = null, private VisitLocation|null $visitLocation = null, public readonly Chronos $date = new Chronos(), ) { @@ -68,6 +69,7 @@ class Visit extends AbstractEntity implements JsonSerializable potentialBot: $visitor->potentialBot, remoteAddr: self::processAddress($visitor->remoteAddress, $anonymize), visitedUrl: $visitor->visitedUrl, + redirectUrl: $visitor->redirectUrl, visitLocation: $geolocation !== null ? VisitLocation::fromGeolocation($geolocation) : null, ); } @@ -156,6 +158,7 @@ class Visit extends AbstractEntity implements JsonSerializable 'visitLocation' => $this->visitLocation, 'potentialBot' => $this->potentialBot, 'visitedUrl' => $this->visitedUrl, + 'redirectUrl' => $this->redirectUrl, ]; if (! $this->isOrphan()) { return $base; diff --git a/module/Core/src/Visit/Model/Visitor.php b/module/Core/src/Visit/Model/Visitor.php index e13712e1..cab834e6 100644 --- a/module/Core/src/Visit/Model/Visitor.php +++ b/module/Core/src/Visit/Model/Visitor.php @@ -13,12 +13,15 @@ use function Shlinkio\Shlink\Core\ipAddressFromRequest; use function Shlinkio\Shlink\Core\isCrawler; use function substr; +use const Shlinkio\Shlink\REDIRECT_URL_REQUEST_ATTRIBUTE; + final readonly class Visitor { public const USER_AGENT_MAX_LENGTH = 512; public const REFERER_MAX_LENGTH = 1024; public const REMOTE_ADDRESS_MAX_LENGTH = 256; public const VISITED_URL_MAX_LENGTH = 2048; + public const REDIRECT_URL_MAX_LENGTH = 2048; private function __construct( public string $userAgent, @@ -27,6 +30,7 @@ final readonly class Visitor public string $visitedUrl, public bool $potentialBot, public Location|null $geolocation, + public string|null $redirectUrl, ) { } @@ -36,6 +40,7 @@ final readonly class Visitor string|null $remoteAddress = null, string $visitedUrl = '', Location|null $geolocation = null, + string|null $redirectUrl = null, ): self { return new self( userAgent: self::cropToLength($userAgent, self::USER_AGENT_MAX_LENGTH), @@ -46,6 +51,7 @@ final readonly class Visitor visitedUrl: self::cropToLength($visitedUrl, self::VISITED_URL_MAX_LENGTH), potentialBot: isCrawler($userAgent), geolocation: $geolocation, + redirectUrl: $redirectUrl === null ? null : self::cropToLength($redirectUrl, self::REDIRECT_URL_MAX_LENGTH), ); } @@ -62,6 +68,7 @@ final readonly class Visitor remoteAddress: ipAddressFromRequest($request), visitedUrl: $request->getUri()->__toString(), geolocation: geolocationFromRequest($request), + redirectUrl: $request->getAttribute(REDIRECT_URL_REQUEST_ATTRIBUTE), ); } @@ -85,6 +92,7 @@ final readonly class Visitor // Keep the fact that the visit was a potential bot, even if we no longer save the user agent potentialBot: $this->potentialBot, geolocation: $this->geolocation, + redirectUrl: $this->redirectUrl, ); } } diff --git a/module/Core/test/Action/PixelActionTest.php b/module/Core/test/Action/PixelActionTest.php index d6f2566a..e78df177 100644 --- a/module/Core/test/Action/PixelActionTest.php +++ b/module/Core/test/Action/PixelActionTest.php @@ -16,6 +16,8 @@ use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\Visit\RequestTrackerInterface; +use const Shlinkio\Shlink\REDIRECT_URL_REQUEST_ATTRIBUTE; + class PixelActionTest extends TestCase { private PixelAction $action; @@ -34,12 +36,17 @@ class PixelActionTest extends TestCase public function imageIsReturned(): void { $shortCode = 'abc123'; + $shortUrl = ShortUrl::withLongUrl('http://domain.com/foo/bar'); + $request = (new ServerRequest())->withAttribute('shortCode', $shortCode); + $this->urlResolver->expects($this->once())->method('resolveEnabledShortUrl')->with( ShortUrlIdentifier::fromShortCodeAndDomain($shortCode, ''), - )->willReturn(ShortUrl::withLongUrl('http://domain.com/foo/bar')); - $this->requestTracker->expects($this->once())->method('trackIfApplicable')->withAnyParameters(); + )->willReturn($shortUrl); + $this->requestTracker->expects($this->once())->method('trackIfApplicable')->with( + $shortUrl, + $request->withAttribute(REDIRECT_URL_REQUEST_ATTRIBUTE, null), + ); - $request = (new ServerRequest())->withAttribute('shortCode', $shortCode); $response = $this->action->process($request, $this->createMock(RequestHandlerInterface::class)); self::assertInstanceOf(PixelResponse::class, $response); diff --git a/module/Core/test/Action/RedirectActionTest.php b/module/Core/test/Action/RedirectActionTest.php index 2364371c..fa4a561d 100644 --- a/module/Core/test/Action/RedirectActionTest.php +++ b/module/Core/test/Action/RedirectActionTest.php @@ -19,6 +19,8 @@ use Shlinkio\Shlink\Core\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\Util\RedirectResponseHelperInterface; use Shlinkio\Shlink\Core\Visit\RequestTrackerInterface; +use const Shlinkio\Shlink\REDIRECT_URL_REQUEST_ATTRIBUTE; + class RedirectActionTest extends TestCase { private const LONG_URL = 'https://domain.com/foo/bar?some=thing'; @@ -50,16 +52,20 @@ class RedirectActionTest extends TestCase { $shortCode = 'abc123'; $shortUrl = ShortUrl::withLongUrl(self::LONG_URL); + $expectedResp = new Response\RedirectResponse(self::LONG_URL); + $request = (new ServerRequest())->withAttribute('shortCode', $shortCode); + $this->urlResolver->expects($this->once())->method('resolveEnabledShortUrl')->with( ShortUrlIdentifier::fromShortCodeAndDomain($shortCode, ''), )->willReturn($shortUrl); - $this->requestTracker->expects($this->once())->method('trackIfApplicable'); - $expectedResp = new Response\RedirectResponse(self::LONG_URL); + $this->requestTracker->expects($this->once())->method('trackIfApplicable')->with( + $shortUrl, + $request->withAttribute(REDIRECT_URL_REQUEST_ATTRIBUTE, self::LONG_URL), + ); $this->redirectRespHelper->expects($this->once())->method('buildRedirectResponse')->with( self::LONG_URL, )->willReturn($expectedResp); - $request = (new ServerRequest())->withAttribute('shortCode', $shortCode); $response = $this->action->process($request, $this->createMock(RequestHandlerInterface::class)); self::assertSame($expectedResp, $response); diff --git a/module/Core/test/ErrorHandler/NotFoundTrackerMiddlewareTest.php b/module/Core/test/ErrorHandler/NotFoundTrackerMiddlewareTest.php index 4558197b..9df12a6d 100644 --- a/module/Core/test/ErrorHandler/NotFoundTrackerMiddlewareTest.php +++ b/module/Core/test/ErrorHandler/NotFoundTrackerMiddlewareTest.php @@ -4,7 +4,9 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\ErrorHandler; +use Laminas\Diactoros\Response; use Laminas\Diactoros\ServerRequestFactory; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; @@ -14,6 +16,8 @@ use Shlinkio\Shlink\Core\ErrorHandler\Model\NotFoundType; use Shlinkio\Shlink\Core\ErrorHandler\NotFoundTrackerMiddleware; use Shlinkio\Shlink\Core\Visit\RequestTrackerInterface; +use const Shlinkio\Shlink\REDIRECT_URL_REQUEST_ATTRIBUTE; + class NotFoundTrackerMiddlewareTest extends TestCase { private NotFoundTrackerMiddleware $middleware; @@ -33,12 +37,22 @@ class NotFoundTrackerMiddlewareTest extends TestCase ); } - #[Test] - public function delegatesIntoRequestTracker(): void + #[Test, DataProvider('provideResponses')] + public function delegatesIntoRequestTracker(Response $resp, string|null $expectedRedirectUrl): void { - $this->handler->expects($this->once())->method('handle')->with($this->request); - $this->requestTracker->expects($this->once())->method('trackNotFoundIfApplicable')->with($this->request); + $this->handler->expects($this->once())->method('handle')->with($this->request)->willReturn($resp); + $this->requestTracker->expects($this->once())->method('trackNotFoundIfApplicable')->with( + $this->request->withAttribute(REDIRECT_URL_REQUEST_ATTRIBUTE, $expectedRedirectUrl), + ); - $this->middleware->process($this->request, $this->handler); + $result = $this->middleware->process($this->request, $this->handler); + + self::assertSame($resp, $result); + } + + public static function provideResponses(): iterable + { + yield 'no location response' => [new Response(), null]; + yield 'location response' => [new Response\RedirectResponse('the_location'), 'the_location']; } } diff --git a/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php b/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php index 2e232038..310c8b3f 100644 --- a/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php +++ b/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php @@ -80,6 +80,7 @@ class PublishingUpdatesGeneratorTest extends TestCase 'date' => $visit->date->toAtomString(), 'potentialBot' => false, 'visitedUrl' => '', + 'redirectUrl' => null, ], ], $update->payload); } @@ -105,6 +106,7 @@ class PublishingUpdatesGeneratorTest extends TestCase 'potentialBot' => false, 'visitedUrl' => $orphanVisit->visitedUrl, 'type' => $orphanVisit->type->value, + 'redirectUrl' => null, ], ], $update->payload); } diff --git a/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php b/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php index 85168020..84ceb790 100644 --- a/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php +++ b/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php @@ -30,6 +30,8 @@ use Shlinkio\Shlink\Core\Visit\RequestTrackerInterface; use function Laminas\Stratigility\middleware; use function str_starts_with; +use const Shlinkio\Shlink\REDIRECT_URL_REQUEST_ATTRIBUTE; + class ExtraPathRedirectMiddlewareTest extends TestCase { private MockObject & ShortUrlResolverInterface $resolver; @@ -159,7 +161,10 @@ class ExtraPathRedirectMiddlewareTest extends TestCase $this->redirectResponseHelper->expects($this->once())->method('buildRedirectResponse')->with( 'the_built_long_url', )->willReturn(new RedirectResponse('')); - $this->requestTracker->expects($this->once())->method('trackIfApplicable')->with($shortUrl, $request); + $this->requestTracker->expects($this->once())->method('trackIfApplicable')->with( + $shortUrl, + $request->withAttribute(REDIRECT_URL_REQUEST_ATTRIBUTE, 'the_built_long_url'), + ); $this->middleware($options)->process($request, $this->handler); } diff --git a/module/Core/test/Visit/Entity/VisitTest.php b/module/Core/test/Visit/Entity/VisitTest.php index db23af97..438ca55f 100644 --- a/module/Core/test/Visit/Entity/VisitTest.php +++ b/module/Core/test/Visit/Entity/VisitTest.php @@ -34,6 +34,7 @@ class VisitTest extends TestCase 'visitLocation' => null, 'potentialBot' => $expectedToBePotentialBot, 'visitedUrl' => $visit->visitedUrl, + 'redirectUrl' => $visit->redirectUrl, ], $visit->jsonSerialize()); } @@ -67,6 +68,7 @@ class VisitTest extends TestCase 'potentialBot' => false, 'visitedUrl' => '', 'type' => VisitType::BASE_URL->value, + 'redirectUrl' => null, ], ]; yield 'invalid short url visit' => [ @@ -83,6 +85,7 @@ class VisitTest extends TestCase 'potentialBot' => false, 'visitedUrl' => 'https://example.com/foo', 'type' => VisitType::INVALID_SHORT_URL->value, + 'redirectUrl' => null, ], ]; yield 'regular 404 visit' => [ @@ -101,6 +104,7 @@ class VisitTest extends TestCase 'potentialBot' => false, 'visitedUrl' => 'https://s.test/foo/bar', 'type' => VisitType::REGULAR_404->value, + 'redirectUrl' => null, ], ]; } diff --git a/module/Rest/test-api/Action/OrphanVisitsTest.php b/module/Rest/test-api/Action/OrphanVisitsTest.php index cf7cee0f..3761e113 100644 --- a/module/Rest/test-api/Action/OrphanVisitsTest.php +++ b/module/Rest/test-api/Action/OrphanVisitsTest.php @@ -21,6 +21,7 @@ class OrphanVisitsTest extends ApiTestCase 'potentialBot' => true, 'visitedUrl' => 'foo.com', 'type' => 'invalid_short_url', + 'redirectUrl' => null, ]; private const REGULAR_NOT_FOUND = [ 'referer' => 'https://s.test/foo/bar', @@ -30,6 +31,7 @@ class OrphanVisitsTest extends ApiTestCase 'potentialBot' => false, 'visitedUrl' => '', 'type' => 'regular_404', + 'redirectUrl' => null, ]; private const BASE_URL = [ 'referer' => 'https://s.test', @@ -39,6 +41,7 @@ class OrphanVisitsTest extends ApiTestCase 'potentialBot' => false, 'visitedUrl' => '', 'type' => 'base_url', + 'redirectUrl' => null, ]; #[Test, DataProvider('provideQueries')]