Dont delete readers by default

.husky/post-checkout

@@ -1,3 +1,2 @@
 #!/bin/sh
-. "$(dirname -- "$0")/_/husky.sh"
 .husky/scripts/update-registry.sh to-private

.husky/post-merge

@@ -1,3 +1,2 @@
 #!/bin/sh
-. "$(dirname -- "$0")/_/husky.sh"
 .husky/scripts/update-registry.sh to-private

.husky/post-rewrite

@@ -1,3 +1,2 @@
 #!/bin/sh
-. "$(dirname -- "$0")/_/husky.sh"
 .husky/scripts/update-registry.sh to-private

.husky/pre-commit

@@ -1,3 +1,2 @@
 #!/bin/sh
-. "$(dirname -- "$0")/_/husky.sh"
 .husky/scripts/update-registry.sh to-public

.husky/scripts/update-registry.sh

@@ -1,5 +1,10 @@
 #!/bin/bash
 
+PRIVATE="packages.ic1.statefarm/artifactory/api/npm/npm-virtual"
+PUBLIC_NODE="registry.npmjs.org"
+PUBLIC_YARN="registry.yarnpkg.com"
+DIRECTION="$1"
+
 # If private registry not resolvable, force public registries
 # Ensures commits never contain internal endpoints outside corp network
 if ! nslookup packages.ic1.statefarm >/dev/null 2>&1; then
@@ -10,14 +15,8 @@ fi
 
 # Invoke: .husky/scripts/update-registry.sh to-private # Turn to private
 #         .husky/scripts/update-registry.sh            # Turn to public
-# Read direction from command line argument
-DIRECTION="$1"
-
-PRIVATE="packages.ic1.statefarm/artifactory/api/npm/npm-virtual"
-PUBLIC_NODE="registry.npmjs.org"
-PUBLIC_YARN="registry.yarnpkg.com"
 
-if [ "$DIRECTION" == "to-private" ]; then
+if [ "$DIRECTION" = "to-private" ]; then
   echo 'making private'
   sed -i '' "s|${PUBLIC_NODE}|${PRIVATE}|g" package-lock.json 2>/dev/null || true
   sed -i '' "s|${PUBLIC_YARN}|${PRIVATE}|g" yarn.lock 2>/dev/null || true

AGENTS.md

@@ -0,0 +1,22 @@
+# Agent Guide
+
+1. Build: `yarn build` (outputs bundled `pruneQuickSightUsers.js`); Dev TS config targets ES5, module esnext.
+2. Test all: `yarn test` (Jest + ts-jest). Single test: `jest path/to/file.test.ts -t "test name"` or `jest QuickSightUserManager.test.ts`.
+3. Lint/auto-fix: `yarn lint` (eslint --fix on src/*.ts and test/*.test.ts). Indent tabs; LF endings; single quotes; no semicolons; object spacing `{ a: 1 }`; dangle commas multiline.
+4. Mutation testing: `yarn mutate` (Stryker). Stryker ignore comments already present; preserve them.
+5. Imports: Use absolute AWS SDK v3 client imports (`@aws-sdk/...`); local relative `./FileName`; group: external libs, then local, blank line separation optional.
+6. Types: Enable `noImplicitAny`; prefer explicit return types on public methods; use enums (e.g. QuickSightRole); narrow with `keyof typeof` pattern as in QuickSightUser.
+7. Naming: Classes PascalCase; methods camelCase; constants lowerCamelCase; environment variables accessed via `process.env.someName` (do not destructure). Avoid single-letter vars.
+8. Error handling: Propagate AWS SDK errors (tests assert rejects); do not swallow; throw original (`rejects.toThrowError(error)`). Use early returns over nested conditionals.
+9. Async: Always `await client.send(command)`; avoid parallelism unless beneficial; preserve ordering for metrics/user operations.
+10. Side effects: Log via `console.debug/info/warn`; keep Stryker disable comments on the line directly above logged statement.
+11. State: Keep private members (`private xyz = ...`); avoid global mutable state; reset collections after emission (see CloudWatchMetricClient.emitQueuedMetrics).
+12. Formatting: Tabs for indent; no trailing whitespace; multiline JSON.stringify indentation matches existing (2 spaces inside).
+13. Tests: Use `aws-sdk-client-mock` for AWS clients; sinon stubs for class methods; snapshots allowed. Fake timers with `jest.useFakeTimers().setSystemTime(...)`.
+14. Adding tests: Mirror existing pattern: arrange stubs, act, assert call counts & inputs. Prefer `toStrictEqual` for input objects.
+15. Commits: Conventional commits (semantic-release). Do not include build artifacts except required root `pruneQuickSightUsers.js`.
+16. Environment logic: Compare dates via `<`, `toLocaleDateString()` for equality by day; treat `new Date(0)` as sentinel.
+17. Performance: Small data sets; pagination loops use `do { ... } while (nextToken)`; maintain token semantics ('' for QS, null for CloudTrail).
+18. Terraform: Lambda depends on `pruneQuickSightUsers.js` existing; do not rename the file. `delete_readers` variable -> env `deleteReaders`; when false skip READER deletions with debug log.
+19. PRs: Link related issue; keep diff minimal; retain logging and Stryker comments.
+20. Husky: Pre-commit runs lint; ensure staged code passes lint & tests before committing.

README.md

@@ -12,6 +12,8 @@ In August of 2021, we implemented automatic user pruning with this Terraform mod
 
 After deploying this module, you will have a Lambda that runs daily (or on a schedule of your choosing -- but keep in mind it is designed to run daily with the way notifications are designed). The Lambda compares CloudTrail events to current QuickSight users. It notifies users when they haven't been used for exactly 30 days (by default) and deletes them when they haven't been used in 45 or more days (by default). This will save you big chunks of money. No underlying resources are touched (yet). If a user makes a dashboard and that user is deleted, then it makes no difference. Other users with access still have access, and the original user may regain access by logging in again.
 
+By default the module will NOT delete READER users (`delete_readers = false`) because deleting them stops scheduled emailed reports and they are not billed at author/admin rates. Set `delete_readers = true` if you explicitly want READER deletions; otherwise they are skipped.
+
 # Prerequisites
 
 1. QuickSight should be enabled in the AWS account. This module provides cost-savings benefits to QuickSight and is not useful if you don't have QuickSight enabled in your account.

eslint.config.js

@@ -0,0 +1,27 @@
+const tsPlugin = require('@typescript-eslint/eslint-plugin')
+const tsParser = require('@typescript-eslint/parser')
+
+module.exports = [
+	{
+		files: ['**/*.ts'],
+		ignores: ['dist/', 'node_modules/'],
+		languageOptions: {
+			parser: tsParser,
+			ecmaVersion: 2021,
+			sourceType: 'module',
+		},
+		plugins: {
+			'@typescript-eslint': tsPlugin,
+		},
+		rules: {
+			...tsPlugin.configs.recommended.rules,
+			indent: ['error', 'tab'],
+			'linebreak-style': ['error', 'unix'],
+			quotes: ['error', 'single'],
+			semi: ['error', 'never'],
+			'quote-props': ['error', 'as-needed'],
+			'object-curly-spacing': ['error', 'always'],
+			'comma-dangle': ['error', 'always-multiline'],
+		},
+	},
+]

src/pruneQuickSightUsers.ts

@@ -44,9 +44,14 @@ export default async () => {
 			// Stryker disable next-line all "I do not care about mutating console statements"
 			console.warn(`Invalid user: ${JSON.stringify(quickSightUser)}`)
 			invalidUsers++
-		} else if (quickSightUser.lastAccess < deleteDate) {
-			usersDeleted++
-			await quickSightUserManager.deleteUser(quickSightUser)
+		} else if (quickSightUser.lastAccess < deleteDate && (process.env.deleteReaders === 'true' || quickSightUser.role !== QuickSightRole.READER)) {
+			if (quickSightUser.role === QuickSightRole.READER && process.env.deleteReaders !== 'true') {
+				// Stryker disable next-line all "I do not care about mutating console statements"
+				console.debug(`Skipping deletion of READER ${quickSightUser.username}; deleteReaders disabled.`)
+			} else {
+				usersDeleted++
+				await quickSightUserManager.deleteUser(quickSightUser)
+			}
 		} else if (enableNotification
       && quickSightUser.role !== QuickSightRole.READER // Readers get into QuickSight through a public page and probably have no idea what QuickSight is. Therefore we shouldn't email them.
       && quickSightUser.lastAccess.toLocaleDateString() === notifyDate.toLocaleDateString()) { // toLocaleDateString strips off the time. If the day matches the notify "day" then we notify the user

test/pruneQuickSightUsers.test.ts

@@ -134,15 +134,15 @@ describe('pruneQuickSightUsers', () => {
 
 		sinon.assert.calledOnce(stubs.QuickSightUserManager.retrieveUsers)
 		sinon.assert.calledOnceWithExactly(stubs.CloudTrailUserEventManager.retrieveQuickSightUserEvents, expectedDeleteDate)
-		sinon.assert.calledOnceWithExactly(stubs.QuickSightUserManager.deleteUser, sillyBilly)
+		sinon.assert.notCalled(stubs.QuickSightUserManager.deleteUser) // Reader skipped because deleteReaders default false
 		sinon.assert.calledOnceWithExactly(stubs.NotificationManager.notifyUser, hannahBanana)
 
 		sinon.assert.callCount(stubs.CloudWatchMetricClient.queueMetric, 5)
 		sinon.assert.calledWith(stubs.CloudWatchMetricClient.queueMetric.firstCall, { MetricName: 'PriorQuickSightUsersCount', Value: validQuickSightUsers.length })
 		sinon.assert.calledWith(stubs.CloudWatchMetricClient.queueMetric.secondCall, { MetricName: 'InvalidUsersCount', Value: 0 })
-		sinon.assert.calledWith(stubs.CloudWatchMetricClient.queueMetric.thirdCall, { MetricName: 'UsersDeletedCount', Value: 1 })
+		sinon.assert.calledWith(stubs.CloudWatchMetricClient.queueMetric.thirdCall, { MetricName: 'UsersDeletedCount', Value: 0 })
 		sinon.assert.calledWith(stubs.CloudWatchMetricClient.queueMetric.getCalls()[3], { MetricName: 'NotificationsSentCount', Value: 1 })
-		sinon.assert.calledWith(stubs.CloudWatchMetricClient.queueMetric.getCalls()[4], { MetricName: 'RemainingQuickSightUsersCount', Value: validQuickSightUsers.length - 1 })
+		sinon.assert.calledWith(stubs.CloudWatchMetricClient.queueMetric.getCalls()[4], { MetricName: 'RemainingQuickSightUsersCount', Value: validQuickSightUsers.length })
 	})
 
 	it('works with IAM users', async () => {
@@ -154,23 +154,43 @@ describe('pruneQuickSightUsers', () => {
 			UserName: 'sample-user',
 			Role: 'ADMIN',
 		})
-
 		stubs.QuickSightUserManager.retrieveUsers.resolves([sampleUser])
-
 		stubs.CloudTrailUserEventManager.retrieveQuickSightUserEvents.resolves([
 			{
 				stsSession: undefined,
 				iamRole: 'sample-user',
 				eventTime: new Date(),
 			},
 		])
-
 		await pruneQuickSightUsers()
-
 		sinon.assert.notCalled(stubs.NotificationManager.notifyUser) // If not called, it means the 2 events matched
 		sinon.assert.notCalled(stubs.QuickSightUserManager.deleteUser) // If not called, it means the 2 events matched
 	})
 
+	it('deletes readers when deleteReaders=true', async () => {
+		process.env.deleteReaders = 'true'
+		const readerToDelete = new QuickSightUser({
+			Active: true,
+			Arn: 'arn:aws:quicksight:us-east-1:1234567890:user/default/quicksight-reader/[email protected]',
+			Email: '[email protected]',
+			PrincipalId: 'federated/iam/ARIAGERHIGWFEHQOFIH:[email protected]',
+			UserName: 'quicksight-reader/[email protected]',
+			Role: 'READER',
+		})
+		const deletePastDate = new Date()
+		deletePastDate.setDate(deletePastDate.getDate() - 31)
+		stubs.QuickSightUserManager.retrieveUsers.resolves([readerToDelete])
+		stubs.CloudTrailUserEventManager.retrieveQuickSightUserEvents.resolves([
+			{
+				stsSession: readerToDelete.stsSession,
+				iamRole: readerToDelete.iamRole,
+				eventTime: deletePastDate,
+			},
+		])
+		await pruneQuickSightUsers()
+		sinon.assert.calledOnceWithExactly(stubs.QuickSightUserManager.deleteUser, readerToDelete)
+	})
+
 	it('does not notify readers', async () => {
 		stubs.QuickSightUserManager.retrieveUsers.resolves(validQuickSightUsers)
 
@@ -219,7 +239,7 @@ describe('pruneQuickSightUsers', () => {
 		sinon.assert.calledWithExactly(stubs.CloudWatchMetricClient.queueMetric, { MetricName: 'InvalidUsersCount', Value: 1 })
 	})
 
-	it('does not delete someone right at deletion day, only after deletion day', async () => {
+	it('does not delete someone right at deletion day, only after deletion day (even if reader)', async () => {
 		const oneUserOnVergeOfDeletion = [new QuickSightUser({
 			Active: true,
 			Arn: 'arn:aws:quicksight:us-east-1:1234567890:user/default/quicksight-reader/[email protected]',

z-prune-quicksight-users.tf

@@ -42,6 +42,7 @@ resource "aws_lambda_function" "quicksight_cleanup" {
       accountAlias       = data.aws_iam_account_alias.current.account_alias
       deleteDays         = var.delete_days
       notifyDays         = var.notify_days
+      deleteReaders      = var.delete_readers
       enableNotification = local.enable_notification ? true : false
       contact            = local.enable_notification ? var.notification_config.contact : null
       replyTo            = local.enable_notification ? var.notification_config.reply_to : null

z-variables.tf

@@ -22,6 +22,12 @@ variable "delete_days" {
   description = "Days since last access when we should delete the user"
 }
 
+variable "delete_readers" {
+  default     = false
+  type        = bool
+  description = "If true, delete READER role users. Defaults false to preserve scheduled emailed reports for readers."
+}
+
 variable "notification_config" {
   description = "Provide these values to enable notification. If not provided, notification is disabled."