diff --git a/compose_files/sql/users.sql b/compose_files/sql/users.sql index c2a34761ae..9eac95cc17 100644 --- a/compose_files/sql/users.sql +++ b/compose_files/sql/users.sql @@ -9,6 +9,7 @@ begin cwms_sec.update_edipi('l2hectest', 1234567890); cwms_sec.add_user_to_group('l2hectest', 'All Users', 'SPK'); cwms_sec.add_user_to_group('l2hectest', 'CWMS Users', 'SPK'); + cwms_sec.add_user_to_group('l2hectest', 'SHOW STACK TRACE', 'SPK'); cwms_sec.add_user_to_group('l2hectest', 'TS ID Creator', 'SPK'); cwms_sec.add_cwms_user('l1hectest', null, 'SPL'); -- intentionally no extra permissions. @@ -47,6 +48,7 @@ begin cwms_sec.add_user_to_group('m5testadmin','All Users', 'LRL'); cwms_sec.add_user_to_group('m5testadmin','CWMS Users', 'LRL'); cwms_sec.add_user_to_group('m5testadmin','CWMS User Admins', 'LRL'); + cwms_sec.add_user_to_group('m5testadmin','SHOW STACK TRACE', 'LRL'); end; / diff --git a/compose_files/togglz/features.properties b/compose_files/togglz/features.properties new file mode 100644 index 0000000000..663171a401 --- /dev/null +++ b/compose_files/togglz/features.properties @@ -0,0 +1,2 @@ +USE_OBJECT_STORAGE_BLOBS=false +INCLUDE_ERROR_STACK_TRACES=true 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 2f838bd155..9ecad0268f 100644 --- a/cwms-data-api/src/main/java/cwms/cda/ApiServlet.java +++ b/cwms-data-api/src/main/java/cwms/cda/ApiServlet.java @@ -107,6 +107,7 @@ import cwms.cda.api.enums.UnitSystem; import cwms.cda.api.errors.ApplicationException; import cwms.cda.api.errors.CdaError; +import cwms.cda.api.errors.ExceptionTraceSupport; import cwms.cda.api.location.kind.GateChangeCreateController; import cwms.cda.api.location.kind.GateChangeDeleteController; import cwms.cda.api.location.kind.GateChangeGetAllController; @@ -357,31 +358,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 = ExceptionTraceSupport.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 = ExceptionTraceSupport.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 = ExceptionTraceSupport.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 = ExceptionTraceSupport.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 = ExceptionTraceSupport.buildError(ctx, e.getMessage(), e); ctx.status(HttpServletResponse.SC_BAD_REQUEST).json(re); }) .exception(DataAccessException.class, (e, ctx) -> { @@ -395,7 +397,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 = ExceptionTraceSupport.buildError(ctx, "Database Error", e); logger.atWarning().withCause(e).log("error on request[%s]: %s", errResponse.getIncidentIdentifier(), ctx.req.getRequestURI()); ctx.status(500); @@ -403,7 +405,7 @@ public void init() { ctx.json(errResponse); }) .exception(Exception.class, (e, ctx) -> { - CdaError errResponse = new CdaError("System Error"); + CdaError errResponse = ExceptionTraceSupport.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 c8bdf4f4a6..bb36013228 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.ExceptionTraceSupport; 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 = ExceptionTraceSupport.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 8f6f1d0e1e..7e866b7aaa 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.ExceptionTraceSupport; 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 = ExceptionTraceSupport.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 f1610e113d..fe4c422b2d 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.ExceptionTraceSupport; 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 = ExceptionTraceSupport.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 = ExceptionTraceSupport.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 = ExceptionTraceSupport.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 b3a103d451..b4fa06aae2 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 31638a5d43..db2ac2b9f6 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 43893dd3a9..993cda2a88 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.ExceptionTraceSupport; 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 = ExceptionTraceSupport.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 d729568380..634f486860 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 @@ -9,6 +9,7 @@ import com.google.common.flogger.FluentLogger; import cwms.cda.api.enums.UnitSystem; import cwms.cda.api.errors.CdaError; +import cwms.cda.api.errors.ExceptionTraceSupport; import cwms.cda.api.errors.NotFoundException; import cwms.cda.data.dao.JooqDao; import cwms.cda.data.dao.StoreRule; @@ -185,7 +186,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 = ExceptionTraceSupport.buildError(ctx, "Internal Error", ex); logger.atSevere().withCause(ex).log("%s", re.toString()); ctx.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR).json(re); } @@ -631,7 +632,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 = ExceptionTraceSupport.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 142a430c66..fcd567cb61 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 @@ -55,7 +55,6 @@ import javax.servlet.http.HttpServletResponse; import org.jetbrains.annotations.NotNull; import org.jooq.DSLContext; -import org.jooq.exception.DataAccessException; public class TimeSeriesIdentifierDescriptorController implements CrudHandler { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); @@ -341,11 +340,6 @@ public void delete(@NotNull Context ctx, @NotNull String timeseriesId) { dao.delete(office, timeseriesId, method); ctx.status(HttpServletResponse.SC_OK); - - } catch (DataAccessException ex) { - CdaError re = new CdaError("Internal Error"); - 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 44cf6d9ff3..fa1ca5c6f2 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 11f1b2aa86..89fb497ead 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/ExceptionTraceSupport.java b/cwms-data-api/src/main/java/cwms/cda/api/errors/ExceptionTraceSupport.java new file mode 100644 index 0000000000..af08693af9 --- /dev/null +++ b/cwms-data-api/src/main/java/cwms/cda/api/errors/ExceptionTraceSupport.java @@ -0,0 +1,101 @@ +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 ExceptionTraceSupport { + public static final String STACK_TRACE_LINES_KEY = "stackTraceLines"; + static final String SHOW_STACK_TRACE_ROLE_NAME = "SHOW STACK TRACE"; + private static final Role SHOW_STACK_TRACE_ROLE = new Role(SHOW_STACK_TRACE_ROLE_NAME); + + private ExceptionTraceSupport() { + } + + 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) { + merged.put(STACK_TRACE_LINES_KEY, stackTraceLinesOf(cause)); + } + return Collections.unmodifiableMap(merged); + } + + static boolean shouldIncludeStackTrace(Context ctx) { + return stackTraceFeatureEnabled() + && resolvePrincipal(ctx).map(ExceptionTraceSupport::hasStackTraceRole).orElse(false); + } + + static boolean shouldIncludeStackTrace(DataApiPrincipal principal, boolean stackTraceFeatureEnabled) { + return stackTraceFeatureEnabled && hasStackTraceRole(principal); + } + + 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 hasStackTraceRole(DataApiPrincipal principal) { + return principal != null + && principal.getRoles().contains(SHOW_STACK_TRACE_ROLE); + } + + static boolean stackTraceFeatureEnabled() { + try { + FeatureManager featureManager = FeatureContext.getFeatureManager(); + return featureManager.isActive(CdaFeatures.INCLUDE_ERROR_STACK_TRACES); + } catch (Exception ignore) { + return false; + } + } + + private static ArrayList stackTraceLinesOf(Throwable cause) { + StringWriter sw = new StringWriter(); + cause.printStackTrace(new PrintWriter(sw)); + ArrayList lines = new ArrayList<>(); + Collections.addAll(lines, sw.toString().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 3658162424..71e4f02b57 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.ExceptionTraceSupport; 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 = ExceptionTraceSupport.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 = ExceptionTraceSupport.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 = ExceptionTraceSupport.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 = ExceptionTraceSupport.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 = ExceptionTraceSupport.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 = ExceptionTraceSupport.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 ce95a2f178..bd684d0e91 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 63f157b450..41eb577b58 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 a56b6ed50c..6f55c1039b 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/data/dao/TimeSeriesDaoImpl.java b/cwms-data-api/src/main/java/cwms/cda/data/dao/TimeSeriesDaoImpl.java index 2102f58eb5..2518ca0b82 100644 --- a/cwms-data-api/src/main/java/cwms/cda/data/dao/TimeSeriesDaoImpl.java +++ b/cwms-data-api/src/main/java/cwms/cda/data/dao/TimeSeriesDaoImpl.java @@ -74,6 +74,7 @@ import java.util.stream.Collectors; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.jooq.Record; import org.jooq.*; import org.jooq.conf.ParamType; import org.jooq.exception.DataAccessException; 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 7af770c7cf..86c1f152fe 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 @@ -6,6 +6,10 @@ public enum CdaFeatures implements Feature { @Label("Use object-storage backed Blob DAO in BlobController") USE_OBJECT_STORAGE_BLOBS, + + @Label("Include stack traces in JSON error responses for authorized debug requests") + INCLUDE_ERROR_STACK_TRACES, + @Label("Re-enable non-hash key support") AUTH_RE_ENABLE_NON_HASH_KEY_SUPPORT } diff --git a/cwms-data-api/src/main/resources/features.properties b/cwms-data-api/src/main/resources/features.properties index 8b5d6a3921..c683b28495 100644 --- a/cwms-data-api/src/main/resources/features.properties +++ b/cwms-data-api/src/main/resources/features.properties @@ -1,2 +1,3 @@ USE_OBJECT_STORAGE_BLOBS=false -AUTH_RE_ENABLE_NON_HASH_KEY_SUPPORT=false \ No newline at end of file +INCLUDE_ERROR_STACK_TRACES=false +AUTH_RE_ENABLE_NON_HASH_KEY_SUPPORT=false diff --git a/cwms-data-api/src/test/java/cwms/cda/api/errors/ExceptionTraceRenderingTestIT.java b/cwms-data-api/src/test/java/cwms/cda/api/errors/ExceptionTraceRenderingTestIT.java new file mode 100644 index 0000000000..893697738b --- /dev/null +++ b/cwms-data-api/src/test/java/cwms/cda/api/errors/ExceptionTraceRenderingTestIT.java @@ -0,0 +1,138 @@ +package cwms.cda.api.errors; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.fasterxml.jackson.databind.JsonNode; +import cwms.cda.data.dao.AuthDao; +import cwms.cda.features.CdaFeatures; +import cwms.cda.formatters.json.JsonV2; +import cwms.cda.security.DataApiPrincipal; +import cwms.cda.security.Role; +import fixtures.TestHttpServletResponse; +import fixtures.TestServletInputStream; +import io.javalin.core.util.Header; +import io.javalin.http.Context; +import io.javalin.http.HandlerType; +import io.javalin.http.util.ContextUtil; +import io.javalin.plugin.json.JavalinJackson; +import io.javalin.plugin.json.JsonMapperKt; +import java.lang.reflect.Proxy; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; +import javax.servlet.http.HttpServletRequest; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.togglz.core.context.FeatureContext; +import org.togglz.core.manager.FeatureManager; + +@Tag("integration") +class ExceptionTraceRenderingTestIT { + private static boolean wasStackTraceFeatureActive; + + @BeforeAll + static void enableStackTraceFeature() { + FeatureManager featureManager = FeatureContext.getFeatureManager(); + wasStackTraceFeatureActive = featureManager.isActive(CdaFeatures.INCLUDE_ERROR_STACK_TRACES); + featureManager.enable(CdaFeatures.INCLUDE_ERROR_STACK_TRACES); + } + + @AfterAll + static void restoreFeature() { + FeatureManager featureManager = FeatureContext.getFeatureManager(); + if (wasStackTraceFeatureActive) { + featureManager.enable(CdaFeatures.INCLUDE_ERROR_STACK_TRACES); + } else { + featureManager.disable(CdaFeatures.INCLUDE_ERROR_STACK_TRACES); + } + } + + @Test + void rendersStackTraceLinesInJsonForAuthenticatedTraceUser() throws Exception { + TestHttpServletResponse response = new TestHttpServletResponse(); + Context ctx = context(response); + ctx.attribute(AuthDao.DATA_API_PRINCIPAL, new DataApiPrincipal("trace-user", + Set.of(new Role(ExceptionTraceSupport.SHOW_STACK_TRACE_ROLE_NAME)))); + + CdaError error = ExceptionTraceSupport.buildError(ctx, "System Error", + new IllegalStateException("Unable to retrieve requested data")); + ctx.status(500).json(error); + + String renderedJson = ctx.resultString(); + assertNotNull(renderedJson); + JsonNode json = JsonV2.buildObjectMapper().readTree(renderedJson); + JsonNode stackTraceLines = json.at("/details/" + ExceptionTraceSupport.STACK_TRACE_LINES_KEY); + if (stackTraceLines.isMissingNode()) { + stackTraceLines = json.at("/details/stack-trace-lines"); + } + + assertFalse(json.at("/details/stackTrace").isValueNode()); + assertTrue(stackTraceLines.isArray(), renderedJson); + assertTrue(stackTraceLines.get(0).asText() + .contains("IllegalStateException: Unable to retrieve requested data")); + } + + private Context context(TestHttpServletResponse response) throws Exception { + HttpServletRequest request = request(); + + Map attributes = new HashMap<>(); + attributes.put(ContextUtil.maxRequestSizeKey, Integer.MAX_VALUE); + attributes.put(JsonMapperKt.JSON_MAPPER_KEY, new JavalinJackson()); + + return ContextUtil.init(request, response, "*", new HashMap<>(), HandlerType.GET, attributes); + } + + private HttpServletRequest request() { + Map requestAttributes = new HashMap<>(); + return (HttpServletRequest) Proxy.newProxyInstance(HttpServletRequest.class.getClassLoader(), + new Class[]{HttpServletRequest.class}, + (proxy, method, args) -> { + switch (method.getName()) { + case "getAttribute": + return requestAttributes.get(args[0]); + case "setAttribute": + requestAttributes.put((String) args[0], args[1]); + return null; + case "getInputStream": + return new TestServletInputStream(""); + case "getRequestURI": + return "/error-trace-test"; + case "getHeader": + return Header.ACCEPT.equals(args[0]) ? "application/json" : null; + case "getMethod": + return "GET"; + case "getParameterMap": + return Map.of(); + default: + return defaultValue(method.getReturnType()); + } + }); + } + + private Object defaultValue(Class returnType) { + if (!returnType.isPrimitive()) { + return null; + } else if (returnType == boolean.class) { + return false; + } else if (returnType == int.class) { + return 0; + } else if (returnType == long.class) { + return 0L; + } else if (returnType == double.class) { + return 0D; + } else if (returnType == float.class) { + return 0F; + } else if (returnType == byte.class) { + return (byte) 0; + } else if (returnType == short.class) { + return (short) 0; + } else if (returnType == char.class) { + return (char) 0; + } + return null; + } +} diff --git a/cwms-data-api/src/test/java/cwms/cda/api/errors/ExceptionTraceSupportTest.java b/cwms-data-api/src/test/java/cwms/cda/api/errors/ExceptionTraceSupportTest.java new file mode 100644 index 0000000000..b566326e13 --- /dev/null +++ b/cwms-data-api/src/test/java/cwms/cda/api/errors/ExceptionTraceSupportTest.java @@ -0,0 +1,70 @@ +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 ExceptionTraceSupportTest { + + @Test + void includesStackTraceLinesWhenFeatureEnabledForTraceRole() { + DataApiPrincipal principal = new DataApiPrincipal("trace-user", + Set.of(new Role(ExceptionTraceSupport.SHOW_STACK_TRACE_ROLE_NAME))); + + Map details = ExceptionTraceSupport.buildDetails(Map.of(), + new IllegalStateException("Unable to retrieve requested data"), + ExceptionTraceSupport.shouldIncludeStackTrace(principal, true)); + + assertFalse(details.containsKey("stackTrace")); + assertTrue(details.containsKey(ExceptionTraceSupport.STACK_TRACE_LINES_KEY)); + assertTrue(assertInstanceOf(Iterable.class, details.get(ExceptionTraceSupport.STACK_TRACE_LINES_KEY)) + .iterator().next().toString().contains("IllegalStateException")); + } + + @Test + void featureEnablesStackTraceForTraceRole() { + DataApiPrincipal principal = new DataApiPrincipal("trace-user", + Set.of(new Role(ExceptionTraceSupport.SHOW_STACK_TRACE_ROLE_NAME))); + + assertTrue(ExceptionTraceSupport.shouldIncludeStackTrace(principal, true)); + } + + @Test + void omitsStackTraceWhenFeatureDisabled() { + DataApiPrincipal principal = new DataApiPrincipal("trace-user", + Set.of(new Role(ExceptionTraceSupport.SHOW_STACK_TRACE_ROLE_NAME))); + + Map details = ExceptionTraceSupport.buildDetails(Map.of(), + new IllegalStateException("Unable to retrieve requested data"), + ExceptionTraceSupport.shouldIncludeStackTrace(principal, false)); + + assertFalse(details.containsKey(ExceptionTraceSupport.STACK_TRACE_LINES_KEY)); + } + + @Test + void omitsStackTraceForDifferentRole() { + DataApiPrincipal principal = new DataApiPrincipal("normal-user", + Set.of(new Role("CWMS Users"))); + + assertFalse(ExceptionTraceSupport.shouldIncludeStackTrace(principal, true)); + } + + @Test + void omitsStackTraceForGuestUser() { + assertFalse(ExceptionTraceSupport.shouldIncludeStackTrace(null, true)); + } + + @Test + void omitsStackTraceForUserAdminsRoleWithoutTraceRole() { + DataApiPrincipal principal = new DataApiPrincipal("admin-user", + Set.of(new Role("CWMS User Admins"))); + + assertFalse(ExceptionTraceSupport.shouldIncludeStackTrace(principal, true)); + } +} 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 f30c847da9..b43b3c6661 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 @@ -63,6 +63,23 @@ void testUseObjectStorageBlobsFeature() throws IOException { assertTrue(manager.isActive(CdaFeatures.AUTH_RE_ENABLE_NON_HASH_KEY_SUPPORT)); } + @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(); @@ -75,6 +92,7 @@ void testFeatureDisabledByDefault() throws IOException { FeatureManager manager = provider.getFeatureManager(); assertFalse(manager.isActive(CdaFeatures.USE_OBJECT_STORAGE_BLOBS)); + assertFalse(manager.isActive(CdaFeatures.INCLUDE_ERROR_STACK_TRACES)); assertFalse(manager.isActive(CdaFeatures.AUTH_RE_ENABLE_NON_HASH_KEY_SUPPORT)); } } diff --git a/cwms-data-api/src/test/java/fixtures/TestAccounts.java b/cwms-data-api/src/test/java/fixtures/TestAccounts.java index 89c72a5098..d754c442f4 100644 --- a/cwms-data-api/src/test/java/fixtures/TestAccounts.java +++ b/cwms-data-api/src/test/java/fixtures/TestAccounts.java @@ -36,7 +36,7 @@ public enum KeyUser { GUEST("guest",null,null,null, null, null), // USED as marker label for tests SPK_NORMAL("l2hectest","l2hectest","1234567890","l2userkey","ATotallyRandomStringL2hectest","SPK", "CWMS Users", ApiServlet.CAC_USER), SPK_OTHER_NORMAL_SAME_ROLES("l2hectest8","l2hectest8","12345678908","l2userkey8","ATotallyRandomStringL2hectest8","SPK", "CWMS Users", ApiServlet.CAC_USER), - SPK_NORMAL2("l2hectest_vt","l2hectestvt","2345678901","l2userkey2","DiffrntStringL2hectest_vt","SPK", "CWMS Users", ApiServlet.CAC_USER, "CWMS User Admins"), + SPK_NORMAL2("l2hectest_vt","l2hectestvt","2345678901","l2userkey2","DiffrntStringL2hectest_vt","SPK", "CWMS Users", ApiServlet.CAC_USER, "CWMS User Admins", "SHOW STACK TRACE"), SWT_NORMAL("m5hectest","swt99db","1234567890","testkey2","ATotallyRandomStringM5hectest","SWT", "CWMS Users", ApiServlet.CAC_USER), SPK_NO_ROLES("user2","user2",null,"User2key","user2SEssion", "SPK"); diff --git a/cwms-data-api/src/test/resources/cwms/cda/data/sql/users.sql b/cwms-data-api/src/test/resources/cwms/cda/data/sql/users.sql index 8d6136b83f..177df8df2a 100644 --- a/cwms-data-api/src/test/resources/cwms/cda/data/sql/users.sql +++ b/cwms-data-api/src/test/resources/cwms/cda/data/sql/users.sql @@ -82,11 +82,13 @@ begin cwms_sec.add_user_to_group('l2hectest_vt','CWMS PD Users', 'HQ'); cwms_sec.add_user_to_group('l2hectest_vt','CWMS User Admins', 'HQ'); cwms_sec.add_user_to_group('l2hectest_vt','CWMS DBA Users', 'HQ'); + cwms_sec.add_user_to_group('l2hectest_vt','SHOW STACK TRACE', 'HQ'); cwms_sec.add_user_to_group('l2hectest_vt','All Users', 'SPK'); cwms_sec.add_user_to_group('l2hectest_vt','CWMS Users', 'SPK'); cwms_sec.add_user_to_group('l2hectest_vt','CWMS PD Users', 'SPK'); cwms_sec.add_user_to_group('l2hectest_vt','CWMS DBA Users', 'SPK'); cwms_sec.add_user_to_group('l2hectest_vt','CWMS User Admins', 'SPK'); + cwms_sec.add_user_to_group('l2hectest_vt','SHOW STACK TRACE', 'SPK'); cwms_sec.add_user_to_group('l2hectest_vt','TS ID Creator','SPK'); cwms_sec.add_cwms_user('m5hectest', NULL, 'SWT'); diff --git a/docker-compose.README.md b/docker-compose.README.md index 5a0be00da2..853e527016 100644 --- a/docker-compose.README.md +++ b/docker-compose.README.md @@ -45,3 +45,24 @@ 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.ExceptionTraceSupport`. + +Stack-trace exposure is controlled through the Togglz feature flag +`INCLUDE_ERROR_STACK_TRACES`. + +For shared environments, enable that feature through the active Togglz properties file configured +with `-Dproperties.file=...` and the API will include traces only for authenticated users with the +`SHOW STACK TRACE` role. + +The local Docker Compose setup enables that feature by default by mounting +`./compose_files/togglz/features.properties` into the API container and passing its path through +`JAVA_OPTS`. + +In that compose environment, `l2hectest` has the `SHOW STACK TRACE` role. When enabled, error +responses include `details.stackTraceLines` along with the existing `incidentIdentifier`. + +Response stack traces should *not* be enabled broadly for production traffic. diff --git a/docker-compose.yml b/docker-compose.yml index b602bf2159..fea4f2c13a 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -69,9 +69,10 @@ services: restart: unless-stopped volumes: - ./compose_files/pki/certs:/conf/ + - ./compose_files/togglz/features.properties:/conf/features.properties:ro - ./compose_files/tomcat/logging.properties:/usr/local/tomcat/conf/logging.properties:ro environment: - - JAVA_OPTS=-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=5005 + - JAVA_OPTS=-Dproperties.file=/conf/features.properties -agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=5005 - CDA_JDBC_DRIVER=oracle.jdbc.driver.OracleDriver - CDA_JDBC_URL=jdbc:oracle:thin:@db/FREEPDB1 - CDA_JDBC_USERNAME=q0webtest