refactor(auth): use @octokit/oauth-methods (#2523)

* refactor(auth): migrate to octokit/oauth-method library

Signed-off-by: Adam Setch <adam.setch@outlook.com>

* refactor(auth): migrate to octokit/oauth-method library

Signed-off-by: Adam Setch <adam.setch@outlook.com>

* refactor(auth): migrate to octokit/oauth-method library

Signed-off-by: Adam Setch <adam.setch@outlook.com>

* refactor(auth): migrate to octokit/oauth-method library

Signed-off-by: Adam Setch <adam.setch@outlook.com>

* refactor(auth): migrate to octokit/oauth-method library

Signed-off-by: Adam Setch <adam.setch@outlook.com>

* refactor(auth): migrate to octokit/oauth-method library

Signed-off-by: Adam Setch <adam.setch@outlook.com>

* move to shared constant

Signed-off-by: Adam Setch <adam.setch@outlook.com>

---------

Signed-off-by: Adam Setch <adam.setch@outlook.com>

Author: setchy

jest.config.ts

@@ -14,9 +14,10 @@ const config: Config = {
   },
   // Allow transforming specific ESM packages in node_modules that ship untranspiled ESM.
   // @primer/* libraries rely on lit and @lit-labs/react internally for some components.
+  // @octokit/* libraries rely on universal-user-agent internally.
   // We also include GitHub web components that ship ESM-only builds.
   transformIgnorePatterns: [
-    'node_modules/(?!(?:@primer/react|@primer/primitives|@primer/octicons-react|@lit-labs/react|lit|@github/relative-time-element|@github/tab-container-element)/)',
+    'node_modules/(?!(?:@primer/react|@primer/primitives|@primer/octicons-react|@lit-labs/react|lit|@github/relative-time-element|@github/tab-container-element|@octokit/oauth-methods|@octokit/oauth-authorization-url|@octokit/request|@octokit/request-error|@octokit/endpoint|universal-user-agent)/)',
   ],
   moduleNameMapper: {
     // Force CommonJS build for http adapter to be available.

package.json

@@ -84,7 +84,9 @@
     "@electron/notarize": "3.1.1",
     "@graphql-codegen/cli": "6.1.1",
     "@graphql-codegen/schema-ast": "5.0.0",
+    "@octokit/oauth-methods": "6.0.2",
     "@octokit/openapi-types": "27.0.0",
+    "@octokit/request": "10.0.7",
     "@parcel/watcher": "2.5.4",
     "@primer/css": "22.1.0",
     "@primer/octicons-react": "19.21.2",

pnpm-lock.yaml

@@ -78,7 +78,9 @@ export default class MenuBuilder {
           {
             label: 'Visit Repository',
             click: () => {
-              shell.openExternal(`https://github.com/${APPLICATION.REPO_SLUG}`);
+              shell.openExternal(
+                `${APPLICATION.GITHUB_BASE_URL}/${APPLICATION.REPO_SLUG}`,
+              );
             },
           },
           {

src/main/menu.ts

@@ -1,4 +1,5 @@
 import type { ClientID, ClientSecret, Hostname, Link } from './types';
+import type { LoginOAuthAppOptions } from './utils/auth/types';
 
 export const Constants = {
   STORAGE_KEY: 'gitify-storage',
@@ -13,7 +14,7 @@ export const Constants = {
     hostname: 'github.com' as Hostname,
     clientId: process.env.OAUTH_CLIENT_ID as ClientID,
     clientSecret: process.env.OAUTH_CLIENT_SECRET as ClientSecret,
-  },
+  } satisfies LoginOAuthAppOptions,
 
   GITHUB_API_BASE_URL: 'https://api.github.com',
   GITHUB_API_GRAPHQL_URL: 'https://api.github.com/graphql',

src/renderer/constants.ts

@@ -39,10 +39,10 @@ import type {
 import { headNotifications } from '../utils/api/client';
 import {
   addAccount,
-  authGitHub,
+  exchangeAuthCodeForAccessToken,
   getAccountUUID,
-  getToken,
   hasAccounts,
+  performGitHubOAuth,
   refreshAccount,
   removeAccount,
 } from '../utils/auth/utils';
@@ -395,28 +395,45 @@ export const AppProvider = ({ children }: { children: ReactNode }) => {
     return hasAccounts(auth);
   }, [auth]);
 
+  /**
+   * Login with GitHub App.
+   *
+   * Note: although we call this "Login with GitHub App", this function actually
+   * authenticates via a predefined "Gitify" GitHub OAuth App.
+   */
   const loginWithGitHubApp = useCallback(async () => {
-    const { authCode } = await authGitHub();
-    const { token } = await getToken(authCode);
+    const { authCode } = await performGitHubOAuth();
+    const token = await exchangeAuthCodeForAccessToken(authCode);
     const hostname = Constants.DEFAULT_AUTH_OPTIONS.hostname;
 
     const updatedAuth = await addAccount(auth, 'GitHub App', token, hostname);
 
     persistAuth(updatedAuth);
   }, [auth, persistAuth]);
 
+  /**
+   * Login with custom GitHub OAuth App.
+   */
   const loginWithOAuthApp = useCallback(
     async (data: LoginOAuthAppOptions) => {
-      const { authOptions, authCode } = await authGitHub(data);
-      const { token, hostname } = await getToken(authCode, authOptions);
+      const { authOptions, authCode } = await performGitHubOAuth(data);
+      const token = await exchangeAuthCodeForAccessToken(authCode, authOptions);
 
-      const updatedAuth = await addAccount(auth, 'OAuth App', token, hostname);
+      const updatedAuth = await addAccount(
+        auth,
+        'OAuth App',
+        token,
+        authOptions.hostname,
+      );
 
       persistAuth(updatedAuth);
     },
     [auth, persistAuth],
   );
 
+  /**
+   * Login with Personal Access Token (PAT).
+   */
   const loginWithPersonalAccessToken = useCallback(
     async ({ token, hostname }: LoginPersonalAccessTokenOptions) => {
       const encryptedToken = (await encryptValue(token)) as Token;

src/renderer/context/App.tsx

@@ -20,15 +20,15 @@ type Documents = {
     "query FetchIssueByNumber($owner: String!, $name: String!, $number: Int!, $lastComments: Int, $firstLabels: Int) {\n  repository(owner: $owner, name: $name) {\n    issue(number: $number) {\n      ...IssueDetails\n    }\n  }\n}\n\nfragment IssueDetails on Issue {\n  __typename\n  number\n  title\n  url\n  state\n  stateReason\n  milestone {\n    ...MilestoneFields\n  }\n  author {\n    ...AuthorFields\n  }\n  comments(last: $lastComments) {\n    totalCount\n    nodes {\n      url\n      author {\n        ...AuthorFields\n      }\n    }\n  }\n  labels(first: $firstLabels) {\n    nodes {\n      name\n    }\n  }\n}": typeof types.FetchIssueByNumberDocument,
     "query FetchMergedDetailsTemplate($ownerINDEX: String!, $nameINDEX: String!, $numberINDEX: Int!, $isDiscussionNotificationINDEX: Boolean!, $isIssueNotificationINDEX: Boolean!, $isPullRequestNotificationINDEX: Boolean!, $lastComments: Int, $lastThreadedComments: Int, $lastReplies: Int, $lastReviews: Int, $firstLabels: Int, $firstClosingIssues: Int, $includeIsAnswered: Boolean!) {\n  ...MergedDetailsQueryTemplate\n}\n\nfragment MergedDetailsQueryTemplate on Query {\n  repository(owner: $ownerINDEX, name: $nameINDEX) {\n    discussion(number: $numberINDEX) @include(if: $isDiscussionNotificationINDEX) {\n      ...DiscussionDetails\n    }\n    issue(number: $numberINDEX) @include(if: $isIssueNotificationINDEX) {\n      ...IssueDetails\n    }\n    pullRequest(number: $numberINDEX) @include(if: $isPullRequestNotificationINDEX) {\n      ...PullRequestDetails\n    }\n  }\n}": typeof types.FetchMergedDetailsTemplateDocument,
     "query FetchPullRequestByNumber($owner: String!, $name: String!, $number: Int!, $firstLabels: Int, $lastComments: Int, $lastReviews: Int, $firstClosingIssues: Int) {\n  repository(owner: $owner, name: $name) {\n    pullRequest(number: $number) {\n      ...PullRequestDetails\n    }\n  }\n}\n\nfragment PullRequestDetails on PullRequest {\n  __typename\n  number\n  title\n  url\n  state\n  merged\n  isDraft\n  isInMergeQueue\n  milestone {\n    ...MilestoneFields\n  }\n  author {\n    ...AuthorFields\n  }\n  comments(last: $lastComments) {\n    totalCount\n    nodes {\n      url\n      author {\n        ...AuthorFields\n      }\n    }\n  }\n  reviews(last: $lastReviews) {\n    totalCount\n    nodes {\n      ...PullRequestReviewFields\n    }\n  }\n  labels(first: $firstLabels) {\n    nodes {\n      name\n    }\n  }\n  closingIssuesReferences(first: $firstClosingIssues) {\n    nodes {\n      number\n    }\n  }\n}\n\nfragment PullRequestReviewFields on PullRequestReview {\n  state\n  author {\n    login\n  }\n}": typeof types.FetchPullRequestByNumberDocument,
-    "query FetchAuthenticatedUserDetails {\n  viewer {\n    id\n    name\n    login\n    avatarUrl\n  }\n}": typeof types.FetchAuthenticatedUserDetailsDocument,
+    "query FetchAuthenticatedUserDetails {\n  viewer {\n    id\n    name\n    login\n    avatar: avatarUrl\n  }\n}": typeof types.FetchAuthenticatedUserDetailsDocument,
 };
 const documents: Documents = {
     "fragment AuthorFields on Actor {\n  login\n  htmlUrl: url\n  avatarUrl: avatarUrl\n  type: __typename\n}\n\nfragment MilestoneFields on Milestone {\n  state\n  title\n}": types.AuthorFieldsFragmentDoc,
     "query FetchDiscussionByNumber($owner: String!, $name: String!, $number: Int!, $lastThreadedComments: Int, $lastReplies: Int, $firstLabels: Int, $includeIsAnswered: Boolean!) {\n  repository(owner: $owner, name: $name) {\n    discussion(number: $number) {\n      ...DiscussionDetails\n    }\n  }\n}\n\nfragment DiscussionDetails on Discussion {\n  __typename\n  number\n  title\n  stateReason\n  isAnswered @include(if: $includeIsAnswered)\n  url\n  author {\n    ...AuthorFields\n  }\n  comments(last: $lastThreadedComments) {\n    totalCount\n    nodes {\n      ...DiscussionCommentFields\n    }\n  }\n  labels(first: $firstLabels) {\n    nodes {\n      name\n    }\n  }\n}\n\nfragment CommentFields on DiscussionComment {\n  databaseId\n  createdAt\n  author {\n    ...AuthorFields\n  }\n  url\n}\n\nfragment DiscussionCommentFields on DiscussionComment {\n  ...CommentFields\n  replies(last: $lastReplies) {\n    totalCount\n    nodes {\n      ...CommentFields\n    }\n  }\n}": types.FetchDiscussionByNumberDocument,
     "query FetchIssueByNumber($owner: String!, $name: String!, $number: Int!, $lastComments: Int, $firstLabels: Int) {\n  repository(owner: $owner, name: $name) {\n    issue(number: $number) {\n      ...IssueDetails\n    }\n  }\n}\n\nfragment IssueDetails on Issue {\n  __typename\n  number\n  title\n  url\n  state\n  stateReason\n  milestone {\n    ...MilestoneFields\n  }\n  author {\n    ...AuthorFields\n  }\n  comments(last: $lastComments) {\n    totalCount\n    nodes {\n      url\n      author {\n        ...AuthorFields\n      }\n    }\n  }\n  labels(first: $firstLabels) {\n    nodes {\n      name\n    }\n  }\n}": types.FetchIssueByNumberDocument,
     "query FetchMergedDetailsTemplate($ownerINDEX: String!, $nameINDEX: String!, $numberINDEX: Int!, $isDiscussionNotificationINDEX: Boolean!, $isIssueNotificationINDEX: Boolean!, $isPullRequestNotificationINDEX: Boolean!, $lastComments: Int, $lastThreadedComments: Int, $lastReplies: Int, $lastReviews: Int, $firstLabels: Int, $firstClosingIssues: Int, $includeIsAnswered: Boolean!) {\n  ...MergedDetailsQueryTemplate\n}\n\nfragment MergedDetailsQueryTemplate on Query {\n  repository(owner: $ownerINDEX, name: $nameINDEX) {\n    discussion(number: $numberINDEX) @include(if: $isDiscussionNotificationINDEX) {\n      ...DiscussionDetails\n    }\n    issue(number: $numberINDEX) @include(if: $isIssueNotificationINDEX) {\n      ...IssueDetails\n    }\n    pullRequest(number: $numberINDEX) @include(if: $isPullRequestNotificationINDEX) {\n      ...PullRequestDetails\n    }\n  }\n}": types.FetchMergedDetailsTemplateDocument,
     "query FetchPullRequestByNumber($owner: String!, $name: String!, $number: Int!, $firstLabels: Int, $lastComments: Int, $lastReviews: Int, $firstClosingIssues: Int) {\n  repository(owner: $owner, name: $name) {\n    pullRequest(number: $number) {\n      ...PullRequestDetails\n    }\n  }\n}\n\nfragment PullRequestDetails on PullRequest {\n  __typename\n  number\n  title\n  url\n  state\n  merged\n  isDraft\n  isInMergeQueue\n  milestone {\n    ...MilestoneFields\n  }\n  author {\n    ...AuthorFields\n  }\n  comments(last: $lastComments) {\n    totalCount\n    nodes {\n      url\n      author {\n        ...AuthorFields\n      }\n    }\n  }\n  reviews(last: $lastReviews) {\n    totalCount\n    nodes {\n      ...PullRequestReviewFields\n    }\n  }\n  labels(first: $firstLabels) {\n    nodes {\n      name\n    }\n  }\n  closingIssuesReferences(first: $firstClosingIssues) {\n    nodes {\n      number\n    }\n  }\n}\n\nfragment PullRequestReviewFields on PullRequestReview {\n  state\n  author {\n    login\n  }\n}": types.FetchPullRequestByNumberDocument,
-    "query FetchAuthenticatedUserDetails {\n  viewer {\n    id\n    name\n    login\n    avatarUrl\n  }\n}": types.FetchAuthenticatedUserDetailsDocument,
+    "query FetchAuthenticatedUserDetails {\n  viewer {\n    id\n    name\n    login\n    avatar: avatarUrl\n  }\n}": types.FetchAuthenticatedUserDetailsDocument,
 };
 
 /**
@@ -54,7 +54,7 @@ export function graphql(source: "query FetchPullRequestByNumber($owner: String!,
 /**
  * The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients.
  */
-export function graphql(source: "query FetchAuthenticatedUserDetails {\n  viewer {\n    id\n    name\n    login\n    avatarUrl\n  }\n}"): typeof import('./graphql').FetchAuthenticatedUserDetailsDocument;
+export function graphql(source: "query FetchAuthenticatedUserDetails {\n  viewer {\n    id\n    name\n    login\n    avatar: avatarUrl\n  }\n}"): typeof import('./graphql').FetchAuthenticatedUserDetailsDocument;
 
 
 export function graphql(source: string) {

src/renderer/utils/api/graphql/generated/gql.ts

@@ -35855,7 +35855,7 @@ export type PullRequestReviewFieldsFragment = { __typename?: 'PullRequestReview'
 export type FetchAuthenticatedUserDetailsQueryVariables = Exact<{ [key: string]: never; }>;
 
 
-export type FetchAuthenticatedUserDetailsQuery = { __typename?: 'Query', viewer: { __typename?: 'User', id: string, name?: string | null, login: string, avatarUrl: any } };
+export type FetchAuthenticatedUserDetailsQuery = { __typename?: 'Query', viewer: { __typename?: 'User', id: string, name?: string | null, login: string, avatar: any } };
 
 export class TypedDocumentString<TResult, TVariables>
   extends String
@@ -36528,7 +36528,7 @@ export const FetchAuthenticatedUserDetailsDocument = new TypedDocumentString(`
     id
     name
     login
-    avatarUrl
+    avatar: avatarUrl
   }
 }
     `) as unknown as TypedDocumentString<FetchAuthenticatedUserDetailsQuery, FetchAuthenticatedUserDetailsQueryVariables>;

src/renderer/utils/api/graphql/generated/graphql.ts

@@ -3,6 +3,6 @@ query FetchAuthenticatedUserDetails {
     id
     name
     login
-    avatarUrl
+    avatar: avatarUrl
   }
 }

src/renderer/utils/api/graphql/user.graphql

@@ -11,7 +11,6 @@ import type { Link } from '../../types';
 
 import { FetchAuthenticatedUserDetailsDocument } from './graphql/generated/graphql';
 import {
-  apiRequest,
   apiRequestAuth,
   getHeaders,
   performGraphQLRequest,
@@ -29,33 +28,6 @@ describe('renderer/utils/api/request.ts', () => {
     jest.clearAllMocks();
   });
 
-  describe('apiRequest', () => {
-    it('should make a request with the correct parameters', async () => {
-      const data = { key: 'value' };
-
-      await apiRequest(url, method, data);
-
-      expect(axios).toHaveBeenCalledWith({
-        method,
-        url,
-        data,
-        headers: mockNoAuthHeaders,
-      });
-    });
-
-    it('should make a request with the correct parameters and default data', async () => {
-      const data = {};
-      await apiRequest(url, method);
-
-      expect(axios).toHaveBeenCalledWith({
-        method,
-        url,
-        data,
-        headers: mockNoAuthHeaders,
-      });
-    });
-  });
-
   describe('apiRequestAuth', () => {
     afterEach(() => {
       jest.clearAllMocks();