Skip to content
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
6 changes: 5 additions & 1 deletion src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
4 changes: 3 additions & 1 deletion src/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
)
Expand Down Expand Up @@ -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}`)
Expand Down Expand Up @@ -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}`)
Expand Down
2 changes: 1 addition & 1 deletion src/google.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
)
Expand Down
4 changes: 4 additions & 0 deletions tests/config.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
44 changes: 44 additions & 0 deletions tests/github.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' }))
})
})
10 changes: 10 additions & 0 deletions tests/google.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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']))
})
})
Loading