fix: trim whitespace in user handling functions and update tests (#792)

Author: Naishy

src/config.ts

@@ -9,7 +9,11 @@ export const config = {
     return parseInt(process.env.EXIT_CODE_ON_MISMATCH ?? '0') || 0
   },
   get ignoredUsers(): string[] {
-    return process.env.IGNORED_USERS?.toLowerCase().split(',') ?? []
+    return (
+      process.env.IGNORED_USERS?.toLowerCase()
+        .split(',')
+        .map((user) => user.trim()) ?? []
+    )
   },
   get githubPrivateKey(): string {
     return Buffer.from(process.env.GITHUB_PRIVATE_KEY, 'base64').toString('utf-8')

src/github.ts

@@ -49,7 +49,7 @@ export async function getGithubUsersFromGithub(): Promise<Set<string>> {
 export function formatUserList(users): Set<string> {
   return new Set(
     users
-      .map((user) => user.login?.toLowerCase())
+      .map((user) => user.login?.trim().toLowerCase())
       .flat()
       .filter(Boolean),
   )
@@ -83,6 +83,7 @@ export async function addUsersToGitHubOrg(users: Set<string>): Promise<Operation
 
 // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
 export async function addUserToGitHubOrg(user: string): Promise<{ error: OperationError } | boolean> {
+  user = user.trim()
   const octokit = mod.getAuthenticatedOctokit()
   if (config.ignoredUsers.includes(user.toLowerCase())) {
     console.log(`Ignoring add for ${user}`)
@@ -126,6 +127,7 @@ export async function removeUsersFromGitHubOrg(users: Set<string>): Promise<Oper
 
 // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
 export async function removeUserFromGitHubOrg(user: string): Promise<{ error: OperationError } | boolean> {
+  user = user.trim()
   const octokit = mod.getAuthenticatedOctokit()
   if (config.ignoredUsers.includes(user.toLowerCase())) {
     console.log(`Ignoring remove for ${user}`)

src/google.ts

@@ -45,7 +45,7 @@ export function formatUserList(users): Set<string> {
   return new Set(
     users
       .filter((user) => !user.suspended && !user.archived)
-      .map((user) => user.customSchemas?.Accounts?.github?.map((account) => account.value?.toLowerCase()))
+      .map((user) => user.customSchemas?.Accounts?.github?.map((account) => account.value?.trim().toLowerCase()))
       .flat()
       .filter(Boolean),
   )

tests/config.spec.ts

@@ -64,6 +64,10 @@ describe('ignoredUsers', () => {
     process.env.IGNORED_USERS = 'user1,user2,user3,USER4'
     expect(mod.config.ignoredUsers).toMatchSnapshot()
   })
+  it('getIgnoredUsers trims whitespace around entries', () => {
+    process.env.IGNORED_USERS = ' user1, user2 ,user3 '
+    expect(mod.config.ignoredUsers).toEqual(['user1', 'user2', 'user3'])
+  })
 })
 
 describe('githubPrivateKey', () => {

tests/github.spec.ts

@@ -182,4 +182,48 @@ describe('github integration', () => {
     const response = [{ login: 'chrisns' }, { login: 'chrisns' }, { login: 'foo' }, {}]
     return expect(mod.formatUserList(response)).toEqual(new Set(['chrisns', 'foo']))
   })
+
+  it('formatUserList trims leading and trailing whitespace from logins', () => {
+    const response = [{ login: ' chrisns' }, { login: 'foo ' }, { login: ' bar ' }]
+    return expect(mod.formatUserList(response)).toEqual(new Set(['chrisns', 'foo', 'bar']))
+  })
+
+  it('addUserToGitHubOrg trims whitespace before checking ignored users', () => {
+    jest.spyOn(config, 'ignoredUsers', 'get').mockReturnValue(['foo'])
+    return expect(mod.addUserToGitHubOrg(' foo ')).resolves.toBe(false)
+  })
+
+  it('addUserToGitHubOrg trims whitespace before looking up the username', async () => {
+    const fakeOctokit = {
+      orgs: {
+        createInvitation: jest.fn().mockResolvedValue(true),
+      },
+    }
+    jest.spyOn(config, 'githubOrg', 'get').mockReturnValue('myorg')
+    const getUserIdSpy = jest.spyOn(mod, 'getUserIdFromUsername').mockResolvedValue(123)
+    // @ts-expect-error mock service isn't a complete implementation, so being lazy and just doing the bare minimum
+    jest.spyOn(mod, 'getAuthenticatedOctokit').mockReturnValue(fakeOctokit)
+    const result = await mod.addUserToGitHubOrg(' foo ')
+    expect(result).toBe(true)
+    expect(getUserIdSpy).toHaveBeenCalledWith('foo')
+  })
+
+  it('removeUserFromGitHubOrg trims whitespace before checking ignored users', () => {
+    jest.spyOn(config, 'ignoredUsers', 'get').mockReturnValue(['foo'])
+    return expect(mod.removeUserFromGitHubOrg(' foo ')).resolves.toBe(false)
+  })
+
+  it('removeUserFromGitHubOrg trims whitespace before calling the API', async () => {
+    const fakeOctokit = {
+      orgs: {
+        removeMembershipForUser: jest.fn().mockResolvedValue(true),
+      },
+    }
+    jest.spyOn(config, 'githubOrg', 'get').mockReturnValue('myorg')
+    // @ts-expect-error mock service isn't a complete implementation, so being lazy and just doing the bare minimum
+    jest.spyOn(mod, 'getAuthenticatedOctokit').mockReturnValue(fakeOctokit)
+    const result = await mod.removeUserFromGitHubOrg(' foo ')
+    expect(result).toBe(true)
+    expect(fakeOctokit.orgs.removeMembershipForUser).toHaveBeenCalledWith(expect.objectContaining({ username: 'foo' }))
+  })
 })

tests/google.spec.ts

@@ -82,4 +82,14 @@ describe('google integration', () => {
     const result = mod.formatUserList(users)
     expect(result).toEqual(new Set(['active']))
   })
+
+  it('formatUserList trims leading and trailing whitespace from handles', () => {
+    const users = [
+      { customSchemas: { Accounts: { github: [{ value: ' chrisns' }] } }, suspended: false, archived: false },
+      { customSchemas: { Accounts: { github: [{ value: 'foo ' }] } }, suspended: false, archived: false },
+      { customSchemas: { Accounts: { github: [{ value: ' bar ' }] } }, suspended: false, archived: false },
+    ]
+    const result = mod.formatUserList(users)
+    expect(result).toEqual(new Set(['chrisns', 'foo', 'bar']))
+  })
 })