From 8aa173154066807c5b9838dda646a1db51450a41 Mon Sep 17 00:00:00 2001 From: Jamie Li Date: Tue, 17 May 2022 23:18:08 +0800 Subject: [PATCH 1/3] Added logging for SEP-1 --- .../main/java/org/stellar/anchor/sep1/Sep1Service.java | 8 ++++++++ .../main/java/org/stellar/anchor/util/FileUtil.java | 6 +++++- core/src/main/java/org/stellar/anchor/util/Log.java | 10 ++++++++++ .../kotlin/org/stellar/anchor/sep1/Sep1ServiceTest.kt | 2 +- platform/src/main/resources/log4j2.yaml | 2 +- 5 files changed, 25 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/stellar/anchor/sep1/Sep1Service.java b/core/src/main/java/org/stellar/anchor/sep1/Sep1Service.java index dedac035d6..2ae27328f3 100644 --- a/core/src/main/java/org/stellar/anchor/sep1/Sep1Service.java +++ b/core/src/main/java/org/stellar/anchor/sep1/Sep1Service.java @@ -1,5 +1,7 @@ package org.stellar.anchor.sep1; +import static org.stellar.anchor.util.Log.*; + import java.io.IOException; import org.stellar.anchor.api.exception.SepNotFoundException; import org.stellar.anchor.config.Sep1Config; @@ -31,10 +33,16 @@ public Sep1Service(Sep1Config sep1Config, ResourceReader resourceReader) { } public String getStellarToml() throws IOException, SepNotFoundException { + infoF("reading SEP1 TOML: {}", sep1Config.getStellarFile()); if (resourceReader == null) { + debugF("reading SEP1 TOML from file system. path={}", sep1Config.getStellarFile()); return FileUtil.getResourceFileAsString(sep1Config.getStellarFile()); } + debugF( + "reading SEP1 TOML from resource reader({}). path={}", + resourceReader.getClass().getSimpleName(), + sep1Config.getStellarFile()); return resourceReader.readResourceAsString(sep1Config.getStellarFile()); } } diff --git a/core/src/main/java/org/stellar/anchor/util/FileUtil.java b/core/src/main/java/org/stellar/anchor/util/FileUtil.java index 62a86357cd..ea717cf2c4 100644 --- a/core/src/main/java/org/stellar/anchor/util/FileUtil.java +++ b/core/src/main/java/org/stellar/anchor/util/FileUtil.java @@ -1,5 +1,7 @@ package org.stellar.anchor.util; +import static org.stellar.anchor.util.Log.errorF; + import java.io.BufferedReader; import java.io.IOException; import java.io.InputStream; @@ -12,8 +14,10 @@ public static String getResourceFileAsString(String fileName) throws IOException, SepNotFoundException { ClassLoader classLoader = FileUtil.class.getClassLoader(); try (InputStream is = classLoader.getResourceAsStream(fileName)) { - if (is == null) + if (is == null) { + errorF("{} was not found in class path.", fileName); throw new SepNotFoundException(String.format("%s was not found in classpath.", fileName)); + } try (InputStreamReader isr = new InputStreamReader(is); BufferedReader reader = new BufferedReader(isr)) { return reader.lines().collect(Collectors.joining(System.lineSeparator())); diff --git a/core/src/main/java/org/stellar/anchor/util/Log.java b/core/src/main/java/org/stellar/anchor/util/Log.java index dbdc3df1f2..f7c5323396 100644 --- a/core/src/main/java/org/stellar/anchor/util/Log.java +++ b/core/src/main/java/org/stellar/anchor/util/Log.java @@ -139,6 +139,16 @@ public static void error(String msg) { logger.error(msg); } + /** + * Send error log with a specified format. + * + * @param format The format + * @param args The arguments of the format + */ + public static void errorF(final String format, final Object... args) { + Logger logger = getLogger(); + logger.error(format, args); + } /** * Send exception ERROR log. * diff --git a/core/src/test/kotlin/org/stellar/anchor/sep1/Sep1ServiceTest.kt b/core/src/test/kotlin/org/stellar/anchor/sep1/Sep1ServiceTest.kt index 57b51b9ac8..765c1485ba 100644 --- a/core/src/test/kotlin/org/stellar/anchor/sep1/Sep1ServiceTest.kt +++ b/core/src/test/kotlin/org/stellar/anchor/sep1/Sep1ServiceTest.kt @@ -18,6 +18,6 @@ internal class Sep1ServiceTest { val sep1Service = Sep1Service(sep1Config) assertNotNull(sep1Service.stellarToml) - verify(exactly = 1) { sep1Config.stellarFile } + verify(exactly = 3) { sep1Config.stellarFile } } } diff --git a/platform/src/main/resources/log4j2.yaml b/platform/src/main/resources/log4j2.yaml index 332d48dac5..0e113de2df 100644 --- a/platform/src/main/resources/log4j2.yaml +++ b/platform/src/main/resources/log4j2.yaml @@ -13,7 +13,7 @@ Configutation: - ref: console_appender Logger: - name: org.stellar - level: info + level: debug - name: org.apache level: warn - name: org.hibernate From c6ddfcbb4999e73edc413c82e1e8d2ad9058d95e Mon Sep 17 00:00:00 2001 From: Jamie Li Date: Wed, 18 May 2022 00:32:25 +0800 Subject: [PATCH 2/3] Add SEP10 logging --- .../org/stellar/anchor/config/AppConfig.java | 1 + .../java/org/stellar/anchor/config/PII.java | 10 + .../org/stellar/anchor/config/Secret.java | 10 + .../stellar/anchor/config/Sep10Config.java | 1 + .../stellar/anchor/sep10/Sep10Service.java | 70 ++++-- .../java/org/stellar/anchor/util/Log.java | 215 +++++++++++++----- .../kotlin/org/stellar/anchor/Constants.kt | 2 +- .../kotlin/org/stellar/anchor/util/LogTest.kt | 64 +++++- .../platform/AnchorPlatformIntegrationTest.kt | 1 - 9 files changed, 291 insertions(+), 83 deletions(-) create mode 100644 core/src/main/java/org/stellar/anchor/config/PII.java create mode 100644 core/src/main/java/org/stellar/anchor/config/Secret.java diff --git a/core/src/main/java/org/stellar/anchor/config/AppConfig.java b/core/src/main/java/org/stellar/anchor/config/AppConfig.java index 1066e2bc18..b370faa888 100644 --- a/core/src/main/java/org/stellar/anchor/config/AppConfig.java +++ b/core/src/main/java/org/stellar/anchor/config/AppConfig.java @@ -9,6 +9,7 @@ public interface AppConfig { String getHorizonUrl(); + @Secret String getJwtSecretKey(); String getAssets(); diff --git a/core/src/main/java/org/stellar/anchor/config/PII.java b/core/src/main/java/org/stellar/anchor/config/PII.java new file mode 100644 index 0000000000..6a059f7ef8 --- /dev/null +++ b/core/src/main/java/org/stellar/anchor/config/PII.java @@ -0,0 +1,10 @@ +package org.stellar.anchor.config; + +import java.lang.annotation.*; + +/** Annotate that a method/field is personal identifiable information */ +@Documented +@Target({ElementType.FIELD, ElementType.METHOD}) +@Inherited +@Retention(RetentionPolicy.RUNTIME) +public @interface PII {} diff --git a/core/src/main/java/org/stellar/anchor/config/Secret.java b/core/src/main/java/org/stellar/anchor/config/Secret.java new file mode 100644 index 0000000000..e8928e6867 --- /dev/null +++ b/core/src/main/java/org/stellar/anchor/config/Secret.java @@ -0,0 +1,10 @@ +package org.stellar.anchor.config; + +import java.lang.annotation.*; + +/** To annotate if a method returns secret information. */ +@Documented +@Target({ElementType.METHOD}) +@Inherited +@Retention(RetentionPolicy.RUNTIME) +public @interface Secret {} diff --git a/core/src/main/java/org/stellar/anchor/config/Sep10Config.java b/core/src/main/java/org/stellar/anchor/config/Sep10Config.java index 8ee5359fcb..99c523c082 100644 --- a/core/src/main/java/org/stellar/anchor/config/Sep10Config.java +++ b/core/src/main/java/org/stellar/anchor/config/Sep10Config.java @@ -9,6 +9,7 @@ public interface Sep10Config { Boolean getEnabled(); + @Secret String getSigningSeed(); Integer getAuthTimeout(); diff --git a/core/src/main/java/org/stellar/anchor/sep10/Sep10Service.java b/core/src/main/java/org/stellar/anchor/sep10/Sep10Service.java index 996269c602..de10c6ed58 100644 --- a/core/src/main/java/org/stellar/anchor/sep10/Sep10Service.java +++ b/core/src/main/java/org/stellar/anchor/sep10/Sep10Service.java @@ -1,13 +1,5 @@ package org.stellar.anchor.sep10; -import static org.stellar.anchor.util.Log.infoF; -import static org.stellar.anchor.util.Log.shorter; - -import java.io.IOException; -import java.net.URI; -import java.net.URISyntaxException; -import java.util.*; -import java.util.stream.Collectors; import org.stellar.anchor.api.exception.SepException; import org.stellar.anchor.api.exception.SepValidationException; import org.stellar.anchor.api.sep.sep10.ChallengeRequest; @@ -17,13 +9,20 @@ import org.stellar.anchor.config.AppConfig; import org.stellar.anchor.config.Sep10Config; import org.stellar.anchor.horizon.Horizon; -import org.stellar.anchor.util.Log; import org.stellar.anchor.util.Sep1Helper; import org.stellar.anchor.util.Sep1Helper.TomlContent; import org.stellar.sdk.*; import org.stellar.sdk.requests.ErrorResponse; import org.stellar.sdk.responses.AccountResponse; +import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; +import java.util.*; +import java.util.stream.Collectors; + +import static org.stellar.anchor.util.Log.*; + /** The Sep-10 protocol service. */ public class Sep10Service { final AppConfig appConfig; @@ -34,27 +33,35 @@ public class Sep10Service { public Sep10Service( AppConfig appConfig, Sep10Config sep10Config, Horizon horizon, JwtService jwtService) { + infoF("Creating Sep10Service"); + infoConfig("appConfig:", appConfig, AppConfig.class); this.appConfig = appConfig; + + infoConfig("sep10Config:", sep10Config, Sep10Config.class); this.sep10Config = sep10Config; + this.horizon = horizon; this.jwtService = jwtService; this.serverAccountId = KeyPair.fromSecretSeed(sep10Config.getSigningSeed()).getAccountId(); } public ChallengeResponse createChallenge(ChallengeRequest challengeRequest) throws SepException { + info("Creating challenge"); // // Validations // if (challengeRequest.getHomeDomain() == null) { + infoF("home_domain is not specified. Use {}", sep10Config.getHomeDomain()); challengeRequest.setHomeDomain(sep10Config.getHomeDomain()); } else if (!sep10Config.getHomeDomain().equalsIgnoreCase(challengeRequest.getHomeDomain())) { + infoF("Bad home_domain: {}", challengeRequest.getHomeDomain()); throw new SepValidationException( String.format("home_domain [%s] is not supported.", challengeRequest.getHomeDomain())); } if (sep10Config.isClientAttributionRequired()) { if (challengeRequest.getClientDomain() == null) { - infoF("ALERT: client domain required and not provided"); + info("client_domain is required but not provided"); throw new SepValidationException("client_domain is required"); } @@ -63,7 +70,7 @@ public ChallengeResponse createChallenge(ChallengeRequest challengeRequest) thro && denyList.size() > 0 && denyList.contains(challengeRequest.getClientDomain())) { infoF( - "ALERT: client domain provided is in configured deny list - {} ", + "client_domain({}) provided is in the configured deny list", challengeRequest.getClientDomain()); throw new SepValidationException("unable to process."); } @@ -73,7 +80,7 @@ public ChallengeResponse createChallenge(ChallengeRequest challengeRequest) thro && allowList.size() > 0 && !allowList.contains(challengeRequest.getClientDomain())) { infoF( - "ALERT: client domain provided is not in configured allow list - {} ", + "client_domain provided ({}) is not in configured allow list", challengeRequest.getClientDomain()); throw new SepValidationException("unable to process"); } @@ -82,7 +89,7 @@ public ChallengeResponse createChallenge(ChallengeRequest challengeRequest) thro try { KeyPair.fromAccountId(challengeRequest.getAccount()); } catch (Exception ex) { - infoF("ALERT: client wallet account is invalid - {}", challengeRequest.getAccount()); + infoF("client wallet account ({}) is invalid", challengeRequest.getAccount()); throw new SepValidationException("Invalid account."); } @@ -91,11 +98,13 @@ public ChallengeResponse createChallenge(ChallengeRequest challengeRequest) thro if (challengeRequest.getMemo() != null) { int memoInt = Integer.parseInt(challengeRequest.getMemo()); if (memoInt <= 0) { + infoF("Invalid memo value: {}", challengeRequest.getMemo()); throw new SepValidationException( String.format("Invalid memo value: %s", challengeRequest.getMemo())); } } } catch (NumberFormatException e) { + infoF("invalid memo format: {}. Only MEMO_INT is supported", challengeRequest.getMemo()); throw new SepValidationException( String.format("Invalid memo format: %s", challengeRequest.getMemo())); } @@ -106,7 +115,9 @@ public ChallengeResponse createChallenge(ChallengeRequest challengeRequest) thro try { String clientSigningKey = null; if (!Objects.toString(challengeRequest.getClientDomain(), "").isEmpty()) { + infoF("Fetching SIGNING_KEY from client_domain: {}", challengeRequest.getClientDomain()); clientSigningKey = getClientAccountId(challengeRequest.getClientDomain()); + debugF("SIGNING_KEY from client_domain fetched: {}", clientSigningKey); } KeyPair signer = KeyPair.fromSecretSeed(sep10Config.getSigningSeed()); @@ -124,12 +135,17 @@ public ChallengeResponse createChallenge(ChallengeRequest challengeRequest) thro : challengeRequest.getClientDomain(), (clientSigningKey == null) ? "" : clientSigningKey); // Convert the challenge to response - return ChallengeResponse.of( - txn.toEnvelopeXdrBase64(), appConfig.getStellarNetworkPassphrase()); + traceB("SEP-10 challenge txn:", txn); + ChallengeResponse challengeResponse = + ChallengeResponse.of(txn.toEnvelopeXdrBase64(), appConfig.getStellarNetworkPassphrase()); + traceB("challengeResponse:", challengeResponse); + return challengeResponse; } catch (URISyntaxException e) { + warnF("Invalid HOST_URL: {}", appConfig.getHostUrl()); throw new SepException( String.format("Invalid HOST_URL [%s} is used.", appConfig.getHostUrl())); } catch (InvalidSep10ChallengeException ex) { + warnEx(ex); throw new SepException("Failed to create the sep-10 challenge.", ex); } } @@ -148,7 +164,7 @@ public ValidationResponse validateChallenge(ValidationRequest validationRequest) public String validateChallenge(String challengeXdr) throws IOException, InvalidSep10ChallengeException, URISyntaxException { - Log.info("Parse challenge string."); + debug("Parse challenge string."); Sep10Challenge.ChallengeTransaction challenge = Sep10Challenge.readChallengeTransaction( challengeXdr, @@ -157,11 +173,13 @@ public String validateChallenge(String challengeXdr) sep10Config.getHomeDomain(), getDomainFromURI(appConfig.getHostUrl())); - infoF( + debugF( "Challenge parsed. account={}, home_domain={}", shorter(challenge.getClientAccountId()), challenge.getMatchedHomeDomain()); + traceB("challenge:", challenge); + String clientDomain = null; Operation operation = Arrays.stream(challenge.getTransaction().getOperations()) @@ -172,15 +190,20 @@ public String validateChallenge(String challengeXdr) .findFirst() .orElse(null); + traceB("Challenge operation:", operation); if (operation != null) { clientDomain = new String(((ManageDataOperation) operation).getValue()); } + debugF("client_domain: {}", clientDomain); // Check the client's account AccountResponse account; try { + infoF("Checking if {} exists in the Stellar network", challenge.getClientAccountId()); account = horizon.getServer().accounts().account(challenge.getClientAccountId()); + traceF("challenge account: {}", account); } catch (ErrorResponse ex) { + infoF("Account {} does not exist in the Stellar Network"); // account not found // The client account does not exist, using the client's master key to verify. Set signers = new HashSet<>(); @@ -194,11 +217,12 @@ public String validateChallenge(String challengeXdr) if ((clientDomain != null && challenge.getTransaction().getSignatures().size() != 3) || (clientDomain == null && challenge.getTransaction().getSignatures().size() != 2)) { infoF( - "ALERT: Invalid SEP 10 challenge exception, there is more than one client signer on challenge transaction for an account that doesn't exist"); + "Invalid SEP 10 challenge exception, there is more than one client signer on challenge transaction for an account that doesn't exist"); throw new InvalidSep10ChallengeException( "There is more than one client signer on challenge transaction for an account that doesn't exist"); } + debug("Calling Sep10Challenge.verifyChallengeTransactionSigners"); Sep10Challenge.verifyChallengeTransactionSigners( challengeXdr, serverAccountId, @@ -219,12 +243,12 @@ public String validateChallenge(String challengeXdr) // the signatures must be greater than the medium threshold of the account. int threshold = account.getThresholds().getMedThreshold(); + infoF( "Verifying challenge threshold. server_account={}, threshold={}, signers={}", shorter(serverAccountId), threshold, signers.size()); - Sep10Challenge.verifyChallengeTransactionThreshold( challengeXdr, serverAccountId, @@ -241,25 +265,31 @@ String getClientAccountId(String clientDomain) throws SepException { String clientSigningKey = ""; String url = "https://" + clientDomain + "/.well-known/stellar.toml"; try { + debugF("Fetching {}", url); TomlContent toml = Sep1Helper.readToml(url); clientSigningKey = toml.getString("SIGNING_KEY"); if (clientSigningKey == null) { + infoF("SIGNING_KEY not present in 'client_domain' TOML."); throw new SepException("SIGNING_KEY not present in 'client_domain' TOML"); } // client key validation + debugF("Validating client_domain signing key: {}", clientSigningKey); KeyPair.fromAccountId(clientSigningKey); return clientSigningKey; } catch (IllegalArgumentException | FormatException ex) { + infoF("SIGNING_KEY {} is not a valid Stellar account Id.", clientSigningKey); throw new SepException( String.format("SIGNING_KEY %s is not a valid Stellar account Id.", clientSigningKey)); } catch (IOException ioex) { + infoF("Unable to read from {}", url); throw new SepException(String.format("Unable to read from %s", url), ioex); } } String generateSep10Jwt(String challengeXdr, String clientDomain) throws InvalidSep10ChallengeException, IOException, URISyntaxException { + infoF("Creating SEP-10 challenge."); Sep10Challenge.ChallengeTransaction challenge = Sep10Challenge.readChallengeTransaction( challengeXdr, @@ -267,6 +297,7 @@ String generateSep10Jwt(String challengeXdr, String clientDomain) new Network(appConfig.getStellarNetworkPassphrase()), sep10Config.getHomeDomain(), getDomainFromURI(appConfig.getHostUrl())); + debugB("challenge:", challenge); long issuedAt = challenge.getTransaction().getTimeBounds().getMinTime(); JwtToken jwtToken = JwtToken.of( @@ -276,6 +307,7 @@ String generateSep10Jwt(String challengeXdr, String clientDomain) issuedAt + sep10Config.getJwtTimeout(), challenge.getTransaction().hashHex(), clientDomain); + debugB("jwtToken:", jwtToken); return jwtService.encode(jwtToken); } diff --git a/core/src/main/java/org/stellar/anchor/util/Log.java b/core/src/main/java/org/stellar/anchor/util/Log.java index f7c5323396..f68b2075fc 100644 --- a/core/src/main/java/org/stellar/anchor/util/Log.java +++ b/core/src/main/java/org/stellar/anchor/util/Log.java @@ -6,13 +6,98 @@ import java.beans.PropertyDescriptor; import java.io.PrintWriter; import java.io.StringWriter; +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.util.function.Consumer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.stellar.anchor.config.PII; +import org.stellar.anchor.config.Secret; /** Logging utility functions. */ public class Log { static final Gson gson = GsonUtils.builder().setPrettyPrinting().create(); + /** + * Send debug log. + * + * @param msg the debug message. + */ + public static void debug(final String msg) { + Logger logger = getLogger(); + logger.debug(msg); + } + + /** + * Send the msg as DEBUG log and detail as JSON. + * + * @param msg the debug message. + * @param detail The additional object to be logged. + */ + public static void debug(final String msg, Object detail) { + Logger logger = getLogger(); + logger.debug(msg); + if (detail != null) { + logger.debug(gson.toJson(detail)); + } + } + + /** + * Send msg as DEBUG log and detail as a Java bean. + * + * @param msg the debug message. + * @param detail The additional object to be logged. + */ + public static void debugB(final String msg, final Object detail) { + Logger logger = getLogger(); + printBeanFormat(msg, detail, logger::debug); + } + + /** + * Send debug log with a specified format. + * + * @param format The format + * @param args The arguments of the format + */ + public static void debugF(final String format, final Object... args) { + Logger logger = getLogger(); + logger.debug(format, args); + } + + /** + * Send message to ERROR log. + * + * @param msg The message + */ + public static void error(String msg) { + Logger logger = getLogger(); + logger.error(msg); + } + + /** + * Send exception ERROR log. + * + * @param ex The exception. + */ + public static void errorEx(final Throwable ex) { + Logger logger = getLogger(); + StringWriter sw = new StringWriter(); + PrintWriter pw = new PrintWriter(sw); + ex.printStackTrace(pw); + logger.error(sw.toString()); + } + + /** + * Send error log with a specified format. + * + * @param format The format + * @param args The arguments of the format + */ + public static void errorF(final String format, final Object... args) { + Logger logger = getLogger(); + logger.error(format, args); + } + /** * Send msg as INFO log. * @@ -42,16 +127,31 @@ public static void info(final String msg, final Object detail) { * @param detail The additional object to be logged. */ public static void infoB(final String msg, final Object detail) { + Logger logger = getLogger(); + printBeanFormat(msg, detail, logger::info); + } + + /** + * Send msg and configuration object as INFO log. + * + * @param msg the message. + * @param config the configuration to be logged. + */ + public static void infoConfig(final String msg, final Object config, final Class configClazz) { Logger logger = getLogger(); logger.info(msg); - BeanInfo beanInfo; try { - StringBuilder sb = new StringBuilder("{\n"); - beanInfo = Introspector.getBeanInfo(detail.getClass()); - PropertyDescriptor[] pds = beanInfo.getPropertyDescriptors(); - for (PropertyDescriptor pd : pds) { - Object value = pd.getReadMethod().invoke(detail); - sb.append(String.format("'%s': '%s'\n", pd.getName(), value)); + StringBuilder sb = new StringBuilder("{"); + Method[] methods = configClazz.getMethods(); + for (int i = 0; i < methods.length; i++) { + Method method = methods[i]; + if (!method.isAnnotationPresent(Secret.class)) { + Object result = method.invoke(config); + sb.append(String.format("'%s': '%s'", method.getName(), result)); + if (i != methods.length - 1) { + sb.append(","); + } + } } sb.append("}"); logger.info(sb.toString()); @@ -72,38 +172,39 @@ public static void infoF(final String format, final Object... args) { } /** - * Send debug log. + * Return shorter version of the account. * - * @param msg the debug message. + * @param account The Stellar account Id. + * @return The shorter version. */ - public static void debug(final String msg) { - Logger logger = getLogger(); - logger.debug(msg); + public static String shorter(String account) { + if (account.length() > 11) { + return account.substring(0, 4) + "..." + account.substring(account.length() - 4); + } else { + return account; + } } /** - * Send the msg as DEBUG log and detail as JSON. + * Send msg as TRACE log and detail as a Java bean. * * @param msg the debug message. * @param detail The additional object to be logged. */ - public static void debug(final String msg, Object detail) { + public static void traceB(final String msg, final Object detail) { Logger logger = getLogger(); - logger.debug(msg); - if (detail != null) { - logger.debug(gson.toJson(detail)); - } + printBeanFormat(msg, detail, logger::trace); } /** - * Send debug log with a specified format. + * Send TRACE log with format. * * @param format The format * @param args The arguments of the format */ - public static void debugF(final String format, final Object... args) { + public static void traceF(final String format, final Object... args) { Logger logger = getLogger(); - logger.debug(format, args); + logger.trace(format, args); } /** @@ -130,54 +231,48 @@ public static void warnEx(final Throwable ex) { } /** - * Send message to ERROR log. - * - * @param msg The message - */ - public static void error(String msg) { - Logger logger = getLogger(); - logger.error(msg); - } - - /** - * Send error log with a specified format. + * Send WARN log with format. * * @param format The format * @param args The arguments of the format */ - public static void errorF(final String format, final Object... args) { + public static void warnF(final String format, final Object... args) { Logger logger = getLogger(); - logger.error(format, args); - } - /** - * Send exception ERROR log. - * - * @param ex The exception. - */ - public static void errorEx(final Throwable ex) { - Logger logger = getLogger(); - StringWriter sw = new StringWriter(); - PrintWriter pw = new PrintWriter(sw); - ex.printStackTrace(pw); - logger.error(sw.toString()); - } - - /** - * Return shorter version of the account. - * - * @param account The Stellar account Id. - * @return The shorter version. - */ - public static String shorter(String account) { - if (account.length() > 11) { - return account.substring(0, 4) + "..." + account.substring(account.length() - 4); - } else { - return account; - } + logger.warn(format, args); } static Logger getLogger() { String cls = Thread.currentThread().getStackTrace()[3].getClassName(); return LoggerFactory.getLogger(cls); } + + static void printBeanFormat(final String msg, final Object detail, Consumer output) { + output.accept(msg); + BeanInfo beanInfo; + try { + StringBuilder sb = new StringBuilder("{\n"); + beanInfo = Introspector.getBeanInfo(detail.getClass()); + PropertyDescriptor[] pds = beanInfo.getPropertyDescriptors(); + for (PropertyDescriptor pd : pds) { + try { + Field field = detail.getClass().getDeclaredField(pd.getName()); + if (field.isAnnotationPresent(PII.class)) { + continue; + } + } catch (NoSuchFieldException ex) { + // do nothing. proceed to check method + } + + if (pd.getReadMethod().isAnnotationPresent(PII.class)) { + continue; + } + Object value = pd.getReadMethod().invoke(detail); + sb.append(String.format("'%s': '%s'\n", pd.getName(), value)); + } + sb.append("}"); + output.accept(sb.toString()); + } catch (Exception e) { + output.accept("Unable to serialize the bean."); + } + } } diff --git a/core/src/test/kotlin/org/stellar/anchor/Constants.kt b/core/src/test/kotlin/org/stellar/anchor/Constants.kt index 6fa4bfe56f..94bf000e74 100644 --- a/core/src/test/kotlin/org/stellar/anchor/Constants.kt +++ b/core/src/test/kotlin/org/stellar/anchor/Constants.kt @@ -19,7 +19,7 @@ class Constants { const val TEST_CLIENT_TOML = "" + " NETWORK_PASSPHRASE=\"Public Global Stellar Network ; September 2015\"\n" + - " HORIZON_URL=\"https://horizon.stellar.lobstr.co\"\n" + + " HORIZON_URL=\"https://horizon.stellar.org\"\n" + " FEDERATION_SERVER=\"https://preview.lobstr.co/federation/\"\n" + " SIGNING_KEY=\"GACYKME36AI6UYAV7A5ZUA6MG4C4K2VAPNYMW5YLOM6E7GS6FSHDPV4F\"\n" } diff --git a/core/src/test/kotlin/org/stellar/anchor/util/LogTest.kt b/core/src/test/kotlin/org/stellar/anchor/util/LogTest.kt index 58920bd78d..393b166726 100644 --- a/core/src/test/kotlin/org/stellar/anchor/util/LogTest.kt +++ b/core/src/test/kotlin/org/stellar/anchor/util/LogTest.kt @@ -5,9 +5,13 @@ import io.mockk.impl.annotations.MockK import java.beans.IntrospectionException import java.beans.Introspector import org.junit.jupiter.api.* -import org.junit.jupiter.api.Assertions.assertEquals -import org.junit.jupiter.api.Assertions.assertNotNull +import org.junit.jupiter.api.Assertions.* import org.slf4j.Logger +import org.stellar.anchor.Constants.Companion.TEST_HOST_URL +import org.stellar.anchor.Constants.Companion.TEST_JWT_SECRET +import org.stellar.anchor.Constants.Companion.TEST_NETWORK_PASS_PHRASE +import org.stellar.anchor.config.AppConfig +import org.stellar.anchor.config.PII import org.stellar.anchor.util.Log.gson import org.stellar.anchor.util.Log.shorter @@ -34,6 +38,13 @@ internal class LogTest { val field2: String = "2" } + @Suppress("unused") + class TestBeanPII { + val fieldNoPII: String = "no secret" + + @PII val fieldPII: String = "secret" + } + @Test fun testInfoDebug() { Log.info("Hello") @@ -66,6 +77,55 @@ internal class LogTest { verify(exactly = 2) { logger.info(ofType(String::class)) } } + @Test + fun testInfoBPII() { + val slot = slot() + every { logger.info(capture(slot)) } answers {} + + val testBeanPII = TestBeanPII() + Log.infoB("Hello", testBeanPII) + verify(exactly = 2) { logger.info(ofType(String::class)) } + assertFalse(slot.captured.contains("fieldPII")) + } + + @Suppress("unused") + class TestAppConfig : AppConfig { + override fun getStellarNetworkPassphrase(): String { + return TEST_NETWORK_PASS_PHRASE + } + + override fun getHostUrl(): String { + return TEST_HOST_URL + } + + override fun getHorizonUrl(): String { + return "https://horizon.stellar.org" + } + + override fun getJwtSecretKey(): String { + return TEST_JWT_SECRET + } + + override fun getAssets(): String { + return "test_assets_file" + } + + override fun getLanguages(): MutableList { + return mutableListOf("en") + } + } + + @Test + fun testInfoConfig() { + val slot = slot() + every { logger.info(capture(slot)) } answers {} + + val testAppConfig = TestAppConfig() + Log.infoConfig("Hello", testAppConfig, AppConfig::class.java) + verify(exactly = 2) { logger.info(ofType(String::class)) } + assertFalse(slot.captured.contains(TEST_JWT_SECRET)) + } + @Test fun testInfoDebugF() { Log.infoF("Hello {}", "world") diff --git a/integration-tests/src/test/kotlin/org/stellar/anchor/platform/AnchorPlatformIntegrationTest.kt b/integration-tests/src/test/kotlin/org/stellar/anchor/platform/AnchorPlatformIntegrationTest.kt index 9d099e114c..3344fa6db6 100644 --- a/integration-tests/src/test/kotlin/org/stellar/anchor/platform/AnchorPlatformIntegrationTest.kt +++ b/integration-tests/src/test/kotlin/org/stellar/anchor/platform/AnchorPlatformIntegrationTest.kt @@ -97,7 +97,6 @@ class AnchorPlatformIntegrationTest { assertEquals(streams[0]["thread_shutdown"], false) assertEquals(streams[0]["thread_terminated"], false) assertEquals(streams[0]["stopped"], false) - assertNotNull(streams[0]["lastEventId"]) } @Test From cdc7835d0b825ccfbc6423f4fdc41e48c941266c Mon Sep 17 00:00:00 2001 From: Jamie Li Date: Thu, 19 May 2022 18:04:03 +0800 Subject: [PATCH 3/3] PR review update --- .../stellar/anchor/sep10/Sep10Service.java | 19 +++++----- .../java/org/stellar/anchor/util/Log.java | 36 ++++++++++++++----- .../kotlin/org/stellar/anchor/util/LogTest.kt | 20 ++++++++--- 3 files changed, 51 insertions(+), 24 deletions(-) diff --git a/core/src/main/java/org/stellar/anchor/sep10/Sep10Service.java b/core/src/main/java/org/stellar/anchor/sep10/Sep10Service.java index de10c6ed58..7c531d233c 100644 --- a/core/src/main/java/org/stellar/anchor/sep10/Sep10Service.java +++ b/core/src/main/java/org/stellar/anchor/sep10/Sep10Service.java @@ -1,5 +1,12 @@ package org.stellar.anchor.sep10; +import static org.stellar.anchor.util.Log.*; + +import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; +import java.util.*; +import java.util.stream.Collectors; import org.stellar.anchor.api.exception.SepException; import org.stellar.anchor.api.exception.SepValidationException; import org.stellar.anchor.api.sep.sep10.ChallengeRequest; @@ -15,14 +22,6 @@ import org.stellar.sdk.requests.ErrorResponse; import org.stellar.sdk.responses.AccountResponse; -import java.io.IOException; -import java.net.URI; -import java.net.URISyntaxException; -import java.util.*; -import java.util.stream.Collectors; - -import static org.stellar.anchor.util.Log.*; - /** The Sep-10 protocol service. */ public class Sep10Service { final AppConfig appConfig; @@ -51,7 +50,7 @@ public ChallengeResponse createChallenge(ChallengeRequest challengeRequest) thro // Validations // if (challengeRequest.getHomeDomain() == null) { - infoF("home_domain is not specified. Use {}", sep10Config.getHomeDomain()); + debugF("home_domain is not specified. Will use the default: {}", sep10Config.getHomeDomain()); challengeRequest.setHomeDomain(sep10Config.getHomeDomain()); } else if (!sep10Config.getHomeDomain().equalsIgnoreCase(challengeRequest.getHomeDomain())) { infoF("Bad home_domain: {}", challengeRequest.getHomeDomain()); @@ -115,7 +114,7 @@ public ChallengeResponse createChallenge(ChallengeRequest challengeRequest) thro try { String clientSigningKey = null; if (!Objects.toString(challengeRequest.getClientDomain(), "").isEmpty()) { - infoF("Fetching SIGNING_KEY from client_domain: {}", challengeRequest.getClientDomain()); + debugF("Fetching SIGNING_KEY from client_domain: {}", challengeRequest.getClientDomain()); clientSigningKey = getClientAccountId(challengeRequest.getClientDomain()); debugF("SIGNING_KEY from client_domain fetched: {}", clientSigningKey); } diff --git a/core/src/main/java/org/stellar/anchor/util/Log.java b/core/src/main/java/org/stellar/anchor/util/Log.java index f68b2075fc..8aa060d49b 100644 --- a/core/src/main/java/org/stellar/anchor/util/Log.java +++ b/core/src/main/java/org/stellar/anchor/util/Log.java @@ -24,8 +24,7 @@ public class Log { * @param msg the debug message. */ public static void debug(final String msg) { - Logger logger = getLogger(); - logger.debug(msg); + debug(msg, null); } /** @@ -43,7 +42,8 @@ public static void debug(final String msg, Object detail) { } /** - * Send msg as DEBUG log and detail as a Java bean. + * Send msg as DEBUG log and detail as a Java bean. Ignore properties that are annotated + * with @PII. * * @param msg the debug message. * @param detail The additional object to be logged. @@ -80,9 +80,21 @@ public static void error(String msg) { * @param ex The exception. */ public static void errorEx(final Throwable ex) { + errorEx(null, ex); + } + + /** + * Send exception ERROR log with a message. + * + * @param ex The exception. + */ + public static void errorEx(String msg, final Throwable ex) { Logger logger = getLogger(); StringWriter sw = new StringWriter(); PrintWriter pw = new PrintWriter(sw); + if (msg != null) { + pw.println(msg); + } ex.printStackTrace(pw); logger.error(sw.toString()); } @@ -104,8 +116,7 @@ public static void errorF(final String format, final Object... args) { * @param msg the debug message. */ public static void info(final String msg) { - Logger logger = getLogger(); - logger.info(msg); + info(msg, null); } /** @@ -117,11 +128,13 @@ public static void info(final String msg) { public static void info(final String msg, final Object detail) { Logger logger = getLogger(); logger.info(msg); - logger.info(gson.toJson(detail)); + if (detail != null) { + logger.info(gson.toJson(detail)); + } } /** - * Send msg as INFO log and detail as a Java bean. + * Send msg as INFO log and detail as a Java bean. Ignore properties that are annotated with @PII. * * @param msg the debug message. * @param detail The additional object to be logged. @@ -132,7 +145,7 @@ public static void infoB(final String msg, final Object detail) { } /** - * Send msg and configuration object as INFO log. + * Send msg and configuration object as INFO log. Ignore methods that are annotated with @Secret. * * @param msg the message. * @param config the configuration to be logged. @@ -186,7 +199,8 @@ public static String shorter(String account) { } /** - * Send msg as TRACE log and detail as a Java bean. + * Send msg as TRACE log and detail as a Java bean. Ignore properties that are annotated + * with @PII. * * @param msg the debug message. * @param detail The additional object to be logged. @@ -266,6 +280,10 @@ static void printBeanFormat(final String msg, final Object detail, Consumer() - every { logger.info(capture(slot)) } answers {} + val slotMessages = mutableListOf() + every { logger.info(capture(slotMessages)) } answers {} val testBeanPII = TestBeanPII() Log.infoB("Hello", testBeanPII) + verify(exactly = 2) { logger.info(ofType(String::class)) } - assertFalse(slot.captured.contains("fieldPII")) + assertEquals("Hello", slotMessages[0]) + val wantBean = """{ +'fieldNoPII': 'no secret' +}""".trimMargin() + assertEquals(wantBean, slotMessages[1]) } - @Suppress("unused") class TestAppConfig : AppConfig { override fun getStellarNetworkPassphrase(): String { @@ -152,8 +156,14 @@ internal class LogTest { @Test fun testErrorEx() { Log.errorEx(Exception("mock exception")) - verify(exactly = 1) { logger.error(any()) } + + val slot = slot() + every { logger.error(capture(slot)) } answers {} + + Log.errorEx("Hello", Exception("mock exception")) + assertTrue(slot.captured.contains("Hello")) + assertTrue(slot.captured.contains("mock exception")) } @Test