diff --git a/cwms-data-api/src/main/java/cwms/cda/ApiServlet.java b/cwms-data-api/src/main/java/cwms/cda/ApiServlet.java index 34207341f..8d26760a6 100644 --- a/cwms-data-api/src/main/java/cwms/cda/ApiServlet.java +++ b/cwms-data-api/src/main/java/cwms/cda/ApiServlet.java @@ -106,6 +106,7 @@ import cwms.cda.api.enums.UnitSystem; import cwms.cda.api.errors.ApplicationException; import cwms.cda.api.errors.CdaError; +import cwms.cda.api.errors.ErrorTraceSupport; import cwms.cda.api.location.kind.GateChangeCreateController; import cwms.cda.api.location.kind.GateChangeDeleteController; import cwms.cda.api.location.kind.GateChangeGetAllController; @@ -355,31 +356,32 @@ public void init() { ctx.header("X-XSS-Protection", "1; mode=block"); }) .exception(ApplicationException.class, (e, ctx) -> { - CdaError re = new CdaError(e.getCdaErrorMessage(), e.getSource(), e.getDetails()); + CdaError re = ErrorTraceSupport.buildError(ctx, e.getCdaErrorMessage(), + e.getSource(), e.getDetails(), e); if (e.getLoggerLevel().isPresent()) { logger.at(e.getLoggerLevel().get()).withCause(e).log(re.toString()); } ctx.status(e.getCdaHttpErrorCode()).json(re); }) .exception(UnsupportedOperationException.class, (e, ctx) -> { - final CdaError re = CdaError.notImplemented(); + final CdaError re = ErrorTraceSupport.buildError(ctx, "Not Implemented", e); logger.atWarning().withCause(e) .log("%s for request: %s", re, ctx.fullUrl()); ctx.status(HttpServletResponse.SC_NOT_IMPLEMENTED).json(re); }) .exception(BadRequestResponse.class, (e, ctx) -> { - CdaError re = new CdaError("Bad Request", - "User Input", new HashMap<>(e.getDetails())); + CdaError re = ErrorTraceSupport.buildError(ctx, "Bad Request", + "User Input", new HashMap<>(e.getDetails()), e); logger.atInfo().withCause(e).log(re.toString()); ctx.status(e.getStatus()).json(re); }) .exception(IllegalArgumentException.class, (e, ctx) -> { - CdaError re = new CdaError("Bad Request"); + CdaError re = ErrorTraceSupport.buildError(ctx, "Bad Request", e); logger.atInfo().withCause(e).log(re.toString()); ctx.status(HttpServletResponse.SC_BAD_REQUEST).json(re); }) .exception(DateTimeException.class, (e, ctx) -> { - CdaError re = new CdaError(e.getMessage()); + CdaError re = ErrorTraceSupport.buildError(ctx, e.getMessage(), e); ctx.status(HttpServletResponse.SC_BAD_REQUEST).json(re); }) .exception(DataAccessException.class, (e, ctx) -> { @@ -393,7 +395,7 @@ public void init() { // CdaError does not include the Oracle exception message b/c this block catches // all unhandled DataAccessExceptions and we don't know what is in the message // it is unknown if the message would be safe/appropriate for users to see. - CdaError errResponse = new CdaError("Database Error"); + CdaError errResponse = ErrorTraceSupport.buildError(ctx, "Database Error", e); logger.atWarning().withCause(e).log("error on request[%s]: %s", errResponse.getIncidentIdentifier(), ctx.req.getRequestURI()); ctx.status(500); @@ -401,7 +403,7 @@ public void init() { ctx.json(errResponse); }) .exception(Exception.class, (e, ctx) -> { - CdaError errResponse = new CdaError("System Error"); + CdaError errResponse = ErrorTraceSupport.buildError(ctx, "System Error", e); logger.atWarning().withCause(e).log("error on request[%s]: %s", errResponse.getIncidentIdentifier(), ctx.req.getRequestURI()); ctx.status(500); diff --git a/cwms-data-api/src/main/java/cwms/cda/api/BasinController.java b/cwms-data-api/src/main/java/cwms/cda/api/BasinController.java index c8bdf4f4a..d66174f98 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/BasinController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/BasinController.java @@ -42,6 +42,7 @@ import com.codahale.metrics.Timer; import cwms.cda.api.enums.UnitSystem; import cwms.cda.api.errors.CdaError; +import cwms.cda.api.errors.ErrorTraceSupport; import cwms.cda.data.dao.JooqDao; import cwms.cda.data.dao.basinconnectivity.BasinDao; import cwms.cda.data.dto.CwmsId; @@ -143,7 +144,7 @@ public void getAll(@NotNull Context ctx) { ctx.result(result); ctx.status(HttpServletResponse.SC_OK); } catch (SQLException ex) { - CdaError error = new CdaError("Error retrieving all basins"); + CdaError error = ErrorTraceSupport.buildError(ctx, "Error retrieving all basins", ex); LOGGER.atSevere().withCause(ex).log("Error retrieving all basins"); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(error); } diff --git a/cwms-data-api/src/main/java/cwms/cda/api/BinaryTimeSeriesController.java b/cwms-data-api/src/main/java/cwms/cda/api/BinaryTimeSeriesController.java index 8f6f1d0e1..980151ac9 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/BinaryTimeSeriesController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/BinaryTimeSeriesController.java @@ -45,6 +45,7 @@ import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.Timer; import cwms.cda.api.errors.CdaError; +import cwms.cda.api.errors.ErrorTraceSupport; import cwms.cda.data.dao.binarytimeseries.TimeSeriesBinaryDao; import cwms.cda.data.dto.binarytimeseries.BinaryTimeSeries; import cwms.cda.formatters.ContentType; @@ -163,8 +164,8 @@ public void getAll(@NotNull Context ctx) { ctx.status(HttpServletResponse.SC_OK); } catch (URISyntaxException | UnsupportedEncodingException ex) { - CdaError re = - new CdaError("Failed to process request: " + ex.getLocalizedMessage()); + CdaError re = ErrorTraceSupport.buildError(ctx, + "Failed to process request: " + ex.getLocalizedMessage(), ex); logger.atSevere().withCause(ex).log("%s", re); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } diff --git a/cwms-data-api/src/main/java/cwms/cda/api/LocationController.java b/cwms-data-api/src/main/java/cwms/cda/api/LocationController.java index f1610e113..13188ea6f 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/LocationController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/LocationController.java @@ -43,6 +43,7 @@ import cwms.cda.api.enums.UnitSystem; import cwms.cda.api.errors.CdaError; import cwms.cda.api.errors.DeleteConflictException; +import cwms.cda.api.errors.ErrorTraceSupport; import cwms.cda.data.dao.LocationsDao; import cwms.cda.data.dao.LocationsDaoImpl; import cwms.cda.data.dto.Location; @@ -200,10 +201,6 @@ public void getAll(@NotNull Context ctx) { ctx.status(HttpServletResponse.SC_OK); - } catch (Exception ex) { - CdaError re = new CdaError("failed to process request"); - logger.atSevere().withCause(ex).log("%s", re.toString()); - ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } } @@ -261,7 +258,7 @@ public void getOne(@NotNull Context ctx, @NotNull String locationId) { addDeprecatedContentTypeWarning(ctx, contentType); } catch (IOException ex) { String errorMsg = "Error retrieving " + locationId; - CdaError re = new CdaError(errorMsg); + CdaError re = ErrorTraceSupport.buildError(ctx, errorMsg, ex); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); logger.atSevere().withCause(ex).log("%s", errorMsg); } @@ -303,7 +300,7 @@ public void create(@NotNull Context ctx) { "Created Location", locationFromBody.getName()); ctx.status(HttpServletResponse.SC_CREATED).json(re); } catch (IOException ex) { - CdaError re = new CdaError("failed to process request"); + CdaError re = ErrorTraceSupport.buildError(ctx, "failed to process request", ex); logger.atSevere().withCause(ex).log("%s", re.toString()); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } @@ -357,8 +354,8 @@ public void update(@NotNull Context ctx, @NotNull String locationId) { "Updated Location", updatedLocation.getName())); } } catch (IOException ex) { - CdaError re = - new CdaError("Failed to process request: " + ex.getLocalizedMessage()); + CdaError re = ErrorTraceSupport.buildError(ctx, + "Failed to process request: " + ex.getLocalizedMessage(), ex); logger.atSevere().withCause(ex).log("%s", re.toString()); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } diff --git a/cwms-data-api/src/main/java/cwms/cda/api/ParametersController.java b/cwms-data-api/src/main/java/cwms/cda/api/ParametersController.java index b3a103d45..b4fa06aae 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/ParametersController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/ParametersController.java @@ -28,12 +28,10 @@ import io.javalin.plugin.openapi.annotations.OpenApiParam; import io.javalin.plugin.openapi.annotations.OpenApiResponse; import java.util.List; -import com.google.common.flogger.FluentLogger; import javax.servlet.http.HttpServletResponse; import org.jooq.DSLContext; public class ParametersController implements CrudHandler { - private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private final MetricRegistry metrics; private final Histogram requestResultSize; @@ -121,10 +119,6 @@ public void getAll(Context ctx) { ctx.result(results); addDeprecatedContentTypeWarning(ctx, contentType); requestResultSize.update(results.length()); - } catch (Exception ex) { - CdaError re = new CdaError("Failed to process request"); - logger.atSevere().withCause(ex).log("%s", re); - ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } } diff --git a/cwms-data-api/src/main/java/cwms/cda/api/SpecifiedLevelController.java b/cwms-data-api/src/main/java/cwms/cda/api/SpecifiedLevelController.java index 31638a5d4..db2ac2b9f 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/SpecifiedLevelController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/SpecifiedLevelController.java @@ -41,7 +41,6 @@ import javax.servlet.http.HttpServletResponse; import java.util.List; -import com.google.common.flogger.FluentLogger; import static com.codahale.metrics.MetricRegistry.name; import static cwms.cda.api.Controllers.*; @@ -49,7 +48,6 @@ public class SpecifiedLevelController implements CrudHandler { - private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private static final String TAG = "Levels"; private final MetricRegistry metrics; @@ -110,11 +108,6 @@ public void getAll(Context ctx) { ctx.result(result); requestResultSize.update(result.length()); ctx.status(HttpServletResponse.SC_OK); - } catch (Exception ex) { - CdaError re = - new CdaError("Failed to process request: " + ex.getLocalizedMessage()); - logger.atSevere().withCause(ex).log("%s", re); - ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } } diff --git a/cwms-data-api/src/main/java/cwms/cda/api/TextTimeSeriesController.java b/cwms-data-api/src/main/java/cwms/cda/api/TextTimeSeriesController.java index 43893dd3a..521f4e945 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/TextTimeSeriesController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/TextTimeSeriesController.java @@ -30,6 +30,7 @@ import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.Timer; import cwms.cda.api.errors.CdaError; +import cwms.cda.api.errors.ErrorTraceSupport; import cwms.cda.data.dao.texttimeseries.TimeSeriesTextDao; import cwms.cda.data.dto.texttimeseries.TextTimeSeries; import cwms.cda.formatters.ContentType; @@ -144,8 +145,8 @@ public void getAll(@NotNull Context ctx) { ctx.status(HttpServletResponse.SC_OK); } catch (URISyntaxException | UnsupportedEncodingException ex) { - CdaError re = - new CdaError("Failed to process request: " + ex.getLocalizedMessage()); + CdaError re = ErrorTraceSupport.buildError(ctx, + "Failed to process request: " + ex.getLocalizedMessage(), ex); logger.atSevere().withCause(ex).log("%s", re); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } diff --git a/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesController.java b/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesController.java index 6a7841ff2..580e82cba 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesController.java @@ -47,6 +47,7 @@ import com.codahale.metrics.Timer; import cwms.cda.api.enums.UnitSystem; import cwms.cda.api.errors.CdaError; +import cwms.cda.api.errors.ErrorTraceSupport; import cwms.cda.api.errors.NotFoundException; import cwms.cda.data.dao.JooqDao; import cwms.cda.data.dao.StoreRule; @@ -222,7 +223,7 @@ public void create(@NotNull Context ctx) { dao.create(timeSeries, createAsLrts, storeRule, overrideProtection, vd); ctx.status(HttpServletResponse.SC_OK); } catch (DataAccessException | IOException ex) { - CdaError re = new CdaError("Internal Error"); + CdaError re = ErrorTraceSupport.buildError(ctx, "Internal Error", ex); logger.atSevere().withCause(ex).log("%s", re.toString()); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } @@ -638,7 +639,7 @@ public void update(@NotNull Context ctx, @NotNull String id) { ctx.status(HttpServletResponse.SC_OK); } catch (DataAccessException | IOException ex) { - CdaError re = new CdaError("Internal Error"); + CdaError re = ErrorTraceSupport.buildError(ctx, "Internal Error", ex); logger.atSevere().withCause(ex).log("%s", re.toString()); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } diff --git a/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesIdentifierDescriptorController.java b/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesIdentifierDescriptorController.java index 142a430c6..f9958a431 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesIdentifierDescriptorController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/TimeSeriesIdentifierDescriptorController.java @@ -31,6 +31,7 @@ import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.Timer; import cwms.cda.api.errors.CdaError; +import cwms.cda.api.errors.ErrorTraceSupport; import cwms.cda.data.dao.JooqDao; import cwms.cda.data.dao.TimeSeriesIdentifierDescriptorDao; import cwms.cda.data.dto.TimeSeriesIdentifierDescriptor; @@ -343,7 +344,7 @@ public void delete(@NotNull Context ctx, @NotNull String timeseriesId) { ctx.status(HttpServletResponse.SC_OK); } catch (DataAccessException ex) { - CdaError re = new CdaError("Internal Error"); + CdaError re = ErrorTraceSupport.buildError(ctx, "Internal Error", ex); logger.atSevere().withCause(ex).log("%s", re.toString()); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } diff --git a/cwms-data-api/src/main/java/cwms/cda/api/TimeZoneController.java b/cwms-data-api/src/main/java/cwms/cda/api/TimeZoneController.java index 44cf6d9ff..fa1ca5c6f 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/TimeZoneController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/TimeZoneController.java @@ -30,13 +30,10 @@ import io.javalin.plugin.openapi.annotations.OpenApiParam; import io.javalin.plugin.openapi.annotations.OpenApiResponse; -import com.google.common.flogger.FluentLogger; import javax.servlet.http.HttpServletResponse; import org.jooq.DSLContext; public class TimeZoneController implements CrudHandler { - private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - private final MetricRegistry metrics; private final Histogram requestResultSize; @@ -121,10 +118,6 @@ public void getAll(Context ctx) { requestResultSize.update(results.length()); ctx.status(HttpServletResponse.SC_OK); ctx.result(results); - } catch (Exception ex) { - logger.atSevere().withCause(ex).log("Failed to process request"); - ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); - ctx.result("Failed to process request"); } } diff --git a/cwms-data-api/src/main/java/cwms/cda/api/UnitsController.java b/cwms-data-api/src/main/java/cwms/cda/api/UnitsController.java index 11f1b2aa8..89fb497ea 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/UnitsController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/UnitsController.java @@ -27,12 +27,10 @@ import io.javalin.plugin.openapi.annotations.OpenApiParam; import io.javalin.plugin.openapi.annotations.OpenApiResponse; import java.util.List; -import com.google.common.flogger.FluentLogger; import javax.servlet.http.HttpServletResponse; import org.jooq.DSLContext; public class UnitsController implements CrudHandler { - private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private final MetricRegistry metrics; private final Histogram requestResultSize; @@ -115,10 +113,6 @@ public void getAll(Context ctx) { ctx.result(results); addDeprecatedContentTypeWarning(ctx, contentType); requestResultSize.update(results.length()); - } catch (Exception ex) { - logger.atSevere().withCause(ex).log("Failed to process request"); - ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); - ctx.result("Failed to process request"); } } diff --git a/cwms-data-api/src/main/java/cwms/cda/api/errors/ErrorTraceSupport.java b/cwms-data-api/src/main/java/cwms/cda/api/errors/ErrorTraceSupport.java new file mode 100644 index 000000000..98f2d0cc1 --- /dev/null +++ b/cwms-data-api/src/main/java/cwms/cda/api/errors/ErrorTraceSupport.java @@ -0,0 +1,124 @@ +package cwms.cda.api.errors; + +import cwms.cda.data.dao.AuthDao; +import cwms.cda.features.CdaFeatures; +import cwms.cda.security.DataApiPrincipal; +import cwms.cda.security.Role; +import cwms.cda.spi.IdentityProvider; +import io.javalin.http.Context; +import java.io.PrintWriter; +import java.io.Serializable; +import java.io.StringWriter; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; +import org.togglz.core.context.FeatureContext; +import org.togglz.core.manager.FeatureManager; + +public final class ErrorTraceSupport { + public static final String STACK_TRACE_KEY = "stackTrace"; + public static final String STACK_TRACE_LINES_KEY = "stackTraceLines"; + private static final String DEV_MARKER = "dev"; + private static final Role CWMS_USER_ADMINS_ROLE = new Role("CWMS User Admins"); + + private ErrorTraceSupport() { + } + + public static CdaError buildError(Context ctx, String message, Throwable cause) { + Map details = buildDetails(ctx, Collections.emptyMap(), cause); + if (details.isEmpty()) { + return new CdaError(message); + } + return new CdaError(message, details); + } + + public static CdaError buildError(Context ctx, String message, String source, + Map details, Throwable cause) { + return new CdaError(message, source, buildDetails(ctx, details, cause)); + } + + static Map buildDetails(Context ctx, Map details, + Throwable cause) { + return buildDetails(details, cause, shouldIncludeStackTrace(ctx)); + } + + static Map buildDetails(Map details, + Throwable cause, boolean includeStackTrace) { + Map merged = new HashMap<>(); + if (details != null) { + merged.putAll(details); + } + if (cause != null && includeStackTrace) { + String stackTrace = stackTraceOf(cause); + merged.put(STACK_TRACE_KEY, stackTrace); + merged.put(STACK_TRACE_LINES_KEY, stackTraceLinesOf(stackTrace)); + } + return Collections.unmodifiableMap(merged); + } + + static boolean shouldIncludeStackTrace(Context ctx) { + if (localhostRequestOverrideEnabled(ctx)) { + return true; + } + return shouldIncludeStackTrace(resolvePrincipal(ctx).orElse(null), stackTraceFeatureEnabled()); + } + + static boolean shouldIncludeStackTrace(DataApiPrincipal principal, boolean stackTraceFeatureEnabled) { + return stackTraceFeatureEnabled && hasAdminRole(principal); + } + + static boolean localhostRequestOverrideEnabled(Context ctx) { + if (ctx == null || ctx.req == null) { + return false; + } + return localhostRequestOverrideEnabled(ctx.req.getServerName()); + } + + static boolean localhostRequestOverrideEnabled(String serverName) { + return serverName != null + && ("localhost".equalsIgnoreCase(serverName) + || "127.0.0.1".equals(serverName) + || "::1".equals(serverName)); + } + + static Optional resolvePrincipal(Context ctx) { + if (ctx == null) { + return Optional.empty(); + } + DataApiPrincipal attributePrincipal = ctx.attribute(AuthDao.DATA_API_PRINCIPAL); + if (attributePrincipal != null) { + return Optional.of(attributePrincipal); + } + DataApiPrincipal sessionPrincipal = ctx.sessionAttribute(IdentityProvider.PRINCIPAL_KEY); + return Optional.ofNullable(sessionPrincipal); + } + + static boolean hasAdminRole(DataApiPrincipal principal) { + return principal != null + && principal.getRoles().contains(CWMS_USER_ADMINS_ROLE); + } + + static boolean stackTraceFeatureEnabled() { + try { + FeatureManager featureManager = FeatureContext.getFeatureManager(); + return featureManager.isActive(CdaFeatures.INCLUDE_ERROR_STACK_TRACES); + } catch (Throwable ignore) { + return false; + } + } + + private static String stackTraceOf(Throwable cause) { + StringWriter sw = new StringWriter(); + cause.printStackTrace(new PrintWriter(sw)); + return sw.toString(); + } + + private static ArrayList stackTraceLinesOf(String stackTrace) { + ArrayList lines = new ArrayList<>(); + Collections.addAll(lines, stackTrace.split("\\R")); + return lines; + } + +} diff --git a/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingController.java b/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingController.java index 365816242..763acee65 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingController.java @@ -62,6 +62,7 @@ import com.fasterxml.jackson.dataformat.xml.XmlMapper; import cwms.cda.api.Controllers; import cwms.cda.api.errors.CdaError; +import cwms.cda.api.errors.ErrorTraceSupport; import cwms.cda.data.dao.JsonRatingUtils; import cwms.cda.data.dao.RatingDao; import cwms.cda.data.dao.RatingSetDao; @@ -171,11 +172,13 @@ public void create(@NotNull Context ctx) { StatusResponse re = new StatusResponse(RatingDao.extractOfficeFromXml(ratingSet), "Rating Set successfully stored to CWMS."); ctx.status(HttpServletResponse.SC_CREATED).json(re); } catch (IOException ex) { - CdaError re = new CdaError("Failed to process request to update RatingSet"); + CdaError re = ErrorTraceSupport.buildError(ctx, + "Failed to process request to update RatingSet", ex); logger.atSevere().withCause(ex).log("%s", re); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } catch (RatingException ex) { - CdaError re = new CdaError("Failed to process request to update RatingSet: " + ex.getLocalizedMessage()); + CdaError re = ErrorTraceSupport.buildError(ctx, + "Failed to process request to update RatingSet: " + ex.getLocalizedMessage(), ex); logger.atSevere().withCause(ex).log("%s", re); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } @@ -492,14 +495,14 @@ private String getRatingSetString(Context ctx, RatingSet.DatabaseLoadMethod meth ctx.status(HttpCode.NOT_FOUND); } } catch (RatingException e) { - CdaError re = - new CdaError("Failed to process request to retrieve RatingSet"); + CdaError re = ErrorTraceSupport.buildError(ctx, + "Failed to process request to retrieve RatingSet", e); logger.atSevere().withCause(e).log("%s", re); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); ctx.json(re); } catch (IOException e) { - CdaError re = - new CdaError("Failed to process request to retrieve RatingSet"); + CdaError re = ErrorTraceSupport.buildError(ctx, + "Failed to process request to retrieve RatingSet", e); logger.atSevere().withCause(e).log("%s", re); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } @@ -580,11 +583,13 @@ public void update(@NotNull Context ctx, @NotNull String ratingId) { StatusResponse re = new StatusResponse(RatingDao.extractOfficeFromXml(ratingSet), "Updated RatingSet"); ctx.status(HttpServletResponse.SC_OK).json(re); } catch (IOException ex) { - CdaError re = new CdaError("Failed to process request to update RatingSet"); + CdaError re = ErrorTraceSupport.buildError(ctx, + "Failed to process request to update RatingSet", ex); logger.atSevere().withCause(ex).log("%s", re); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } catch (RatingException ex) { - CdaError re = new CdaError("Failed to process request to update RatingSet: " + ex.getLocalizedMessage()); + CdaError re = ErrorTraceSupport.buildError(ctx, + "Failed to process request to update RatingSet: " + ex.getLocalizedMessage(), ex); logger.atSevere().withCause(ex).log("%s", re); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } diff --git a/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingMetadataController.java b/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingMetadataController.java index ce95a2f17..bd684d0e9 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingMetadataController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingMetadataController.java @@ -167,11 +167,6 @@ public void getAll(Context ctx) { requestResultSize.update(result.length()); ctx.status(HttpServletResponse.SC_OK); - } catch (Exception ex) { - CdaError re = - new CdaError("Failed to process request: " + ex.getLocalizedMessage()); - logger.atSevere().withCause(ex).log("%s", re); - ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } } diff --git a/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingSpecController.java b/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingSpecController.java index 63f157b45..41eb577b5 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingSpecController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingSpecController.java @@ -138,11 +138,6 @@ public void getAll(Context ctx) { ctx.result(result); requestResultSize.update(result.length()); ctx.status(HttpServletResponse.SC_OK); - } catch (Exception ex) { - CdaError re = - new CdaError("Failed to process request: " + ex.getLocalizedMessage()); - logger.atSevere().withCause(ex).log("%s", re); - ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } } diff --git a/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingTemplateController.java b/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingTemplateController.java index a56b6ed50..6f55c1039 100644 --- a/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingTemplateController.java +++ b/cwms-data-api/src/main/java/cwms/cda/api/rating/RatingTemplateController.java @@ -134,11 +134,6 @@ public void getAll(Context ctx) { String result = Formats.format(contentType, ratingTemplates); ctx.result(result); requestResultSize.update(result.length()); - } catch (Exception ex) { - CdaError re = - new CdaError("Failed to process request: " + ex.getLocalizedMessage()); - logger.atSevere().withCause(ex).log("%s", re); - ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } } diff --git a/cwms-data-api/src/main/java/cwms/cda/features/CdaFeatures.java b/cwms-data-api/src/main/java/cwms/cda/features/CdaFeatures.java index fa2a662ef..c8bcbd3dd 100644 --- a/cwms-data-api/src/main/java/cwms/cda/features/CdaFeatures.java +++ b/cwms-data-api/src/main/java/cwms/cda/features/CdaFeatures.java @@ -5,5 +5,7 @@ public enum CdaFeatures implements Feature { @Label("Use object-storage backed Blob DAO in BlobController") - USE_OBJECT_STORAGE_BLOBS + USE_OBJECT_STORAGE_BLOBS, + @Label("Include stack traces in JSON error responses for authorized debug requests") + INCLUDE_ERROR_STACK_TRACES } diff --git a/cwms-data-api/src/main/resources/features.properties b/cwms-data-api/src/main/resources/features.properties index 94eeb1556..c7c091f5d 100644 --- a/cwms-data-api/src/main/resources/features.properties +++ b/cwms-data-api/src/main/resources/features.properties @@ -1 +1,2 @@ -USE_OBJECT_STORAGE_BLOBS=false \ No newline at end of file +USE_OBJECT_STORAGE_BLOBS=false +INCLUDE_ERROR_STACK_TRACES=false diff --git a/cwms-data-api/src/test/java/cwms/cda/api/errors/ErrorTraceSupportTest.java b/cwms-data-api/src/test/java/cwms/cda/api/errors/ErrorTraceSupportTest.java new file mode 100644 index 000000000..dce8e4352 --- /dev/null +++ b/cwms-data-api/src/test/java/cwms/cda/api/errors/ErrorTraceSupportTest.java @@ -0,0 +1,80 @@ +package cwms.cda.api.errors; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertTrue; +import cwms.cda.security.DataApiPrincipal; +import cwms.cda.security.Role; +import java.io.Serializable; +import java.util.Map; +import java.util.Set; +import org.junit.jupiter.api.Test; + +class ErrorTraceSupportTest { + + @Test + void includesStackTraceWhenFeatureEnabledForAdminUser() { + DataApiPrincipal principal = new DataApiPrincipal("admin-user", + Set.of(new Role("CWMS User Admins"))); + + Map details = ErrorTraceSupport.buildDetails(Map.of(), + new IllegalStateException("boom"), + ErrorTraceSupport.shouldIncludeStackTrace(principal, true)); + + assertTrue(details.containsKey(ErrorTraceSupport.STACK_TRACE_KEY)); + assertTrue(details.containsKey(ErrorTraceSupport.STACK_TRACE_LINES_KEY)); + assertTrue(details.get(ErrorTraceSupport.STACK_TRACE_KEY).toString() + .contains("IllegalStateException")); + assertTrue(assertInstanceOf(Iterable.class, details.get(ErrorTraceSupport.STACK_TRACE_LINES_KEY)) + .iterator().next().toString().contains("IllegalStateException")); + } + + @Test + void featureEnablesStackTraceForUserAdminsRole() { + DataApiPrincipal principal = new DataApiPrincipal("admin-user", + Set.of(new Role("CWMS User Admins"))); + + assertTrue(ErrorTraceSupport.shouldIncludeStackTrace(principal, true)); + } + + @Test + void omitsStackTraceWhenFeatureDisabled() { + DataApiPrincipal principal = new DataApiPrincipal("admin-user", + Set.of(new Role("CWMS User Admins"))); + + Map details = ErrorTraceSupport.buildDetails(Map.of(), + new IllegalStateException("boom"), + ErrorTraceSupport.shouldIncludeStackTrace(principal, false)); + + assertFalse(details.containsKey(ErrorTraceSupport.STACK_TRACE_KEY)); + assertFalse(details.containsKey(ErrorTraceSupport.STACK_TRACE_LINES_KEY)); + } + + @Test + void omitsStackTraceForNonAdminUser() { + DataApiPrincipal principal = new DataApiPrincipal("normal-user", + Set.of(new Role("CWMS Users"))); + + assertFalse(ErrorTraceSupport.shouldIncludeStackTrace(principal, true)); + } + + @Test + void omitsStackTraceForGuestUser() { + assertFalse(ErrorTraceSupport.shouldIncludeStackTrace(null, true)); + } + + @Test + void omitsStackTraceForUndefinedCwmsAdminRole() { + DataApiPrincipal principal = new DataApiPrincipal("admin-user", + Set.of(new Role("CWMS Admin"))); + + assertFalse(ErrorTraceSupport.shouldIncludeStackTrace(principal, true)); + } + + @Test + void localhostRequestEnablesStackTraceWithoutAuth() { + assertTrue(ErrorTraceSupport.localhostRequestOverrideEnabled("localhost")); + assertTrue(ErrorTraceSupport.localhostRequestOverrideEnabled("127.0.0.1")); + assertFalse(ErrorTraceSupport.localhostRequestOverrideEnabled("example.com")); + } +} diff --git a/cwms-data-api/src/test/java/cwms/cda/features/CdaFeatureManagerProviderTest.java b/cwms-data-api/src/test/java/cwms/cda/features/CdaFeatureManagerProviderTest.java index 7a2f68a2a..8506eec9e 100644 --- a/cwms-data-api/src/test/java/cwms/cda/features/CdaFeatureManagerProviderTest.java +++ b/cwms-data-api/src/test/java/cwms/cda/features/CdaFeatureManagerProviderTest.java @@ -61,6 +61,23 @@ void testUseObjectStorageBlobsFeature() throws IOException { assertTrue(manager.isActive(CdaFeatures.USE_OBJECT_STORAGE_BLOBS)); } + @Test + void testIncludeErrorStackTracesFeature() throws IOException { + File tempFile = Files.createTempFile("features", ".properties").toFile(); + tempFile.deleteOnExit(); + + try (FileWriter writer = new FileWriter(tempFile)) { + writer.write(CdaFeatures.INCLUDE_ERROR_STACK_TRACES.name() + " = true"); + } + + System.setProperty(CdaFeatureManagerProvider.PROPERTIES_FILE, tempFile.getAbsolutePath()); + + CdaFeatureManagerProvider provider = new CdaFeatureManagerProvider(); + FeatureManager manager = provider.getFeatureManager(); + + assertTrue(manager.isActive(CdaFeatures.INCLUDE_ERROR_STACK_TRACES)); + } + @Test void testFeatureDisabledByDefault() throws IOException { File tempFile = Files.createTempFile("features_disabled", ".properties").toFile(); @@ -73,5 +90,6 @@ void testFeatureDisabledByDefault() throws IOException { FeatureManager manager = provider.getFeatureManager(); assertFalse(manager.isActive(CdaFeatures.USE_OBJECT_STORAGE_BLOBS)); + assertFalse(manager.isActive(CdaFeatures.INCLUDE_ERROR_STACK_TRACES)); } } diff --git a/docker-compose.README.md b/docker-compose.README.md index 5a0be00da..5dde9ba3a 100644 --- a/docker-compose.README.md +++ b/docker-compose.README.md @@ -45,3 +45,20 @@ The following users and permissions are available: Traefik uses port 8081 by default, if this conflicts with existing services on your machine it can be changed by setting the APP_PORT variable. + +## Local error trace debugging + +The API supports conditional stack traces in JSON error responses through +`cwms.cda.api.errors.ErrorTraceSupport`. + +Stack-trace exposure is controlled through the Togglz feature flag +`INCLUDE_ERROR_STACK_TRACES`. + +For shared environments, enable that feature in the active `features.properties` and the API will +include traces only for authenticated users with the `CWMS User Admins` role. + +For local Docker Compose or other localhost testing, requests to `localhost`, `127.0.0.1`, and +`::1` are still treated as local debug requests even when the feature is disabled. When enabled, +error responses include `details.stackTrace` along with the existing `incidentIdentifier`. + +Response stack traces should *not* be enabled broadly for production traffic.