From c0d83869d94df15a639fcfdec86b9d4f9d991370 Mon Sep 17 00:00:00 2001 From: Fayupable <90789180+Fayupable@users.noreply.github.com> Date: Fri, 14 Aug 2026 20:54:26 +0300 Subject: [PATCH] Add caller IP recording for security-sensitive methods, logged-test module, and JSON emitter - Add @Logged(includeIp = true): records the caller's IP address alongside callerIdentity, instead of only as its fallback tier, for security-sensitive operations (login, password reset, TOTP verification, admin mutations) where IP remains valuable for audit/rate-limiting even on a failed attempt. Adds MethodInvocationEvent#callerIp(), IClientInfoPort#resolveCallerIp() (a default method, source-compatible with existing implementations), real IP resolution in SpringSecurityClientInfoAdapter/HttpRequestClientInfoAdapter, LoggedMdcKeys.CALLER_IP, callerIp output in both emitters, and IP support in logged-test's InMemoryClientInfoPort. - Add logged-test module: in-memory test doubles for all three output ports. - Add JsonInvocationEventEmitter for structured JSON log output. Bumps version to 1.3.0. --- CHANGELOG.md | 11 +- README.md | 15 +++ logged-benchmarks/dependency-reduced-pom.xml | 2 +- logged-benchmarks/pom.xml | 2 +- logged-core/pom.xml | 2 +- .../logged/core/annotation/Logged.java | 27 ++++ .../core/model/MethodInvocationEvent.java | 8 ++ .../logged/core/port/IClientInfoPort.java | 31 +++++ .../logged/core/port/NoOpAdaptersTest.java | 18 ++- logged-spring/pom.xml | 2 +- .../logged/spring/aspect/LoggedAspect.java | 38 +++--- .../logged/spring/aspect/LoggedMdcKeys.java | 8 ++ .../logged/spring/aspect/MdcPropagation.java | 21 ++- .../emitter/JsonInvocationEventEmitter.java | 2 + .../emitter/Slf4jInvocationEventEmitter.java | 16 ++- .../HttpRequestClientInfoAdapter.java | 5 + .../security/RequestClientIpResolver.java | 25 +++- .../SpringSecurityClientInfoAdapter.java | 12 ++ .../spring/aspect/LoggedAspectTest.java | 123 +++++++++++++++++- .../spring/aspect/MdcPropagationTest.java | 62 ++++++++- .../JsonInvocationEventEmitterTest.java | 33 ++++- .../Slf4jInvocationEventEmitterTest.java | 35 ++++- .../HttpRequestClientInfoAdapterTest.java | 23 ++++ .../SpringSecurityClientInfoAdapterTest.java | 28 ++++ logged-test/pom.xml | 2 +- .../logged/test/InMemoryClientInfoPort.java | 19 +++ .../test/InMemoryClientInfoPortTest.java | 28 ++++ .../InMemoryInvocationEventEmitterTest.java | 2 +- pom.xml | 2 +- 29 files changed, 550 insertions(+), 52 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9460539..b91ee80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,14 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and ## [Unreleased] +## [1.3.0] - 2026-08-14 + +### Added + +- New `logged-test` module: in-memory, thread-safe test doubles for all three output ports — `InMemoryInvocationEventEmitter`, `InMemoryMetricsRecorder` (backed by a new `RecordedMetric` record), and `InMemoryClientInfoPort` — so a consuming project can assert directly on `@Logged` behavior in tests instead of parsing log output or hand-rolling its own stubs. Depends only on `logged-core`, so it works with or without `logged-spring` on the classpath. +- New `JsonInvocationEventEmitter` (`logged-spring`): writes each `MethodInvocationEvent` as a single line of structured JSON through SLF4J instead of `Slf4jInvocationEventEmitter`'s human-readable line, for applications shipping logs to Loki/Elasticsearch/Datadog and similar backends that parse each line as JSON. Uses a small hand-written JSON writer with proper string escaping instead of adding a JSON library dependency. +- `@Logged(includeIp = true)`: records the caller's IP address alongside whatever `callerIdentity` resolves to, instead of only as its fallback tier, for security-sensitive operations (login, password reset, TOTP verification, admin mutations) where the IP remains valuable for audit/rate-limiting purposes even when the call also resolves to an authenticated identity — especially on a failed attempt. Adds `MethodInvocationEvent#callerIp()`, `IClientInfoPort#resolveCallerIp()` (a `default` method returning `null`, so existing implementations remain source-compatible), real implementations in `SpringSecurityClientInfoAdapter`/`HttpRequestClientInfoAdapter`, a new `LoggedMdcKeys.CALLER_IP` MDC key, `callerIp` output in both `Slf4jInvocationEventEmitter` and `JsonInvocationEventEmitter`, and IP support in `logged-test`'s `InMemoryClientInfoPort`. Resolved synchronously alongside `callerIdentity`, so it is correctly captured for `CompletableFuture`-returning methods the same way `callerIdentity` already is. + ## [1.2.2] - 2026-08-14 ### Fixed @@ -61,7 +69,8 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and - `logged-benchmarks`: internal JMH benchmarks measuring the aspect's overhead against a direct, uninstrumented call. - Quality gates: Pitest mutation testing (`logged-core` 100%, `logged-spring` 98%) and Checkstyle (0 violations), both enforced via `mvn verify`. -[Unreleased]: https://github.com/fayupable/logged-lib/compare/v1.2.2...HEAD +[Unreleased]: https://github.com/fayupable/logged-lib/compare/v1.3.0...HEAD +[1.3.0]: https://github.com/fayupable/logged-lib/compare/v1.2.2...v1.3.0 [1.2.2]: https://github.com/fayupable/logged-lib/compare/v1.2.0...v1.2.2 [1.2.0]: https://github.com/fayupable/logged-lib/compare/v1.1.0...v1.2.0 [1.1.0]: https://github.com/fayupable/logged-lib/compare/v1.0.0...v1.1.0 diff --git a/README.md b/README.md index a26bed2..37e2c13 100644 --- a/README.md +++ b/README.md @@ -536,6 +536,21 @@ The default `IClientInfoPort` (active whenever Spring Security and Spring Web ar The client IP is read from `getRemoteAddr()`, **not** from `X-Forwarded-For`, by default. That header is controlled by the client and can be forged by anyone unless the application sits behind a proxy configured to strip and re-set it — a deployment detail this library cannot know on its own. If your application does sit behind such a proxy, set `logged.client-info.trust-forwarded-headers=true` explicitly, mirroring how Spring Security itself requires trusted proxies to be declared rather than assumed. +### Recording the caller's IP alongside identity + +The resolution chain above is exclusive: once an authenticated principal resolves, the IP is discarded. For most `@Logged` methods that's the right tradeoff. For a small set of security-sensitive operations — login, password reset, TOTP verification, admin mutations — the IP remains valuable for audit and rate-limiting purposes even when the call also resolves to an authenticated identity, and especially on a *failed* attempt, where the caller's identity may be unverified or entirely absent: + +```java +@Logged(includeIp = true) +public LoginResponse login(String username, String password) { + // ... +} +``` + +Setting `includeIp = true` records the caller's IP unconditionally, in `MethodInvocationEvent#callerIp()` and, if MDC is enabled, under `LoggedMdcKeys.CALLER_IP` (`logged.callerIp`) — as a field alongside `callerIdentity`, not in place of it, and independent of whatever the identity tier resolves to. This is per-annotation, matching `slowThresholdMs`/`sampleRate`, rather than a global flag: IP is personal data under most privacy frameworks, so it is only recorded on the methods that explicitly opt in, not on every `@Logged` call across the application. + +`callerIp` is resolved synchronously on the calling thread, at the same point `callerIdentity` is — including for a method returning `CompletableFuture`, where it is captured before the async work begins, for the same reason described in [Async, `@Async`, and virtual thread support](#async-async-and-virtual-thread-support). It is `null` whenever it cannot be resolved (no active HTTP request, or a custom `IClientInfoPort` that does not implement `resolveCallerIp()`), never a placeholder string like `"unknown"` — consistent with `exceptionType`/`rootCauseType`, this library's other nullable structured fields. + ## Benchmarks `LoggedAspect`'s overhead is measured with [JMH](https://openjdk.org/projects/code-tools/jmh/), comparing a direct method call against the same call made through a `@Logged`-intercepted proxy, with every port wired to its no-op implementation. This isolates the cost of proxy dispatch, `FlowContext` `ThreadLocal` management, and the emission/metrics decision path, from the cost of any actual logging or metrics backend I/O. diff --git a/logged-benchmarks/dependency-reduced-pom.xml b/logged-benchmarks/dependency-reduced-pom.xml index 1b9f8b0..ef45b47 100644 --- a/logged-benchmarks/dependency-reduced-pom.xml +++ b/logged-benchmarks/dependency-reduced-pom.xml @@ -3,7 +3,7 @@ logged-lib com.fayupable - 1.2.2 + 1.3.0 4.0.0 logged-benchmarks diff --git a/logged-benchmarks/pom.xml b/logged-benchmarks/pom.xml index 7585639..fc0174e 100644 --- a/logged-benchmarks/pom.xml +++ b/logged-benchmarks/pom.xml @@ -7,7 +7,7 @@ com.fayupable logged-lib - 1.2.2 + 1.3.0 logged-benchmarks diff --git a/logged-core/pom.xml b/logged-core/pom.xml index 9bf7b3a..a51e48a 100644 --- a/logged-core/pom.xml +++ b/logged-core/pom.xml @@ -7,7 +7,7 @@ com.fayupable logged-lib - 1.2.2 + 1.3.0 logged-core diff --git a/logged-core/src/main/java/com/fayupable/logged/core/annotation/Logged.java b/logged-core/src/main/java/com/fayupable/logged/core/annotation/Logged.java index f5bb458..a1d8b59 100644 --- a/logged-core/src/main/java/com/fayupable/logged/core/annotation/Logged.java +++ b/logged-core/src/main/java/com/fayupable/logged/core/annotation/Logged.java @@ -68,4 +68,31 @@ * @return the sampling rate, defaulting to 1.0 (log every call) */ double sampleRate() default 1.0; + + /** + * Whether the caller's IP address should be resolved and recorded + * alongside whatever else the caller-identity resolver produces, + * instead of only as a fallback used when no identity is available. + * + *

By default, an interceptor's caller-identity resolution is an + * either/or chain: an authenticated principal, if one resolves, takes + * priority over the caller's IP, which is then discarded. For most + * {@code @Logged} methods that is the right tradeoff — knowing who + * called is more useful than knowing where from, and recording both + * unconditionally would add IP address (personal data under most + * privacy frameworks) to every log line for no benefit. + * + *

For a small set of security-sensitive operations — login, password + * reset, TOTP verification, admin mutations — the IP remains valuable + * for audit and rate-limiting purposes even when the call also resolves + * to an authenticated identity, and especially on a failed attempt, + * where the caller's identity may be unverified or entirely absent. + * Setting this to {@code true} on exactly those methods records the IP + * unconditionally, without changing IP resolution for every other + * {@code @Logged} method in the application. + * + * @return whether to record the caller's IP address in addition to + * caller identity, defaulting to {@code false} + */ + boolean includeIp() default false; } \ No newline at end of file diff --git a/logged-core/src/main/java/com/fayupable/logged/core/model/MethodInvocationEvent.java b/logged-core/src/main/java/com/fayupable/logged/core/model/MethodInvocationEvent.java index ac9b109..4300582 100644 --- a/logged-core/src/main/java/com/fayupable/logged/core/model/MethodInvocationEvent.java +++ b/logged-core/src/main/java/com/fayupable/logged/core/model/MethodInvocationEvent.java @@ -41,6 +41,13 @@ * call (for example an authenticated user id or a * client IP address), resolved by an adapter-specific * implementation + * @param callerIp the caller's IP address, resolved independently of + * {@code callerIdentity} rather than only as its + * fallback tier, or {@code null} if not resolved. + * Only populated when {@link com.fayupable.logged.core.annotation.Logged#includeIp()} + * is {@code true} for the invoked method, or when + * the caller-identity resolver in use does not + * support resolving it at all * @param traceId the {@link FlowContext#traceId()} shared by every * call in the same chain of nested {@code @Logged} * invocations, allowing log output to be @@ -58,6 +65,7 @@ public record MethodInvocationEvent( String exceptionType, String rootCauseType, String callerIdentity, + String callerIp, String traceId, int depth ) { diff --git a/logged-core/src/main/java/com/fayupable/logged/core/port/IClientInfoPort.java b/logged-core/src/main/java/com/fayupable/logged/core/port/IClientInfoPort.java index 744bd5a..3b27468 100644 --- a/logged-core/src/main/java/com/fayupable/logged/core/port/IClientInfoPort.java +++ b/logged-core/src/main/java/com/fayupable/logged/core/port/IClientInfoPort.java @@ -29,4 +29,35 @@ public interface IClientInfoPort { * resolved */ String resolveCallerIdentity(); + + /** + * Resolves the IP address of the caller that triggered the current + * invocation, independent of whatever {@link #resolveCallerIdentity()} + * itself resolves to. + * + *

Unlike {@link #resolveCallerIdentity()}, which resolves a single + * identity from an either/or chain of tiers (authenticated principal, + * then IP, then a placeholder), this method exists so that a caller's IP + * can be recorded alongside an authenticated identity rather than only + * as a fallback used when no identity is available — valuable for + * security-sensitive operations (login, password reset, admin actions) + * where the IP remains useful for audit and rate-limiting purposes even + * when the call also resolves to an authenticated user. + * + *

This is a default method, not an abstract one, so that adding it + * does not break existing implementations of this interface compiled + * against an earlier version of this library. The default returns + * {@code null}, meaning "this adapter does not support resolving an IP + * independent of caller identity" — consistent with this library's + * convention of using {@code null} for "not available" on structured + * fields, rather than a placeholder string. + * + * @return the caller's IP address, or {@code null} if it cannot be + * resolved (for example, no HTTP request is available on the + * current thread, or this adapter does not implement IP + * resolution) + */ + default String resolveCallerIp() { + return null; + } } \ No newline at end of file diff --git a/logged-core/src/test/java/com/fayupable/logged/core/port/NoOpAdaptersTest.java b/logged-core/src/test/java/com/fayupable/logged/core/port/NoOpAdaptersTest.java index a5ab734..5b73d73 100644 --- a/logged-core/src/test/java/com/fayupable/logged/core/port/NoOpAdaptersTest.java +++ b/logged-core/src/test/java/com/fayupable/logged/core/port/NoOpAdaptersTest.java @@ -17,7 +17,7 @@ class NoOpAdaptersTest { void noOpInvocationEventEmitterDoesNotThrow() { InvocationEventEmitter emitter = new NoOpInvocationEventEmitter(); MethodInvocationEvent event = new MethodInvocationEvent( - "SomeClass", "someMethod", Instant.now(), 1_000_000L, true, null, null, "unknown", "trace-1", 0 + "SomeClass", "someMethod", Instant.now(), 1_000_000L, true, null, null, "unknown", null, "trace-1", 0 ); assertThatCode(() -> emitter.emit(event)).doesNotThrowAnyException(); @@ -39,4 +39,20 @@ void noOpClientInfoPortReportsUnknownCaller() { assertThat(clientInfoPort.resolveCallerIdentity()).isEqualTo("unknown"); } + + @Test + @DisplayName("NoOpClientInfoPort inherits the default resolveCallerIp, reporting null") + void noOpClientInfoPortReportsNullIp() { + IClientInfoPort clientInfoPort = new NoOpClientInfoPort(); + + assertThat(clientInfoPort.resolveCallerIp()).isNull(); + } + + @Test + @DisplayName("IClientInfoPort's default resolveCallerIp returns null for any implementation that does not override it") + void interfaceDefaultResolveCallerIpReturnsNull() { + IClientInfoPort minimalImplementation = () -> "user:42"; + + assertThat(minimalImplementation.resolveCallerIp()).isNull(); + } } \ No newline at end of file diff --git a/logged-spring/pom.xml b/logged-spring/pom.xml index 9a1527b..6c8c51b 100644 --- a/logged-spring/pom.xml +++ b/logged-spring/pom.xml @@ -7,7 +7,7 @@ com.fayupable logged-lib - 1.2.2 + 1.3.0 logged-spring diff --git a/logged-spring/src/main/java/com/fayupable/logged/spring/aspect/LoggedAspect.java b/logged-spring/src/main/java/com/fayupable/logged/spring/aspect/LoggedAspect.java index 46b6f37..bdb5e42 100644 --- a/logged-spring/src/main/java/com/fayupable/logged/spring/aspect/LoggedAspect.java +++ b/logged-spring/src/main/java/com/fayupable/logged/spring/aspect/LoggedAspect.java @@ -183,11 +183,12 @@ public Object logInvocation(ProceedingJoinPoint pjp, Logged logged) throws Throw String className = names.className(); String methodName = names.methodName(); String callerIdentity = clientInfoPort.resolveCallerIdentity(); + String callerIp = logged.includeIp() ? clientInfoPort.resolveCallerIp() : null; long start = System.nanoTime(); FlowContextHolder.FlowScope flowScope = FlowContextHolder.enter(); Runnable restoreMdc = mdcEnabled - ? MdcPropagation.push(flowScope.context(), className, methodName) + ? MdcPropagation.push(flowScope.context(), className, methodName, callerIp) : NO_OP_MDC_RESTORE; try { Object result; @@ -197,18 +198,18 @@ public Object logInvocation(ProceedingJoinPoint pjp, Logged logged) throws Throw String exceptionType = t.getClass().getSimpleName(); String rootCauseType = resolveRootCauseType(t); long durationNanos = System.nanoTime() - start; - recordObservability(logged, className, methodName, callerIdentity, durationNanos, + recordObservability(logged, className, methodName, callerIdentity, callerIp, durationNanos, false, exceptionType, rootCauseType, flowScope.context()); throw t; } if (result instanceof CompletableFuture future) { FlowContext capturedContext = flowScope.context(); - return instrumentCompletableFuture(future, logged, className, methodName, callerIdentity, start, capturedContext); + return instrumentCompletableFuture(future, logged, className, methodName, callerIdentity, callerIp, start, capturedContext); } long durationNanos = System.nanoTime() - start; - recordObservability(logged, className, methodName, callerIdentity, durationNanos, + recordObservability(logged, className, methodName, callerIdentity, callerIp, durationNanos, true, null, null, flowScope.context()); return result; } finally { @@ -245,6 +246,11 @@ public Object logInvocation(ProceedingJoinPoint pjp, Logged logged) throws Throw * @param methodName the method name resolved for this invocation * @param callerIdentity the caller identity resolved synchronously on * the original calling thread + * @param callerIp the caller's IP address resolved synchronously + * on the original calling thread, for the same + * reason as {@code callerIdentity} above; or + * {@code null} if {@link Logged#includeIp()} is + * {@code false} for this invocation * @param start the {@link System#nanoTime()} reading taken * when this invocation began * @param capturedContext the call-chain position this invocation was @@ -255,7 +261,7 @@ public Object logInvocation(ProceedingJoinPoint pjp, Logged logged) throws Throw */ private CompletableFuture instrumentCompletableFuture(CompletableFuture future, Logged logged, String className, String methodName, String callerIdentity, - long start, FlowContext capturedContext) { + String callerIp, long start, FlowContext capturedContext) { return future.whenComplete((value, throwable) -> { long durationNanos = System.nanoTime() - start; boolean success = throwable == null; @@ -268,7 +274,7 @@ private CompletableFuture instrumentCompletableFuture(CompletableFuture fu rootCauseType = resolveRootCauseType(unwrapped); } - recordObservability(logged, className, methodName, callerIdentity, durationNanos, + recordObservability(logged, className, methodName, callerIdentity, callerIp, durationNanos, success, exceptionType, rootCauseType, capturedContext); }); } @@ -341,24 +347,24 @@ private String resolveRootCauseType(Throwable thrown) { * #metricsRecorder} and, if {@link EmissionPolicy} selects it, through * {@link #eventEmitter}. * - *

Takes an already-resolved {@code callerIdentity} and {@link - * FlowContext} rather than resolving them itself, so this method works - * identically whether it is called synchronously, right after {@link - * ProceedingJoinPoint#proceed()} returns, or later from a {@link - * CompletableFuture} completion callback running on a different thread: - * neither value depends on {@link ThreadLocal} state at the point this - * method runs. + *

Takes an already-resolved {@code callerIdentity}, {@code callerIp}, + * and {@link FlowContext} rather than resolving them itself, so this + * method works identically whether it is called synchronously, right + * after {@link ProceedingJoinPoint#proceed()} returns, or later from a + * {@link CompletableFuture} completion callback running on a different + * thread: none of these values depend on {@link ThreadLocal} state at + * the point this method runs. */ private void recordObservability(Logged logged, String className, String methodName, String callerIdentity, - long durationNanos, boolean success, String exceptionType, String rootCauseType, - FlowContext context) { + String callerIp, long durationNanos, boolean success, String exceptionType, + String rootCauseType, FlowContext context) { try { metricsRecorder.record(className, methodName, durationNanos, success, exceptionType); if (EmissionPolicy.shouldEmit(logged, durationNanos, success)) { eventEmitter.emit(new MethodInvocationEvent( className, methodName, Instant.now(), durationNanos, success, exceptionType, rootCauseType, - callerIdentity, context.traceId(), context.depth() + callerIdentity, callerIp, context.traceId(), context.depth() )); } } catch (RuntimeException e) { diff --git a/logged-spring/src/main/java/com/fayupable/logged/spring/aspect/LoggedMdcKeys.java b/logged-spring/src/main/java/com/fayupable/logged/spring/aspect/LoggedMdcKeys.java index edfd07d..3a8ea33 100644 --- a/logged-spring/src/main/java/com/fayupable/logged/spring/aspect/LoggedMdcKeys.java +++ b/logged-spring/src/main/java/com/fayupable/logged/spring/aspect/LoggedMdcKeys.java @@ -48,6 +48,14 @@ public final class LoggedMdcKeys { */ public static final String METHOD_NAME = "logged.methodName"; + /** + * The caller's IP address, the same value reported as + * {@link com.fayupable.logged.core.model.MethodInvocationEvent#callerIp()}. + * Only written while executing a method annotated with + * {@code @Logged(includeIp = true)}; absent for every other invocation. + */ + public static final String CALLER_IP = "logged.callerIp"; + private LoggedMdcKeys() { } } diff --git a/logged-spring/src/main/java/com/fayupable/logged/spring/aspect/MdcPropagation.java b/logged-spring/src/main/java/com/fayupable/logged/spring/aspect/MdcPropagation.java index 3f9bff2..da046cf 100644 --- a/logged-spring/src/main/java/com/fayupable/logged/spring/aspect/MdcPropagation.java +++ b/logged-spring/src/main/java/com/fayupable/logged/spring/aspect/MdcPropagation.java @@ -25,34 +25,45 @@ private MdcPropagation() { } /** - * Writes {@code context}, {@code className}, and {@code methodName} into - * the current thread's MDC under {@link LoggedMdcKeys}, and returns a - * {@link Runnable} that restores whatever value each key held before - * this call, once run. + * Writes {@code context}, {@code className}, {@code methodName}, and, + * when present, {@code callerIp} into the current thread's MDC under + * {@link LoggedMdcKeys}, and returns a {@link Runnable} that restores + * whatever value each key held before this call, once run. * * @param context the call-chain position of the invocation currently * starting * @param className the class name resolved for this invocation * @param methodName the method name resolved for this invocation + * @param callerIp the caller's IP address resolved for this + * invocation, or {@code null} if + * {@code @Logged(includeIp = true)} was not set on the + * invoked method; when {@code null}, {@link LoggedMdcKeys#CALLER_IP} + * is left untouched rather than written as an empty + * or missing value * @return a {@link Runnable} that undoes this write; must be run once * the invocation ends, typically in a {@code finally} block */ - static Runnable push(FlowContext context, String className, String methodName) { + static Runnable push(FlowContext context, String className, String methodName, String callerIp) { String previousTraceId = MDC.get(LoggedMdcKeys.TRACE_ID); String previousDepth = MDC.get(LoggedMdcKeys.DEPTH); String previousClassName = MDC.get(LoggedMdcKeys.CLASS_NAME); String previousMethodName = MDC.get(LoggedMdcKeys.METHOD_NAME); + String previousCallerIp = MDC.get(LoggedMdcKeys.CALLER_IP); MDC.put(LoggedMdcKeys.TRACE_ID, context.traceId()); MDC.put(LoggedMdcKeys.DEPTH, String.valueOf(context.depth())); MDC.put(LoggedMdcKeys.CLASS_NAME, className); MDC.put(LoggedMdcKeys.METHOD_NAME, methodName); + if (callerIp != null) { + MDC.put(LoggedMdcKeys.CALLER_IP, callerIp); + } return () -> { restore(LoggedMdcKeys.TRACE_ID, previousTraceId); restore(LoggedMdcKeys.DEPTH, previousDepth); restore(LoggedMdcKeys.CLASS_NAME, previousClassName); restore(LoggedMdcKeys.METHOD_NAME, previousMethodName); + restore(LoggedMdcKeys.CALLER_IP, previousCallerIp); }; } diff --git a/logged-spring/src/main/java/com/fayupable/logged/spring/emitter/JsonInvocationEventEmitter.java b/logged-spring/src/main/java/com/fayupable/logged/spring/emitter/JsonInvocationEventEmitter.java index 6241390..64b3da0 100644 --- a/logged-spring/src/main/java/com/fayupable/logged/spring/emitter/JsonInvocationEventEmitter.java +++ b/logged-spring/src/main/java/com/fayupable/logged/spring/emitter/JsonInvocationEventEmitter.java @@ -68,6 +68,8 @@ private static String toJson(MethodInvocationEvent event) { json.append(','); appendString(json, "callerIdentity", event.callerIdentity()); json.append(','); + appendString(json, "callerIp", event.callerIp()); + json.append(','); appendString(json, "traceId", event.traceId()); json.append(','); appendNumber(json, "depth", event.depth()); diff --git a/logged-spring/src/main/java/com/fayupable/logged/spring/emitter/Slf4jInvocationEventEmitter.java b/logged-spring/src/main/java/com/fayupable/logged/spring/emitter/Slf4jInvocationEventEmitter.java index 6f24f00..0b3f26d 100644 --- a/logged-spring/src/main/java/com/fayupable/logged/spring/emitter/Slf4jInvocationEventEmitter.java +++ b/logged-spring/src/main/java/com/fayupable/logged/spring/emitter/Slf4jInvocationEventEmitter.java @@ -26,6 +26,13 @@ * that is, when the thrown exception actually wraps a different underlying * cause. When the two are equal, showing the root cause a second time would * add nothing, so it is omitted to keep the common case concise. + * + *

{@link MethodInvocationEvent#callerIp()} is only appended when present, + * that is, only for the security-sensitive methods explicitly annotated + * with {@code @Logged(includeIp = true)}. It is deliberately shown alongside + * {@link MethodInvocationEvent#callerIdentity()} rather than only in its + * place, since the two answer different questions ("who" versus "from + * where") that both matter for the operations this attribute is meant for. */ public class Slf4jInvocationEventEmitter implements InvocationEventEmitter { @@ -34,19 +41,20 @@ public class Slf4jInvocationEventEmitter implements InvocationEventEmitter { @Override public void emit(MethodInvocationEvent event) { long durationMs = event.durationNanos() / 1_000_000; + String ipSuffix = event.callerIp() != null ? " ip=" + event.callerIp() : ""; String chainSuffix = event.depth() > 0 ? " (trace=" + event.traceId() + ", depth=" + event.depth() + ")" : ""; if (event.success()) { - log.info("[{}] {}.{} completed in {}ms{}", - event.callerIdentity(), event.className(), event.methodName(), durationMs, chainSuffix); + log.info("[{}]{} {}.{} completed in {}ms{}", + event.callerIdentity(), ipSuffix, event.className(), event.methodName(), durationMs, chainSuffix); } else { boolean rootCauseDiffers = event.rootCauseType() != null && !event.rootCauseType().equals(event.exceptionType()); String causeSuffix = rootCauseDiffers ? " (caused by " + event.rootCauseType() + ")" : ""; - log.warn("[{}] {}.{} failed after {}ms - {}{}{}", - event.callerIdentity(), event.className(), event.methodName(), durationMs, + log.warn("[{}]{} {}.{} failed after {}ms - {}{}{}", + event.callerIdentity(), ipSuffix, event.className(), event.methodName(), durationMs, event.exceptionType(), causeSuffix, chainSuffix); } } diff --git a/logged-spring/src/main/java/com/fayupable/logged/spring/security/HttpRequestClientInfoAdapter.java b/logged-spring/src/main/java/com/fayupable/logged/spring/security/HttpRequestClientInfoAdapter.java index 5519577..9208e94 100644 --- a/logged-spring/src/main/java/com/fayupable/logged/spring/security/HttpRequestClientInfoAdapter.java +++ b/logged-spring/src/main/java/com/fayupable/logged/spring/security/HttpRequestClientInfoAdapter.java @@ -36,4 +36,9 @@ public String resolveCallerIdentity() { String clientIp = RequestClientIpResolver.resolveClientIp(trustForwardedHeaders); return clientIp != null ? clientIp : UNKNOWN_CALLER; } + + @Override + public String resolveCallerIp() { + return RequestClientIpResolver.resolveIp(trustForwardedHeaders); + } } diff --git a/logged-spring/src/main/java/com/fayupable/logged/spring/security/RequestClientIpResolver.java b/logged-spring/src/main/java/com/fayupable/logged/spring/security/RequestClientIpResolver.java index 594fab8..175a03c 100644 --- a/logged-spring/src/main/java/com/fayupable/logged/spring/security/RequestClientIpResolver.java +++ b/logged-spring/src/main/java/com/fayupable/logged/spring/security/RequestClientIpResolver.java @@ -30,7 +30,9 @@ private RequestClientIpResolver() { } /** - * Resolves the current request's client IP address. + * Resolves the current request's client IP address, prefixed for use as + * a {@link com.fayupable.logged.core.port.IClientInfoPort#resolveCallerIdentity()} + * fallback tier. * * @param trustForwardedHeaders whether the {@code X-Forwarded-For} * header may be trusted over @@ -40,6 +42,23 @@ private RequestClientIpResolver() { * thread (for example, a scheduled job) */ static String resolveClientIp(boolean trustForwardedHeaders) { + String ip = resolveIp(trustForwardedHeaders); + return ip != null ? "ip:" + ip : null; + } + + /** + * Resolves the current request's client IP address, unprefixed, for use + * as a {@link com.fayupable.logged.core.port.IClientInfoPort#resolveCallerIp()} + * dedicated field rather than as part of the caller-identity string. + * + * @param trustForwardedHeaders whether the {@code X-Forwarded-For} + * header may be trusted over + * {@link HttpServletRequest#getRemoteAddr()} + * @return the caller's raw IP address, or {@code null} if there is no + * active HTTP request on this thread (for example, a scheduled + * job) + */ + static String resolveIp(boolean trustForwardedHeaders) { HttpServletRequest request = currentRequest(); if (request == null) { return null; @@ -48,11 +67,11 @@ static String resolveClientIp(boolean trustForwardedHeaders) { if (trustForwardedHeaders) { String forwardedFor = request.getHeader(FORWARDED_FOR_HEADER); if (forwardedFor != null && !forwardedFor.isBlank()) { - return "ip:" + forwardedFor.split(",")[0].trim(); + return forwardedFor.split(",")[0].trim(); } } - return "ip:" + request.getRemoteAddr(); + return request.getRemoteAddr(); } private static HttpServletRequest currentRequest() { diff --git a/logged-spring/src/main/java/com/fayupable/logged/spring/security/SpringSecurityClientInfoAdapter.java b/logged-spring/src/main/java/com/fayupable/logged/spring/security/SpringSecurityClientInfoAdapter.java index 7da81c3..8826615 100644 --- a/logged-spring/src/main/java/com/fayupable/logged/spring/security/SpringSecurityClientInfoAdapter.java +++ b/logged-spring/src/main/java/com/fayupable/logged/spring/security/SpringSecurityClientInfoAdapter.java @@ -56,4 +56,16 @@ private String resolveAuthenticatedUser() { private boolean isAnonymous(Authentication authentication) { return ANONYMOUS_PRINCIPAL.equals(authentication.getPrincipal()); } + + /** + * Resolves the caller's IP address independent of {@link #resolveCallerIdentity()}'s + * outcome, unlike that method's authenticated-principal-first fallback + * chain. This has no dependency on {@link SecurityContextHolder} state, + * so it resolves the same way whether or not the current invocation + * also resolves to an authenticated user. + */ + @Override + public String resolveCallerIp() { + return RequestClientIpResolver.resolveIp(trustForwardedHeaders); + } } diff --git a/logged-spring/src/test/java/com/fayupable/logged/spring/aspect/LoggedAspectTest.java b/logged-spring/src/test/java/com/fayupable/logged/spring/aspect/LoggedAspectTest.java index 7cdb6df..fbc6b84 100644 --- a/logged-spring/src/test/java/com/fayupable/logged/spring/aspect/LoggedAspectTest.java +++ b/logged-spring/src/test/java/com/fayupable/logged/spring/aspect/LoggedAspectTest.java @@ -49,8 +49,12 @@ void tearDown() { } private T proxy(T target) { + return proxyWith(aspect, target); + } + + private T proxyWith(LoggedAspect aspectInstance, T target) { AspectJProxyFactory factory = new AspectJProxyFactory(target); - factory.addAspect(aspect); + factory.addAspect(aspectInstance); return factory.getProxy(); } @@ -564,6 +568,68 @@ void doesNotTouchMdcWhenDisabled() { } } + @Nested + @DisplayName("Logged#includeIp") + class IncludeIp { + + private static final String CALLER_IP = "203.0.113.5"; + + @Test + @DisplayName("populates callerIp on a successful call when includeIp = true") + void populatesCallerIpOnSuccessWhenEnabled() { + CountingClientInfoPort clientInfoPort = new CountingClientInfoPort(CALLER_IDENTITY, CALLER_IP); + LoggedAspect ipAwareAspect = new LoggedAspect(eventEmitter, metricsRecorder, clientInfoPort); + SecuritySensitiveService service = proxyWith(ipAwareAspect, new SecuritySensitiveServiceImpl()); + + service.login("enis"); + + assertThat(eventEmitter.events.get(0).callerIp()).isEqualTo(CALLER_IP); + assertThat(clientInfoPort.resolveCallerIpInvocations).isEqualTo(1); + } + + @Test + @DisplayName("leaves callerIp null when includeIp = false (the default), without calling resolveCallerIp") + void leavesCallerIpNullWhenDisabled() { + CountingClientInfoPort clientInfoPort = new CountingClientInfoPort(CALLER_IDENTITY, CALLER_IP); + LoggedAspect ipAwareAspect = new LoggedAspect(eventEmitter, metricsRecorder, clientInfoPort); + SecuritySensitiveService service = proxyWith(ipAwareAspect, new SecuritySensitiveServiceImpl()); + + service.readData(); + + assertThat(eventEmitter.events.get(0).callerIp()).isNull(); + assertThat(clientInfoPort.resolveCallerIpInvocations).isZero(); + } + + @Test + @DisplayName("populates callerIp on a failed call when includeIp = true") + void populatesCallerIpOnFailureWhenEnabled() { + CountingClientInfoPort clientInfoPort = new CountingClientInfoPort(CALLER_IDENTITY, CALLER_IP); + LoggedAspect ipAwareAspect = new LoggedAspect(eventEmitter, metricsRecorder, clientInfoPort); + SecuritySensitiveService service = proxyWith(ipAwareAspect, new SecuritySensitiveServiceImpl()); + + assertThatThrownBy(() -> service.login("invalid")).isInstanceOf(IllegalArgumentException.class); + + assertThat(eventEmitter.events.get(0).success()).isFalse(); + assertThat(eventEmitter.events.get(0).callerIp()).isEqualTo(CALLER_IP); + } + + @Test + @DisplayName("populates callerIp once a CompletableFuture-returning call actually completes") + void populatesCallerIpAfterFutureCompletion() { + CountingClientInfoPort clientInfoPort = new CountingClientInfoPort(CALLER_IDENTITY, CALLER_IP); + LoggedAspect ipAwareAspect = new LoggedAspect(eventEmitter, metricsRecorder, clientInfoPort); + SecuritySensitiveService service = proxyWith(ipAwareAspect, new SecuritySensitiveServiceImpl()); + CompletableFuture manuallyCompleted = new CompletableFuture<>(); + + service.loginAsync(manuallyCompleted); + assertThat(eventEmitter.events).isEmpty(); + + manuallyCompleted.complete("ok"); + + assertThat(eventEmitter.events.get(0).callerIp()).isEqualTo(CALLER_IP); + } + } + @Nested @DisplayName("robustness against misbehaving observability collaborators") class Robustness { @@ -711,6 +777,59 @@ public int divide(int a, int b) { } } + interface SecuritySensitiveService { + String login(String username); + + String readData(); + + CompletableFuture loginAsync(CompletableFuture future); + } + + static class SecuritySensitiveServiceImpl implements SecuritySensitiveService { + @Logged(includeIp = true) + @Override + public String login(String username) { + if ("invalid".equals(username)) { + throw new IllegalArgumentException("bad credentials"); + } + return "ok:" + username; + } + + @Logged + @Override + public String readData() { + return "data"; + } + + @Logged(includeIp = true) + @Override + public CompletableFuture loginAsync(CompletableFuture future) { + return future; + } + } + + static class CountingClientInfoPort implements IClientInfoPort { + private final String identity; + private final String ip; + int resolveCallerIpInvocations; + + CountingClientInfoPort(String identity, String ip) { + this.identity = identity; + this.ip = ip; + } + + @Override + public String resolveCallerIdentity() { + return identity; + } + + @Override + public String resolveCallerIp() { + resolveCallerIpInvocations++; + return ip; + } + } + static class SlowCalculatorImpl implements Calculator { @Logged(slowThresholdMs = 0, sampleRate = 0.0) @Override @@ -892,7 +1011,7 @@ static class RecordingMetricsRecorder implements MetricsRecorder { @Override public void record(String className, String methodName, long durationNanos, boolean success, String exceptionType) { MethodInvocationEvent recorded = new MethodInvocationEvent( - className, methodName, java.time.Instant.now(), durationNanos, success, exceptionType, exceptionType, "n/a", "n/a", -1 + className, methodName, java.time.Instant.now(), durationNanos, success, exceptionType, exceptionType, "n/a", null, "n/a", -1 ); recordings.add(recorded); System.out.println("METRIC -> " + recorded); diff --git a/logged-spring/src/test/java/com/fayupable/logged/spring/aspect/MdcPropagationTest.java b/logged-spring/src/test/java/com/fayupable/logged/spring/aspect/MdcPropagationTest.java index 3188c4e..17258de 100644 --- a/logged-spring/src/test/java/com/fayupable/logged/spring/aspect/MdcPropagationTest.java +++ b/logged-spring/src/test/java/com/fayupable/logged/spring/aspect/MdcPropagationTest.java @@ -26,7 +26,7 @@ class Push { void writesAllKeys() { FlowContext context = new FlowContext("trace-abc", 2); - MdcPropagation.push(context, "OrderService", "placeOrder"); + MdcPropagation.push(context, "OrderService", "placeOrder", null); assertThat(MDC.get(LoggedMdcKeys.TRACE_ID)).isEqualTo("trace-abc"); assertThat(MDC.get(LoggedMdcKeys.DEPTH)).isEqualTo("2"); @@ -44,7 +44,7 @@ class Restore { void removesKeysWhenNothingWasActiveBefore() { FlowContext context = new FlowContext("trace-abc", 0); - Runnable restore = MdcPropagation.push(context, "OrderService", "placeOrder"); + Runnable restore = MdcPropagation.push(context, "OrderService", "placeOrder", null); restore.run(); assertThat(MDC.get(LoggedMdcKeys.TRACE_ID)).isNull(); @@ -57,10 +57,10 @@ void removesKeysWhenNothingWasActiveBefore() { @DisplayName("restores the outer call's values once a nested call's push() is undone") void restoresOuterValuesAfterNestedCall() { FlowContext outerContext = new FlowContext("outer-trace", 0); - Runnable restoreOuter = MdcPropagation.push(outerContext, "ServiceA", "process"); + Runnable restoreOuter = MdcPropagation.push(outerContext, "ServiceA", "process", null); FlowContext innerContext = new FlowContext("outer-trace", 1); - Runnable restoreInner = MdcPropagation.push(innerContext, "ServiceB", "doWork"); + Runnable restoreInner = MdcPropagation.push(innerContext, "ServiceB", "doWork", null); restoreInner.run(); assertThat(MDC.get(LoggedMdcKeys.DEPTH)).isEqualTo("0"); @@ -77,10 +77,62 @@ void doesNotDisturbUnrelatedKeys() { MDC.put("requestId", "req-123"); FlowContext context = new FlowContext("trace-abc", 0); - Runnable restore = MdcPropagation.push(context, "OrderService", "placeOrder"); + Runnable restore = MdcPropagation.push(context, "OrderService", "placeOrder", null); restore.run(); assertThat(MDC.get("requestId")).isEqualTo("req-123"); } } + + @Nested + @DisplayName("callerIp") + class CallerIp { + + @Test + @DisplayName("writes CALLER_IP when present") + void writesCallerIpWhenPresent() { + FlowContext context = new FlowContext("trace-abc", 0); + + MdcPropagation.push(context, "OrderService", "placeOrder", "203.0.113.5"); + + assertThat(MDC.get(LoggedMdcKeys.CALLER_IP)).isEqualTo("203.0.113.5"); + } + + @Test + @DisplayName("leaves CALLER_IP untouched when null") + void leavesCallerIpUntouchedWhenNull() { + FlowContext context = new FlowContext("trace-abc", 0); + + MdcPropagation.push(context, "OrderService", "placeOrder", null); + + assertThat(MDC.get(LoggedMdcKeys.CALLER_IP)).isNull(); + } + + @Test + @DisplayName("restores CALLER_IP once push() is undone") + void restoresCallerIpAfterUndo() { + FlowContext context = new FlowContext("trace-abc", 0); + + Runnable restore = MdcPropagation.push(context, "OrderService", "placeOrder", "203.0.113.5"); + restore.run(); + + assertThat(MDC.get(LoggedMdcKeys.CALLER_IP)).isNull(); + } + + @Test + @DisplayName("restores the outer call's IP once a nested call's push() is undone") + void restoresOuterCallerIpAfterNestedCall() { + FlowContext outerContext = new FlowContext("outer-trace", 0); + Runnable restoreOuter = MdcPropagation.push(outerContext, "ServiceA", "process", "198.51.100.1"); + + FlowContext innerContext = new FlowContext("outer-trace", 1); + Runnable restoreInner = MdcPropagation.push(innerContext, "ServiceB", "doWork", "203.0.113.5"); + restoreInner.run(); + + assertThat(MDC.get(LoggedMdcKeys.CALLER_IP)).isEqualTo("198.51.100.1"); + + restoreOuter.run(); + assertThat(MDC.get(LoggedMdcKeys.CALLER_IP)).isNull(); + } + } } diff --git a/logged-spring/src/test/java/com/fayupable/logged/spring/emitter/JsonInvocationEventEmitterTest.java b/logged-spring/src/test/java/com/fayupable/logged/spring/emitter/JsonInvocationEventEmitterTest.java index 83f4da3..ba35172 100644 --- a/logged-spring/src/test/java/com/fayupable/logged/spring/emitter/JsonInvocationEventEmitterTest.java +++ b/logged-spring/src/test/java/com/fayupable/logged/spring/emitter/JsonInvocationEventEmitterTest.java @@ -43,14 +43,14 @@ void tearDown() { private MethodInvocationEvent successEvent() { return new MethodInvocationEvent( "UserService", "getUser", Instant.parse("2026-01-01T00:00:00Z"), 12_000_000L, true, null, null, - CALLER_IDENTITY, "abc123", 2 + CALLER_IDENTITY, null, "abc123", 2 ); } private MethodInvocationEvent failureEvent(String exceptionType, String rootCauseType) { return new MethodInvocationEvent( "UserService", "getUser", Instant.parse("2026-01-01T00:00:00Z"), 5_000_000L, false, exceptionType, - rootCauseType, CALLER_IDENTITY, "abc123", 0 + rootCauseType, CALLER_IDENTITY, null, "abc123", 0 ); } @@ -90,6 +90,7 @@ void writesEveryFieldAsJsonProperty() { .contains("\"exceptionType\":null") .contains("\"rootCauseType\":null") .contains("\"callerIdentity\":\"user:42\"") + .contains("\"callerIp\":null") .contains("\"traceId\":\"abc123\"") .contains("\"depth\":2"); } @@ -129,7 +130,7 @@ class StringEscaping { void escapesSpecialCharacters() { MethodInvocationEvent event = new MethodInvocationEvent( "Weird\"Class\\Name", "method\nWithNewline", Instant.parse("2026-01-01T00:00:00Z"), - 1_000_000L, true, null, null, CALLER_IDENTITY, "abc123", 0 + 1_000_000L, true, null, null, CALLER_IDENTITY, null, "abc123", 0 ); emitter.emit(event); @@ -147,7 +148,7 @@ void escapesOtherControlCharacters() { String methodNameWithControlChar = "method" + '' + "WithControlChar"; MethodInvocationEvent event = new MethodInvocationEvent( "UserService", methodNameWithControlChar, Instant.parse("2026-01-01T00:00:00Z"), - 1_000_000L, true, null, null, CALLER_IDENTITY, "abc123", 0 + 1_000_000L, true, null, null, CALLER_IDENTITY, null, "abc123", 0 ); emitter.emit(event); @@ -162,7 +163,7 @@ void escapesOtherControlCharacters() { void doesNotEscapeSpace() { MethodInvocationEvent event = new MethodInvocationEvent( "UserService", "method with space", Instant.parse("2026-01-01T00:00:00Z"), - 1_000_000L, true, null, null, CALLER_IDENTITY, "abc123", 0 + 1_000_000L, true, null, null, CALLER_IDENTITY, null, "abc123", 0 ); emitter.emit(event); @@ -179,7 +180,7 @@ void doesNotEscapeSpace() { void producesSingleLogLine() { MethodInvocationEvent event = new MethodInvocationEvent( "UserService", "method\nWithNewline", Instant.parse("2026-01-01T00:00:00Z"), - 1_000_000L, true, null, null, CALLER_IDENTITY, "abc123", 0 + 1_000_000L, true, null, null, CALLER_IDENTITY, null, "abc123", 0 ); emitter.emit(event); @@ -189,4 +190,24 @@ void producesSingleLogLine() { assertThat(message).doesNotContain("\n"); } } + + @Nested + @DisplayName("callerIp") + class CallerIp { + + @Test + @DisplayName("writes the IP as its own field when present") + void writesIpWhenPresent() { + MethodInvocationEvent event = new MethodInvocationEvent( + "UserService", "getUser", Instant.parse("2026-01-01T00:00:00Z"), 12_000_000L, true, null, null, + CALLER_IDENTITY, "203.0.113.5", "abc123", 0 + ); + + emitter.emit(event); + + String message = printedMessage(); + + assertThat(message).contains("\"callerIp\":\"203.0.113.5\""); + } + } } diff --git a/logged-spring/src/test/java/com/fayupable/logged/spring/emitter/Slf4jInvocationEventEmitterTest.java b/logged-spring/src/test/java/com/fayupable/logged/spring/emitter/Slf4jInvocationEventEmitterTest.java index 57d434e..309f6d6 100644 --- a/logged-spring/src/test/java/com/fayupable/logged/spring/emitter/Slf4jInvocationEventEmitterTest.java +++ b/logged-spring/src/test/java/com/fayupable/logged/spring/emitter/Slf4jInvocationEventEmitterTest.java @@ -42,7 +42,7 @@ void tearDown() { private MethodInvocationEvent successEvent(int depth) { return new MethodInvocationEvent( - "UserService", "getUser", Instant.now(), 12_000_000L, true, null, null, CALLER_IDENTITY, "abc123", depth + "UserService", "getUser", Instant.now(), 12_000_000L, true, null, null, CALLER_IDENTITY, null, "abc123", depth ); } @@ -53,7 +53,13 @@ private MethodInvocationEvent failureEvent(int depth) { private MethodInvocationEvent failureEvent(int depth, String rootCauseType) { return new MethodInvocationEvent( "UserService", "getUser", Instant.now(), 5_000_000L, false, "IllegalArgumentException", rootCauseType, - CALLER_IDENTITY, "abc123", depth + CALLER_IDENTITY, null, "abc123", depth + ); + } + + private MethodInvocationEvent successEventWithIp(String callerIp) { + return new MethodInvocationEvent( + "UserService", "getUser", Instant.now(), 12_000_000L, true, null, null, CALLER_IDENTITY, callerIp, "abc123", 0 ); } @@ -161,4 +167,29 @@ void includesRootCauseWhenItDiffers() { .contains("caused by SQLException"); } } + + @Nested + @DisplayName("callerIp") + class CallerIp { + + @Test + @DisplayName("includes the IP when present") + void includesIpWhenPresent() { + emitter.emit(successEventWithIp("203.0.113.5")); + + String message = printedMessage(); + + assertThat(message).contains("ip=203.0.113.5"); + } + + @Test + @DisplayName("omits any IP marker when not present") + void omitsIpWhenAbsent() { + emitter.emit(successEventWithIp(null)); + + String message = printedMessage(); + + assertThat(message).doesNotContain("ip="); + } + } } diff --git a/logged-spring/src/test/java/com/fayupable/logged/spring/security/HttpRequestClientInfoAdapterTest.java b/logged-spring/src/test/java/com/fayupable/logged/spring/security/HttpRequestClientInfoAdapterTest.java index 434fd52..cfad072 100644 --- a/logged-spring/src/test/java/com/fayupable/logged/spring/security/HttpRequestClientInfoAdapterTest.java +++ b/logged-spring/src/test/java/com/fayupable/logged/spring/security/HttpRequestClientInfoAdapterTest.java @@ -64,4 +64,27 @@ void usesForwardedForHeader() { assertThat(adapter.resolveCallerIdentity()).isEqualTo("ip:198.51.100.99"); } } + + @Nested + @DisplayName("resolveCallerIp") + class ResolveCallerIp { + + @Test + @DisplayName("resolves the raw IP address, unprefixed") + void resolvesRawIp() { + withRequestFrom("203.0.113.5"); + + HttpRequestClientInfoAdapter adapter = new HttpRequestClientInfoAdapter(false); + + assertThat(adapter.resolveCallerIp()).isEqualTo("203.0.113.5"); + } + + @Test + @DisplayName("resolves to null when there is no active HTTP request") + void resolvesNullWithoutRequest() { + HttpRequestClientInfoAdapter adapter = new HttpRequestClientInfoAdapter(false); + + assertThat(adapter.resolveCallerIp()).isNull(); + } + } } diff --git a/logged-spring/src/test/java/com/fayupable/logged/spring/security/SpringSecurityClientInfoAdapterTest.java b/logged-spring/src/test/java/com/fayupable/logged/spring/security/SpringSecurityClientInfoAdapterTest.java index f055201..2ebf7b8 100644 --- a/logged-spring/src/test/java/com/fayupable/logged/spring/security/SpringSecurityClientInfoAdapterTest.java +++ b/logged-spring/src/test/java/com/fayupable/logged/spring/security/SpringSecurityClientInfoAdapterTest.java @@ -169,4 +169,32 @@ void fallsBackToRemoteAddressWithoutHeader() { assertThat(trustingAdapter.resolveCallerIdentity()).isEqualTo("ip:203.0.113.5"); } } + + @Nested + @DisplayName("resolveCallerIp") + class ResolveCallerIp { + + @Test + @DisplayName("resolves the raw IP address, unprefixed, even without authentication") + void resolvesRawIpWithoutAuthentication() { + withRequestFrom("203.0.113.5"); + + assertThat(adapter.resolveCallerIp()).isEqualTo("203.0.113.5"); + } + + @Test + @DisplayName("resolves the raw IP address even when the caller also has an authenticated identity") + void resolvesRawIpAlongsideAuthenticatedIdentity() { + authenticateAs("enis"); + withRequestFrom("203.0.113.5"); + + assertThat(adapter.resolveCallerIp()).isEqualTo("203.0.113.5"); + } + + @Test + @DisplayName("resolves to null when there is no active HTTP request") + void resolvesNullWithoutRequest() { + assertThat(adapter.resolveCallerIp()).isNull(); + } + } } \ No newline at end of file diff --git a/logged-test/pom.xml b/logged-test/pom.xml index ad1b4a3..6d2d0c4 100644 --- a/logged-test/pom.xml +++ b/logged-test/pom.xml @@ -7,7 +7,7 @@ com.fayupable logged-lib - 1.2.2 + 1.3.0 logged-test diff --git a/logged-test/src/main/java/com/fayupable/logged/test/InMemoryClientInfoPort.java b/logged-test/src/main/java/com/fayupable/logged/test/InMemoryClientInfoPort.java index a82a068..a24d957 100644 --- a/logged-test/src/main/java/com/fayupable/logged/test/InMemoryClientInfoPort.java +++ b/logged-test/src/main/java/com/fayupable/logged/test/InMemoryClientInfoPort.java @@ -17,6 +17,7 @@ public final class InMemoryClientInfoPort implements IClientInfoPort { private static final String DEFAULT_CALLER_IDENTITY = "unknown"; private volatile String callerIdentity; + private volatile String callerIp; /** * Creates an instance that reports the caller identity as @@ -49,4 +50,22 @@ public String resolveCallerIdentity() { public void setCallerIdentity(String callerIdentity) { this.callerIdentity = callerIdentity; } + + @Override + public String resolveCallerIp() { + return callerIp; + } + + /** + * Changes the IP address reported by subsequent calls to + * {@link #resolveCallerIp()}, for testing a {@code @Logged(includeIp = true)} + * method's {@link com.fayupable.logged.core.model.MethodInvocationEvent#callerIp()} + * without wiring a real HTTP request. Defaults to {@code null} + * (not resolved) until this is called. + * + * @param callerIp the IP address to report from now on + */ + public void setCallerIp(String callerIp) { + this.callerIp = callerIp; + } } diff --git a/logged-test/src/test/java/com/fayupable/logged/test/InMemoryClientInfoPortTest.java b/logged-test/src/test/java/com/fayupable/logged/test/InMemoryClientInfoPortTest.java index bc885a5..86c1173 100644 --- a/logged-test/src/test/java/com/fayupable/logged/test/InMemoryClientInfoPortTest.java +++ b/logged-test/src/test/java/com/fayupable/logged/test/InMemoryClientInfoPortTest.java @@ -49,4 +49,32 @@ void changesReportedIdentity() { assertThat(clientInfoPort.resolveCallerIdentity()).isEqualTo("user:99"); } } + + @Nested + @DisplayName("resolveCallerIp") + class ResolveCallerIp { + + @Test + @DisplayName("reports null until setCallerIp is called") + void reportsNullByDefault() { + InMemoryClientInfoPort clientInfoPort = new InMemoryClientInfoPort(); + + assertThat(clientInfoPort.resolveCallerIp()).isNull(); + } + } + + @Nested + @DisplayName("setCallerIp") + class SetCallerIp { + + @Test + @DisplayName("changes the IP reported by subsequent calls") + void changesReportedIp() { + InMemoryClientInfoPort clientInfoPort = new InMemoryClientInfoPort(); + + clientInfoPort.setCallerIp("203.0.113.5"); + + assertThat(clientInfoPort.resolveCallerIp()).isEqualTo("203.0.113.5"); + } + } } diff --git a/logged-test/src/test/java/com/fayupable/logged/test/InMemoryInvocationEventEmitterTest.java b/logged-test/src/test/java/com/fayupable/logged/test/InMemoryInvocationEventEmitterTest.java index 7365701..b3c4715 100644 --- a/logged-test/src/test/java/com/fayupable/logged/test/InMemoryInvocationEventEmitterTest.java +++ b/logged-test/src/test/java/com/fayupable/logged/test/InMemoryInvocationEventEmitterTest.java @@ -15,7 +15,7 @@ class InMemoryInvocationEventEmitterTest { private static MethodInvocationEvent event(String methodName) { return new MethodInvocationEvent( - "SomeClass", methodName, Instant.now(), 1_000_000L, true, null, null, "unknown", "trace-1", 0 + "SomeClass", methodName, Instant.now(), 1_000_000L, true, null, null, "unknown", null, "trace-1", 0 ); } diff --git a/pom.xml b/pom.xml index 092ff62..d7a5f8c 100644 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ com.fayupable logged-lib - 1.2.2 + 1.3.0 pom logged-lib