From d4967cbfb25a9620c541340256da022722474e9f Mon Sep 17 00:00:00 2001 From: fzhao99 Date: Tue, 9 Jul 2024 14:46:35 -0400 Subject: [PATCH] break out okta endpoint from health check (#7891) * add in separate okta component * add in okta messages * add in tests * add in okta message * undo hardcode for test branch * fix frontend test * fix frontend test * make status check reflect api shape for okta * make status check reflect api shape for okta * make tests more readable and handle NPE in health check * make smoke test script include a check for okta * woops * weave in config values into endpoint Co-authored-by: Merethe Hansen * add in old api exception * status check reflect subapi endpoint (sorry merethe) * dedupe application.yaml --------- Co-authored-by: Merethe Hansen --- .../BackendAndDatabaseHealthIndicator.java | 14 ---- .../api/heathcheck/OktaHealthIndicator.java | 38 +++++++++++ .../idp/repository/DemoOktaRepository.java | 2 +- .../idp/repository/LiveOktaRepository.java | 3 +- backend/src/main/resources/application.yaml | 12 +++- ...BackendAndDatabaseHealthIndicatorTest.java | 10 --- .../healthcheck/OktaHealthIndicatorTest.java | 53 +++++++++++++++ .../test_util/SliceTestConfiguration.java | 2 + frontend/deploy-smoke.js | 24 ++++++- frontend/src/app/DeploySmokeTest.test.tsx | 36 +++++++--- frontend/src/app/DeploySmokeTest.tsx | 65 +++++++++++++++++-- .../src/app/deploySmokeTestTestConstants.ts | 54 +++++++++++++++ 12 files changed, 266 insertions(+), 47 deletions(-) create mode 100644 backend/src/main/java/gov/cdc/usds/simplereport/api/heathcheck/OktaHealthIndicator.java create mode 100644 backend/src/test/java/gov/cdc/usds/simplereport/api/healthcheck/OktaHealthIndicatorTest.java create mode 100644 frontend/src/app/deploySmokeTestTestConstants.ts diff --git a/backend/src/main/java/gov/cdc/usds/simplereport/api/heathcheck/BackendAndDatabaseHealthIndicator.java b/backend/src/main/java/gov/cdc/usds/simplereport/api/heathcheck/BackendAndDatabaseHealthIndicator.java index bb077eadff..7a7b8e403b 100644 --- a/backend/src/main/java/gov/cdc/usds/simplereport/api/heathcheck/BackendAndDatabaseHealthIndicator.java +++ b/backend/src/main/java/gov/cdc/usds/simplereport/api/heathcheck/BackendAndDatabaseHealthIndicator.java @@ -1,8 +1,6 @@ package gov.cdc.usds.simplereport.api.heathcheck; -import com.okta.sdk.resource.client.ApiException; import gov.cdc.usds.simplereport.db.repository.FeatureFlagRepository; -import gov.cdc.usds.simplereport.idp.repository.OktaRepository; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.hibernate.exception.JDBCConnectionException; @@ -15,28 +13,16 @@ @RequiredArgsConstructor public class BackendAndDatabaseHealthIndicator implements HealthIndicator { private final FeatureFlagRepository _ffRepo; - private final OktaRepository _oktaRepo; - public static final String ACTIVE_LITERAL = "ACTIVE"; @Override public Health health() { try { _ffRepo.findAll(); - String oktaStatus = _oktaRepo.getApplicationStatusForHealthCheck(); - - if (!ACTIVE_LITERAL.equals(oktaStatus)) { - log.info("Okta status didn't return ACTIVE, instead returned " + oktaStatus); - return Health.down().build(); - } return Health.up().build(); // reach into the ff repository returned a bad value or db connection issue respectively } catch (IllegalArgumentException | JDBCConnectionException e) { return Health.down().build(); - } catch (ApiException e) { - // Okta API call errored - log.info(e.getMessage()); - return Health.down().build(); } } } diff --git a/backend/src/main/java/gov/cdc/usds/simplereport/api/heathcheck/OktaHealthIndicator.java b/backend/src/main/java/gov/cdc/usds/simplereport/api/heathcheck/OktaHealthIndicator.java new file mode 100644 index 0000000000..cc9f264a07 --- /dev/null +++ b/backend/src/main/java/gov/cdc/usds/simplereport/api/heathcheck/OktaHealthIndicator.java @@ -0,0 +1,38 @@ +package gov.cdc.usds.simplereport.api.heathcheck; + +import com.okta.sdk.resource.client.ApiException; +import gov.cdc.usds.simplereport.idp.repository.OktaRepository; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.springframework.boot.actuate.health.Health; +import org.springframework.boot.actuate.health.HealthIndicator; +import org.springframework.stereotype.Component; + +@Component +@Slf4j +@RequiredArgsConstructor +public class OktaHealthIndicator implements HealthIndicator { + private final OktaRepository _oktaRepo; + public static final String ACTIVE_LITERAL = "ACTIVE"; + + @Override + public Health health() { + Health.Builder oktaDegradedWarning = Health.status("OKTA_DEGRADED"); + try { + String oktaStatus = _oktaRepo.getApplicationStatusForHealthCheck(); + if (!ACTIVE_LITERAL.equals(oktaStatus)) { + log.info("Okta status didn't return ACTIVE, instead returned " + oktaStatus); + return oktaDegradedWarning.build(); + } + } catch (NullPointerException e) { + log.info("Call to Okta repository status returned null"); + return oktaDegradedWarning.build(); + } catch (ApiException e) { + // Okta API call errored + log.info("Okta status call raised an exception: " + e.getMessage()); + return oktaDegradedWarning.build(); + } + + return Health.up().build(); + } +} diff --git a/backend/src/main/java/gov/cdc/usds/simplereport/idp/repository/DemoOktaRepository.java b/backend/src/main/java/gov/cdc/usds/simplereport/idp/repository/DemoOktaRepository.java index 01eb007649..9cafca9de3 100644 --- a/backend/src/main/java/gov/cdc/usds/simplereport/idp/repository/DemoOktaRepository.java +++ b/backend/src/main/java/gov/cdc/usds/simplereport/idp/repository/DemoOktaRepository.java @@ -1,6 +1,6 @@ package gov.cdc.usds.simplereport.idp.repository; -import static gov.cdc.usds.simplereport.api.heathcheck.BackendAndDatabaseHealthIndicator.ACTIVE_LITERAL; +import static gov.cdc.usds.simplereport.api.heathcheck.OktaHealthIndicator.ACTIVE_LITERAL; import com.okta.sdk.resource.model.UserStatus; import gov.cdc.usds.simplereport.api.CurrentTenantDataAccessContextHolder; diff --git a/backend/src/main/java/gov/cdc/usds/simplereport/idp/repository/LiveOktaRepository.java b/backend/src/main/java/gov/cdc/usds/simplereport/idp/repository/LiveOktaRepository.java index 9ecb41ecaf..113c2c9faa 100644 --- a/backend/src/main/java/gov/cdc/usds/simplereport/idp/repository/LiveOktaRepository.java +++ b/backend/src/main/java/gov/cdc/usds/simplereport/idp/repository/LiveOktaRepository.java @@ -39,6 +39,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.UUID; @@ -690,7 +691,7 @@ public PartialOktaUser findUser(String username) { @Override public String getApplicationStatusForHealthCheck() { - return app.getStatus().toString(); + return Objects.requireNonNull(app.getStatus()).toString(); } private Optional getOrganizationRoleClaimsFromAuthorities( diff --git a/backend/src/main/resources/application.yaml b/backend/src/main/resources/application.yaml index 477bda79d2..156d354421 100644 --- a/backend/src/main/resources/application.yaml +++ b/backend/src/main/resources/application.yaml @@ -74,10 +74,16 @@ server: error: include-stacktrace: never management: - endpoint.health.probes.enabled: true - endpoint.info.enabled: true endpoints.web.exposure.include: health, info - endpoint.health.show-components: always + endpoint: + info: + enabled: true + health: + show-components: always + status: + http-mapping: + okta_degraded: 204 + probes.enabled: true okta: oauth2: issuer: https://hhs-prime.okta.com/oauth2/default diff --git a/backend/src/test/java/gov/cdc/usds/simplereport/api/healthcheck/BackendAndDatabaseHealthIndicatorTest.java b/backend/src/test/java/gov/cdc/usds/simplereport/api/healthcheck/BackendAndDatabaseHealthIndicatorTest.java index 9e0cfa936c..44064a9510 100644 --- a/backend/src/test/java/gov/cdc/usds/simplereport/api/healthcheck/BackendAndDatabaseHealthIndicatorTest.java +++ b/backend/src/test/java/gov/cdc/usds/simplereport/api/healthcheck/BackendAndDatabaseHealthIndicatorTest.java @@ -1,13 +1,11 @@ package gov.cdc.usds.simplereport.api.healthcheck; -import static gov.cdc.usds.simplereport.api.heathcheck.BackendAndDatabaseHealthIndicator.ACTIVE_LITERAL; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.when; import gov.cdc.usds.simplereport.api.heathcheck.BackendAndDatabaseHealthIndicator; import gov.cdc.usds.simplereport.db.repository.BaseRepositoryTest; import gov.cdc.usds.simplereport.db.repository.FeatureFlagRepository; -import gov.cdc.usds.simplereport.idp.repository.OktaRepository; import java.sql.SQLException; import java.util.List; import lombok.RequiredArgsConstructor; @@ -23,14 +21,12 @@ class BackendAndDatabaseHealthIndicatorTest extends BaseRepositoryTest { @SpyBean private FeatureFlagRepository mockFeatureFlagRepo; - @SpyBean private OktaRepository mockOktaRepo; @Autowired private BackendAndDatabaseHealthIndicator indicator; @Test void health_succeedsWhenReposDoesntThrow() { when(mockFeatureFlagRepo.findAll()).thenReturn(List.of()); - when(mockOktaRepo.getApplicationStatusForHealthCheck()).thenReturn(ACTIVE_LITERAL); assertThat(indicator.health()).isEqualTo(Health.up().build()); } @@ -51,10 +47,4 @@ void health_failsWhenFeatureFlagRepoThrows() { when(mockFeatureFlagRepo.findAll()).thenThrow(dbConnectionException); assertThat(indicator.health()).isEqualTo(Health.down().build()); } - - @Test - void health_failsWhenOktaRepoDoesntReturnActive() { - when(mockOktaRepo.getApplicationStatusForHealthCheck()).thenReturn("INACTIVE"); - assertThat(indicator.health()).isEqualTo(Health.down().build()); - } } diff --git a/backend/src/test/java/gov/cdc/usds/simplereport/api/healthcheck/OktaHealthIndicatorTest.java b/backend/src/test/java/gov/cdc/usds/simplereport/api/healthcheck/OktaHealthIndicatorTest.java new file mode 100644 index 0000000000..f47285eedf --- /dev/null +++ b/backend/src/test/java/gov/cdc/usds/simplereport/api/healthcheck/OktaHealthIndicatorTest.java @@ -0,0 +1,53 @@ +package gov.cdc.usds.simplereport.api.healthcheck; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.when; + +import com.okta.sdk.resource.client.ApiException; +import gov.cdc.usds.simplereport.api.heathcheck.OktaHealthIndicator; +import gov.cdc.usds.simplereport.db.repository.BaseRepositoryTest; +import gov.cdc.usds.simplereport.idp.repository.OktaRepository; +import lombok.RequiredArgsConstructor; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.actuate.health.Health; +import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.boot.test.mock.mockito.SpyBean; + +@RequiredArgsConstructor +@EnableConfigurationProperties +class OktaHealthIndicatorTest extends BaseRepositoryTest { + + @SpyBean private OktaRepository mockOktaRepo; + + @Autowired private OktaHealthIndicator indicator; + + @Test + void health_SucceedsWhenOktaRepoReturnsActive() { + when(mockOktaRepo.getApplicationStatusForHealthCheck()).thenReturn("ACTIVE"); + assertThat(indicator.health()).isEqualTo(Health.up().build()); + } + + @Test + void health_FailsWhenOktaApiThrowsErrors() { + when(mockOktaRepo.getApplicationStatusForHealthCheck()) + .thenThrow(new ApiException("some api error")); + Health.Builder oktaDegradedWarning = Health.status("OKTA_DEGRADED"); + assertThat(indicator.health()).isEqualTo(oktaDegradedWarning.build()); + } + + @Test + void health_FailsWhenOktaApiThrowsNPE() { + when(mockOktaRepo.getApplicationStatusForHealthCheck()).thenThrow(new NullPointerException()); + Health.Builder oktaDegradedWarning = Health.status("OKTA_DEGRADED"); + assertThat(indicator.health()).isEqualTo(oktaDegradedWarning.build()); + } + + @Test + void health_failsWhenOktaRepoDoesntReturnActive() { + when(mockOktaRepo.getApplicationStatusForHealthCheck()).thenReturn("INACTIVE"); + Health.Builder oktaDegradedWarning = Health.status("OKTA_DEGRADED"); + + assertThat(indicator.health()).isEqualTo(oktaDegradedWarning.build()); + } +} diff --git a/backend/src/test/java/gov/cdc/usds/simplereport/test_util/SliceTestConfiguration.java b/backend/src/test/java/gov/cdc/usds/simplereport/test_util/SliceTestConfiguration.java index 90d8e55b71..442dd219d0 100644 --- a/backend/src/test/java/gov/cdc/usds/simplereport/test_util/SliceTestConfiguration.java +++ b/backend/src/test/java/gov/cdc/usds/simplereport/test_util/SliceTestConfiguration.java @@ -6,6 +6,7 @@ import gov.cdc.usds.simplereport.api.CurrentTenantDataAccessContextHolder; import gov.cdc.usds.simplereport.api.WebhookContextHolder; import gov.cdc.usds.simplereport.api.heathcheck.BackendAndDatabaseHealthIndicator; +import gov.cdc.usds.simplereport.api.heathcheck.OktaHealthIndicator; import gov.cdc.usds.simplereport.api.pxp.CurrentPatientContextHolder; import gov.cdc.usds.simplereport.config.AuditingConfig; import gov.cdc.usds.simplereport.config.AuthorizationProperties; @@ -105,6 +106,7 @@ TenantDataAccessService.class, PatientSelfRegistrationLinkService.class, BackendAndDatabaseHealthIndicator.class, + OktaHealthIndicator.class, EmailService.class, SendGridDisabledConfiguration.class, }) diff --git a/frontend/deploy-smoke.js b/frontend/deploy-smoke.js index d252ebb7e9..685d555ae7 100644 --- a/frontend/deploy-smoke.js +++ b/frontend/deploy-smoke.js @@ -3,7 +3,6 @@ // endpoint which does a simple ping to a non-sensitive DB table to verify // all the connections are good. // https://github.com/CDCgov/prime-simplereport/pull/7057 - require("dotenv").config(); let { Builder } = require("selenium-webdriver"); const Chrome = require("selenium-webdriver/chrome"); @@ -33,16 +32,35 @@ driver return value; }) .then((value) => { - if (value.includes("success")) { + let appStatusSuccess, oktaStatusSuccess; + if (value.includes("App status returned success")) { + appStatusSuccess = true; + } + if (value.includes("Okta status returned success")) { + oktaStatusSuccess = true; + } + if (value.includes("App status returned failure")) { + appStatusSuccess = false; + } + if (value.includes("Okta status returned failure")) { + oktaStatusSuccess = false; + } + + if (appStatusSuccess && oktaStatusSuccess) { console.log(`Smoke test returned success status for ${appUrl}`); process.exitCode = 0; return; } - if (value.includes("failure")) { + + if (appStatusSuccess === false || oktaStatusSuccess === false) { console.log(`Smoke test returned failure status for ${appUrl}`); + console.log( + `App health returned ${appStatusSuccess}, okta health returned ${oktaStatusSuccess}` + ); process.exitCode = 1; return; } + console.log("Smoke test encountered unknown failure."); console.log(`Root element value was: ${value}`); process.exitCode = 1; diff --git a/frontend/src/app/DeploySmokeTest.test.tsx b/frontend/src/app/DeploySmokeTest.test.tsx index 0d3a8cbde0..606b6606d0 100644 --- a/frontend/src/app/DeploySmokeTest.test.tsx +++ b/frontend/src/app/DeploySmokeTest.test.tsx @@ -1,30 +1,50 @@ import { render, screen, waitFor } from "@testing-library/react"; import { FetchMock } from "jest-fetch-mock"; -import DeploySmokeTest from "./DeploySmokeTest"; +import DeploySmokeTest, { + APP_STATUS_FAILURE, + APP_STATUS_LOADING, + APP_STATUS_SUCCESS, + OKTA_STATUS_LOADING, + OKTA_STATUS_SUCCESS, +} from "./DeploySmokeTest"; +import { generateBackendApiHealthResponse } from "./deploySmokeTestTestConstants"; describe("DeploySmokeTest", () => { beforeEach(() => { (fetch as FetchMock).resetMocks(); }); - it("renders success when returned from the API endpoint", async () => { + it("renders success when returned from the backend API smoke test endpoint", async () => { (fetch as FetchMock).mockResponseOnce(JSON.stringify({ status: "UP" })); + (fetch as FetchMock).mockResponseOnce(JSON.stringify({ status: "DOWN" })); render(); await waitFor(() => - expect(screen.queryByText("Status loading...")).not.toBeInTheDocument() + expect(screen.queryByText(APP_STATUS_LOADING)).not.toBeInTheDocument() ); - expect(screen.getByText("Status returned success :)")); + expect(screen.getByText(APP_STATUS_SUCCESS)); }); - it("renders failure when returned from the API endpoint", async () => { - (fetch as FetchMock).mockResponseOnce(JSON.stringify({ status: "DOWN" })); + it("renders failure when returned from the backend API smoke test endpoint", async () => { + (fetch as FetchMock).mockResponse(JSON.stringify({ status: "DOWN" })); + + render(); + await waitFor(() => + expect(screen.queryByText(APP_STATUS_LOADING)).not.toBeInTheDocument() + ); + expect(screen.getByText(APP_STATUS_FAILURE)); + }); + + it("renders Okta success when returned from the backend API health endpoint", async () => { + (fetch as FetchMock).mockResponse( + JSON.stringify(generateBackendApiHealthResponse()) + ); render(); await waitFor(() => - expect(screen.queryByText("Status loading...")).not.toBeInTheDocument() + expect(screen.queryByText(OKTA_STATUS_LOADING)).not.toBeInTheDocument() ); - expect(screen.getByText("Status returned failure :(")); + expect(screen.getByText(OKTA_STATUS_SUCCESS)); }); }); diff --git a/frontend/src/app/DeploySmokeTest.tsx b/frontend/src/app/DeploySmokeTest.tsx index 1ddcbc3e9e..e52abe92f9 100644 --- a/frontend/src/app/DeploySmokeTest.tsx +++ b/frontend/src/app/DeploySmokeTest.tsx @@ -2,26 +2,77 @@ import { useEffect, useState } from "react"; import FetchClient from "./utils/api"; +export const APP_STATUS_LOADING = "App status loading..."; +export const APP_STATUS_SUCCESS = "App status returned success :)"; +export const APP_STATUS_FAILURE = "App status returned failure :("; + +export const OKTA_STATUS_LOADING = "Okta status loading..."; +export const OKTA_STATUS_SUCCESS = "Okta status returned success :)"; +export const OKTA_STATUS_FAILURE = "Okta status returned failure :("; + const api = new FetchClient(undefined, { mode: "cors" }); const DeploySmokeTest = (): JSX.Element => { - const [success, setSuccess] = useState(); + const [appStatus, setAppStatus] = useState(); + const [oktaStatus, setOktaStatus] = useState(); + useEffect(() => { api .getRequest("/actuator/health/backend-and-db-smoke-test") .then((response) => { const status = JSON.parse(response); - if (status.status === "UP") return setSuccess(true); - setSuccess(false); + if (status.status === "UP") return setAppStatus(true); + setAppStatus(false); + }) + .catch((e) => { + console.error(e); + setAppStatus(false); + }); + }, []); + + useEffect(() => { + api + .getRequest("/actuator/health") + .then((response) => { + const status = JSON.parse(response); + if (status.components.okta.status === "UP") return setOktaStatus(true); + setOktaStatus(false); }) .catch((e) => { console.error(e); - setSuccess(false); + setOktaStatus(false); }); }, []); - if (success === undefined) return <>Status loading...; - if (success) return <> Status returned success :) ; - return <> Status returned failure :( ; + let appStatusMessage; + switch (appStatus) { + case undefined: + appStatusMessage = APP_STATUS_LOADING; + break; + case true: + appStatusMessage = APP_STATUS_SUCCESS; + break; + case false: + appStatusMessage = APP_STATUS_FAILURE; + break; + } + let oktaStatusMessage; + switch (oktaStatus) { + case undefined: + oktaStatusMessage = OKTA_STATUS_LOADING; + break; + case true: + oktaStatusMessage = OKTA_STATUS_SUCCESS; + break; + case false: + oktaStatusMessage = OKTA_STATUS_FAILURE; + break; + } + return ( + <> +
{appStatusMessage}
+
{oktaStatusMessage}
+ + ); }; export default DeploySmokeTest; diff --git a/frontend/src/app/deploySmokeTestTestConstants.ts b/frontend/src/app/deploySmokeTestTestConstants.ts new file mode 100644 index 0000000000..df68fa9bc1 --- /dev/null +++ b/frontend/src/app/deploySmokeTestTestConstants.ts @@ -0,0 +1,54 @@ +type SpringHealthStatusValues = "UP" | "DOWN" | "OKTA_DEGRADED"; +export const generateBackendApiHealthResponse = ( + overallStatus?: SpringHealthStatusValues, + oktaStatus?: SpringHealthStatusValues +) => { + return { + status: overallStatus ?? "UP", + components: { + "backend-and-db-smoke-test": { + status: "UP", + }, + db: { + status: "UP", + components: { + metabaseDataSource: { + status: "UP", + }, + primaryDataSource: { + status: "UP", + }, + }, + }, + discoveryComposite: { + description: "Discovery Client not initialized", + status: "UNKNOWN", + components: { + discoveryClient: { + description: "Discovery Client not initialized", + status: "UNKNOWN", + }, + }, + }, + diskSpace: { + status: "UP", + }, + livenessState: { + status: "UP", + }, + okta: { + status: oktaStatus ?? "UP", + }, + ping: { + status: "UP", + }, + readinessState: { + status: "UP", + }, + refreshScope: { + status: "UP", + }, + }, + groups: ["liveness", "readiness"], + }; +};