cleanup user validation

gov4git · Nov 30, 2023 · 7f79f7c · 7f79f7c
1 parent 18ae18f
commit 7f79f7c
Show file tree

Hide file tree

Showing 9 changed files with 226 additions and 200 deletions.
diff --git a/package.json b/package.json
@@ -14,7 +14,7 @@
     "format": "prettier --write \"./src/**/*.{js,jsx,ts,tsx}\"",
     "lint:check": "eslint \"./src/**/*.{js,jsx,ts,tsx}\"",
     "lint": "eslint --fix \"./src/**/*.{js,jsx,ts,tsx}\"",
-    "test": "jest testRunner.test.ts",
+    "test": "jest testRunner.test.ts --testTimeout=30000",
     "build:webapp": "vite build --config ./vite.webapp.config.mts",
     "build:electron": "tsup --dts",
     "build": "conc npm:build:webapp npm:build:electron",

diff --git a/src/electron/services/GitService.ts b/src/electron/services/GitService.ts
@@ -30,32 +30,51 @@ export class GitService {
 
   protected getAuthHeader = (user: GitUserInfo) => {
     return {
-      Authorization: `Basic ${Buffer.from(
-        `${user.username}:${user.pat}`,
-      ).toString('base64')}`,
+      Authorization: `token ${user.pat}`,
     }
   }
 
-  public getOAuthScopes = async (token: string): Promise<string[]> => {
-    const response = await fetch(`${this.apiBaseUrl}`, {
-      method: 'GET',
-      headers: {
-        Authorization: `Token ${token}`,
-      },
-    })
-    await response.text()
-    const scopes = (response.headers.get('X-OAuth-Scopes') ?? '').split(', ')
-    return scopes
+  public getTokenMetaData = async (
+    token: string,
+  ): Promise<[{ scopes: string[] }, null] | [null, string[]]> => {
+    try {
+      const response = await fetch(`${this.apiBaseUrl}`, {
+        headers: {
+          Authorization: `token ${token}`,
+        },
+      })
+      await response.text()
+      if (response.status !== 200) {
+        return [
+          null,
+          [
+            `Invalid user credentials. Personal Access Token may have expired or has insufficient privileges. Please create a classic token with top-level repo rights.`,
+          ],
+        ]
+      }
+      const scopes = (response.headers.get('X-OAuth-Scopes') ?? '').split(', ')
+      return [
+        {
+          scopes,
+        },
+        null,
+      ]
+    } catch (ex) {
+      return [
+        {
+          scopes: [] as string[],
+        },
+        null,
+      ]
+    }
   }
 
   protected throwIfUserDoesNotExist = async (
     user: GitUserInfo,
     from: string,
   ) => {
-    if (!(await this.doesUserExist(user))) {
-      throw new Error(
-        `${user.username} does not exist or PAT is invalid. From ${from}`,
-      )
+    if ((await this.validateUser(user)).length > 0) {
+      throw new Error(`Invalid user credentials. From ${from}`)
     }
   }
 
@@ -71,17 +90,6 @@ export class GitService {
     }
   }
 
-  public doesPublicRepoExist = async (repoUrl: string): Promise<boolean> => {
-    this.throwIfNotUrl(repoUrl, 'doesPublicRepoExist')
-    try {
-      const response = await fetch(repoUrl)
-      await response.text()
-      return response.status === 200
-    } catch (ex) {
-      throw new Error(`Unable to check if public repo exists. Error: ${ex}`)
-    }
-  }
-
   public doesRemoteRepoExist = async (
     repoUrl: string,
     user: GitUserInfo,
@@ -112,7 +120,7 @@ export class GitService {
     user: GitUserInfo,
   ): Promise<string | null> => {
     this.throwIfNotUrl(repoUrl, 'getDefaultBranch')
-    await this.throwIfUserDoesNotExist(user, 'doesRemoteRepoExist')
+    await this.throwIfUserDoesNotExist(user, 'getDefaultBranch')
     await this.throwIfRepoDoesNotExist(repoUrl, user, 'getDefaultBranch')
     try {
       const authHeader = this.getAuthHeader(user)
@@ -138,7 +146,7 @@ export class GitService {
     user: GitUserInfo,
   ): Promise<boolean> => {
     this.throwIfNotUrl(repoUrl, 'hasCommits')
-    await this.throwIfUserDoesNotExist(user, 'doesRemoteRepoExist')
+    await this.throwIfUserDoesNotExist(user, 'hasCommits')
     await this.throwIfRepoDoesNotExist(repoUrl, user, 'hasCommits')
     try {
       const authHeader = this.getAuthHeader(user)
@@ -162,20 +170,43 @@ export class GitService {
     }
   }
 
-  public doesUserExist = async (user: GitUserInfo): Promise<boolean> => {
-    const authHeader = this.getAuthHeader(user)
-    const response = await fetch(`${this.apiBaseUrl}/users/${user.username}`, {
-      method: 'GET',
-      headers: {
-        Accept: 'application/vnd.github+json',
-        ...authHeader,
-      },
-    })
-    await response.text()
-    if (response.status !== 200) {
-      return false
+  public validateUser = async (user: GitUserInfo): Promise<string[]> => {
+    const [tokenData, errors] = await this.getTokenMetaData(user.pat)
+
+    if (Array.isArray(errors) && errors.length) {
+      return errors
+    }
+
+    if (!tokenData!.scopes.includes('repo')) {
+      return [
+        'Personal Access Token has insufficient privileges. Please create a classic token with the top-level repo scope selected.',
+      ]
+    }
+
+    try {
+      const authHeader = this.getAuthHeader(user)
+      const response = await fetch(`${this.apiBaseUrl}/user`, {
+        method: 'GET',
+        headers: {
+          Accept: 'application/vnd.github+json',
+          ...authHeader,
+        },
+      })
+      if (response.status !== 200) {
+        return ['Invalid user credentials.']
+      }
+      const payload: any = await response.json()
+      if (
+        payload == null ||
+        payload.login == null ||
+        (payload.login as string).toLowerCase() !== user.username.toLowerCase()
+      ) {
+        return ['Invalid user credentials']
+      }
+      return []
+    } catch (ex) {
+      return [`Failed to validate user credentials. ${ex}`]
     }
-    return true
   }
 
   protected getRepoName = (repo: string): string => {

diff --git a/src/electron/services/UserService.ts b/src/electron/services/UserService.ts
@@ -67,25 +67,10 @@ export class UserService extends AbstractUserService {
     if (missingErrors.length > 0) {
       return missingErrors
     }
-    const errors: string[] = []
-    const scopes = await this.gitService.getOAuthScopes(pat)
-    if (!scopes.includes('repo')) {
-      errors.push(
-        'Personal Access Token has insufficient privileges. Please ensure PAT has rights to top-level repo scope.',
-      )
-      return errors
-    }
-
-    if (
-      !(await this.gitService.doesUserExist({
-        username,
-        pat,
-      }))
-    ) {
-      errors.push(`Invalid user credentials`)
-    }
-
-    return errors
+    return await this.gitService.validateUser({
+      username,
+      pat,
+    })
   }
 
   private getOpenBallots = async () => {

diff --git a/test/BallotService.test.ts b/test/BallotService.test.ts
@@ -58,6 +58,7 @@ export default function run(services: Services) {
         '10',
       )
     })
+
     test('Vote flow', async () => {
       expect(true).toEqual(true)
       await userService.loadUser()

diff --git a/test/CommunityService.test.ts b/test/CommunityService.test.ts
@@ -10,52 +10,56 @@ export default function run(services: Services) {
   let communityService: CommunityService
   let db: DB
 
-  beforeAll(async () => {
-    communityService = services.load<CommunityService>('community')
-    db = services.load<DB>('db')
-  })
+  describe('Community Tests', () => {
+    beforeAll(async () => {
+      communityService = services.load<CommunityService>('community')
+      db = services.load<DB>('db')
+    })
 
-  describe('Validate Community URL', () => {
-    test('Invalid', async () => {
-      // Act
-      const errors = await communityService.validateCommunityUrl(
-        `${config.baseUrl}/${config.user.username}/repo-does-not-exist.git`,
-      )
+    describe('Validate Community URL', () => {
+      test('Invalid', async () => {
+        // Act
+        const errors = await communityService.validateCommunityUrl(
+          `${config.baseUrl}/${config.user.username}/repo-does-not-exist.git`,
+        )
 
-      // Assert
-      expect(errors[0]).toBeNull()
-      expect((errors[1] ?? []).length > 0).toEqual(true)
-    })
+        // Assert
+        expect(errors[0]).toBeNull()
+        expect((errors[1] ?? []).length > 0).toEqual(true)
+      })
+
+      test('Valid', async () => {
+        // Act
+        const errors = await communityService.validateCommunityUrl(
+          config.communityUrl,
+        )
 
-    test('Valid', async () => {
-      // Act
-      const errors = await communityService.validateCommunityUrl(
-        config.communityUrl,
-      )
-
-      // Assert
-      expect(errors[0]).not.toBeNull()
-      expect(typeof errors[0] === 'string').toEqual(true)
-      expect(errors[0] !== '').toEqual(true)
-      expect((errors[1] ?? []).length).toEqual(0)
+        // Assert
+        expect(errors[0]).not.toBeNull()
+        expect(typeof errors[0] === 'string').toEqual(true)
+        expect(errors[0] !== '').toEqual(true)
+        expect((errors[1] ?? []).length).toEqual(0)
+      })
     })
-  })
 
-  describe('Inserting new community', () => {
-    test('Insert', async () => {
-      const errors = await communityService.insertCommunity(config.projectRepo)
-      expect((errors[1] ?? []).length).toEqual(0)
-
-      const selectedCommunity = (
-        await db
-          .select()
-          .from(communities)
-          .where(eq(communities.selected, true))
-          .limit(1)
-      )[0]
-
-      expect(selectedCommunity).not.toBeUndefined()
-      expect(selectedCommunity?.projectUrl).toEqual(config.projectRepo)
+    describe('Inserting new community', () => {
+      test('Insert', async () => {
+        const errors = await communityService.insertCommunity(
+          config.projectRepo,
+        )
+        expect((errors[1] ?? []).length).toEqual(0)
+
+        const selectedCommunity = (
+          await db
+            .select()
+            .from(communities)
+            .where(eq(communities.selected, true))
+            .limit(1)
+        )[0]
+
+        expect(selectedCommunity).not.toBeUndefined()
+        expect(selectedCommunity?.projectUrl).toEqual(config.projectRepo)
+      })
     })
   })
 }
diff --git a/test/GitService.test.ts b/test/GitService.test.ts
@@ -9,7 +9,7 @@ import {
   type GitUserInfo,
 } from '../src/electron/services/GitService.js'
 
-const gitService = new GitService()
+let gitService: GitService
 const user: GitUserInfo = {
   username: process.env['GH_USER']!,
   pat: process.env['GH_TOKEN']!,
@@ -19,15 +19,16 @@ const baseUrl = 'https://github.com'
 const projectRepo = `${baseUrl}/${user.username}/test-gov4git-creating-deleting-repos`
 
 export default function run() {
-  beforeAll(async () => {
-    await gitService.deleteRepo(projectRepo, user)
-  }, 30000)
-
-  describe('Working with Repos', () => {
+  describe('Git Tests', () => {
+    beforeAll(async () => {
+      gitService = new GitService()
+      await gitService.deleteRepo(projectRepo, user)
+    })
     test('Does public repo exist', async () => {
       // Act
-      const shouldNotExist = !(await gitService.doesPublicRepoExist(
+      const shouldNotExist = !(await gitService.doesRemoteRepoExist(
         projectRepo,
+        user,
       ))
 
       // Assert