Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(feature-manager): remove Sentry from feature manager #2130

Merged
merged 2 commits into from
Jun 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,9 @@ services:
- MYINFO_CLIENT_SECRET=mockClientSecret
- WEBHOOK_SQS_URL=http://localhost:4566/000000000000/local-webhooks-sqs-main
- INTRANET_IP_LIST_PATH
- SENTRY_CONFIG_URL=https://[email protected]/123456
- CSP_REPORT_URI=https://[email protected]/123456
- GA_TRACKING_ID
- SENTRY_CONFIG_URL
- TWILIO_ACCOUNT_SID
- TWILIO_API_KEY
- TWILIO_API_SECRET
Expand Down
2 changes: 0 additions & 2 deletions src/app/config/feature-manager/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import FeatureManager from './util/FeatureManager.class'
import captcha from './captcha.config'
import googleAnalytics from './google-analytics.config'
import sentry from './sentry.config'
import sms from './sms.config'
import spcpMyInfo from './spcp-myinfo.config'
import verifiedFields from './verified-fields.config'
Expand All @@ -13,7 +12,6 @@ const featureManager = new FeatureManager()

// Register features and associated middleware/fallbacks
featureManager.register(captcha)
featureManager.register(sentry)
featureManager.register(googleAnalytics)
featureManager.register(spcpMyInfo)
featureManager.register(webhookVerifiedContent)
Expand Down
41 changes: 24 additions & 17 deletions src/app/config/feature-manager/sentry.config.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,28 @@
import { FeatureNames, RegisterableFeature } from './types'
import convict, { Schema } from 'convict'
import { url } from 'convict-format-with-validator'

const sentryFeature: RegisterableFeature<FeatureNames.Sentry> = {
name: FeatureNames.Sentry,
schema: {
sentryConfigUrl: {
doc: 'Sentry.io URL for configuring the Sentry SDK',
format: 'url',
default: null,
env: 'SENTRY_CONFIG_URL',
},
cspReportUri: {
doc: 'Endpoint for content security policy reporting',
format: 'url',
default: null,
env: 'CSP_REPORT_URI',
},
export interface ISentry {
sentryConfigUrl: string
cspReportUri: string
}

convict.addFormat(url)

const sentryFeature: Schema<ISentry> = {
sentryConfigUrl: {
doc: 'Sentry.io URL for configuring the Sentry SDK',
format: 'url',
default: null,
env: 'SENTRY_CONFIG_URL',
},
cspReportUri: {
doc: 'Endpoint for content security policy reporting',
format: 'url',
default: null,
env: 'CSP_REPORT_URI',
},
}

export default sentryFeature
export const sentryConfig = convict(sentryFeature)
.validate({ allowed: 'strict' })
.getProperties()
7 changes: 0 additions & 7 deletions src/app/config/feature-manager/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { Schema } from 'convict'
export enum FeatureNames {
Captcha = 'captcha',
GoogleAnalytics = 'google-analytics',
Sentry = 'sentry',
Sms = 'sms',
SpcpMyInfo = 'spcp-myinfo',
VerifiedFields = 'verified-fields',
Expand All @@ -20,11 +19,6 @@ export interface IGoogleAnalytics {
GATrackingID: string
}

export interface ISentry {
sentryConfigUrl: string
cspReportUri: string
}

export interface ISms {
twilioAccountSid: string
twilioApiKey: string
Expand Down Expand Up @@ -79,7 +73,6 @@ export interface IWebhookVerifiedContent {
export interface IFeatureManager {
[FeatureNames.Captcha]: ICaptcha
[FeatureNames.GoogleAnalytics]: IGoogleAnalytics
[FeatureNames.Sentry]: ISentry
[FeatureNames.Sms]: ISms
[FeatureNames.SpcpMyInfo]: ISpcpMyInfo
[FeatureNames.VerifiedFields]: IVerifiedFields
Expand Down
12 changes: 5 additions & 7 deletions src/app/loaders/express/__tests__/helmet.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import helmet from 'helmet'
import { mocked } from 'ts-jest/utils'

import config from 'src/app/config/config'
import featureManager from 'src/app/config/feature-manager'
import { sentryConfig } from 'src/app/config/feature-manager/sentry.config'

import expressHandler from 'tests/unit/backend/helpers/jest-express'

Expand All @@ -11,10 +11,10 @@ import helmetMiddlewares from '../helmet'
describe('helmetMiddlewares', () => {
jest.mock('helmet')
const mockHelmet = mocked(helmet, true)
jest.mock('src/app/config/feature-manager')
const mockFeatureManager = mocked(featureManager, true)
jest.mock('src/app/config/config')
const mockConfig = mocked(config, true)
jest.mock('src/app/config/feature-manager/sentry.config')
const mockSentryConfig = mocked(sentryConfig, true)

const cspCoreDirectives = {
defaultSrc: ["'self'"],
Expand Down Expand Up @@ -136,9 +136,7 @@ describe('helmetMiddlewares', () => {
})

it('should call helmet.contentSecurityPolicy() with the correct directives if cspReportUri and !isDev', () => {
mockFeatureManager.props = jest
.fn()
.mockReturnValue({ cspReportUri: 'value' })
mockSentryConfig.cspReportUri = 'value'
mockConfig.isDev = false
helmetMiddlewares()
expect(mockHelmet.contentSecurityPolicy).toHaveBeenCalledWith({
Expand All @@ -151,7 +149,7 @@ describe('helmetMiddlewares', () => {
})

it('should call helmet.contentSecurityPolicy() with the correct directives if !cspReportUri and isDev', () => {
mockFeatureManager.props = jest.fn()
mockSentryConfig.cspReportUri = ''
mockConfig.isDev = true
helmetMiddlewares()
expect(mockHelmet.contentSecurityPolicy).toHaveBeenCalledWith({
Expand Down
9 changes: 2 additions & 7 deletions src/app/loaders/express/helmet.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { RequestHandler } from 'express'
import helmet from 'helmet'
import { ContentSecurityPolicyOptions } from 'helmet/dist/middlewares/content-security-policy'
import { get } from 'lodash'

import config from '../../config/config'
import featureManager, { FeatureNames } from '../../config/feature-manager'
import { sentryConfig } from '../../config/feature-manager/sentry.config'

const helmetMiddlewares = () => {
// Only add the "Strict-Transport-Security" header if request is https.
Expand Down Expand Up @@ -79,11 +78,7 @@ const helmetMiddlewares = () => {
formAction: ["'self'"],
}

const reportUri = get(
featureManager.props(FeatureNames.Sentry),
'cspReportUri',
undefined,
)
const reportUri = sentryConfig.cspReportUri

const cspOptionalDirectives: ContentSecurityPolicyOptions['directives'] = {}

Expand Down
7 changes: 2 additions & 5 deletions src/app/loaders/express/locals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { get } from 'lodash'

import config from '../../config/config'
import featureManager, { FeatureNames } from '../../config/feature-manager'
import { sentryConfig } from '../../config/feature-manager/sentry.config'

// Construct js with environment variables needed by frontend
const frontendVars = {
Expand All @@ -17,11 +18,7 @@ const frontendVars = {
'captchaPublicKey',
null,
), // Recaptcha
sentryConfigUrl: get(
featureManager.props(FeatureNames.Sentry),
'sentryConfigUrl',
null,
), // Sentry.IO
sentryConfigUrl: sentryConfig.sentryConfigUrl, // Sentry.IO
isSPMaintenance: get(
featureManager.props(FeatureNames.SpcpMyInfo),
'isSPMaintenance',
Expand Down