Skip to content

Commit cde40e7

Browse files
setchysetchy
committed
refactor(auth): migrate to octokit/oauth-method library
Signed-off-by: Adam Setch <adam.setch@outlook.com>
1 parent 11d9d84 commit cde40e7

5 files changed

Lines changed: 25 additions & 37 deletions

File tree

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,15 @@ type Documents = {
2020
"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,
2121
"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,
2222
"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,
23-
"query FetchAuthenticatedUserDetails {\n viewer {\n id\n name\n login\n avatarUrl\n }\n}": typeof types.FetchAuthenticatedUserDetailsDocument,
23+
"query FetchAuthenticatedUserDetails {\n viewer {\n id\n name\n login\n avatar: avatarUrl\n }\n}": typeof types.FetchAuthenticatedUserDetailsDocument,
2424
};
2525
const documents: Documents = {
2626
"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,
2727
"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,
2828
"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,
2929
"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,
3030
"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,
31-
"query FetchAuthenticatedUserDetails {\n viewer {\n id\n name\n login\n avatarUrl\n }\n}": types.FetchAuthenticatedUserDetailsDocument,
31+
"query FetchAuthenticatedUserDetails {\n viewer {\n id\n name\n login\n avatar: avatarUrl\n }\n}": types.FetchAuthenticatedUserDetailsDocument,
3232
};
3333

3434
/**
@@ -54,7 +54,7 @@ export function graphql(source: "query FetchPullRequestByNumber($owner: String!,
5454
/**
5555
* The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients.
5656
*/
57-
export function graphql(source: "query FetchAuthenticatedUserDetails {\n viewer {\n id\n name\n login\n avatarUrl\n }\n}"): typeof import('./graphql').FetchAuthenticatedUserDetailsDocument;
57+
export function graphql(source: "query FetchAuthenticatedUserDetails {\n viewer {\n id\n name\n login\n avatar: avatarUrl\n }\n}"): typeof import('./graphql').FetchAuthenticatedUserDetailsDocument;
5858

5959

6060
export function graphql(source: string) {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35855,7 +35855,7 @@ export type PullRequestReviewFieldsFragment = { __typename?: 'PullRequestReview'
3585535855
export type FetchAuthenticatedUserDetailsQueryVariables = Exact<{ [key: string]: never; }>;
3585635856

3585735857

35858-
export type FetchAuthenticatedUserDetailsQuery = { __typename?: 'Query', viewer: { __typename?: 'User', id: string, name?: string | null, login: string, avatarUrl: any } };
35858+
export type FetchAuthenticatedUserDetailsQuery = { __typename?: 'Query', viewer: { __typename?: 'User', id: string, name?: string | null, login: string, avatar: any } };
3585935859

3586035860
export class TypedDocumentString<TResult, TVariables>
3586135861
extends String
@@ -36528,7 +36528,7 @@ export const FetchAuthenticatedUserDetailsDocument = new TypedDocumentString(`
3652836528
id
3652936529
name
3653036530
login
36531-
avatarUrl
36531+
avatar: avatarUrl
3653236532
}
3653336533
}
3653436534
`) as unknown as TypedDocumentString<FetchAuthenticatedUserDetailsQuery, FetchAuthenticatedUserDetailsQueryVariables>;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,6 @@ query FetchAuthenticatedUserDetails {
33
id
44
name
55
login
6-
avatarUrl
6+
avatar: avatarUrl
77
}
88
}

‎src/renderer/utils/auth/utils.test.ts‎

Lines changed: 15 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,11 @@ import type {
1515
} from '../../types';
1616
import * as comms from '../../utils/comms';
1717
import * as apiClient from '../api/client';
18-
import type { FetchAuthenticatedUserDetailsQuery } from '../api/graphql/generated/graphql';
1918
import * as logger from '../logger';
2019
import type { AuthMethod } from './types';
2120
import * as authUtils from './utils';
2221
import { getNewOAuthAppURL, getNewTokenURL } from './utils';
2322

24-
// FIXME remove type , mock correct client level?
25-
type UserDetailsResponse = FetchAuthenticatedUserDetailsQuery['viewer'];
26-
2723
const exchangeWebFlowCodeMock = jest.fn();
2824
jest.mock('@octokit/oauth-methods', () => ({
2925
...jest.requireActual('@octokit/oauth-methods'),
@@ -118,7 +114,7 @@ describe('renderer/utils/auth/utils.ts', () => {
118114
expect(openExternalLinkSpy).toHaveBeenCalledTimes(1);
119115
expect(openExternalLinkSpy).toHaveBeenCalledWith(
120116
expect.stringContaining(
121-
'https://github.com/login/oauth/authorize?client_id=FAKE_CLIENT_ID_123&scope=read%3Auser%2Cnotifications%2Crepo',
117+
'https://github.com/login/oauth/authorize?allow_signup=false&client_id=FAKE_CLIENT_ID_123&scope=read%3Auser%2Cnotifications%2Crepo',
122118
),
123119
);
124120

@@ -132,26 +128,21 @@ describe('renderer/utils/auth/utils.ts', () => {
132128
describe('exchangeAuthCodeForAccessToken', () => {
133129
const authCode = '123-456' as AuthCode;
134130

135-
it('should get a token', async () => {
136-
exchangeWebFlowCodeMock.mockResolvedValueOnce(
137-
Promise.resolve({
138-
authentication: {
139-
token: 'this-is-a-token',
140-
},
141-
}),
142-
);
131+
it('should exchange auth code for access token', async () => {
132+
exchangeWebFlowCodeMock.mockResolvedValueOnce({
133+
authentication: {
134+
token: 'this-is-a-token',
135+
},
136+
});
143137

144138
const res = await authUtils.exchangeAuthCodeForAccessToken(authCode);
145139

146-
expect(exchangeWebFlowCodeMock).toHaveBeenCalledWith(
147-
'https://github.com/login/oauth/access_token',
148-
'POST',
149-
{
150-
client_id: 'FAKE_CLIENT_ID_123',
151-
client_secret: 'FAKE_CLIENT_SECRET_123',
152-
code: '123-456',
153-
},
154-
);
140+
expect(exchangeWebFlowCodeMock).toHaveBeenCalledWith({
141+
clientType: 'oauth-app',
142+
clientId: 'FAKE_CLIENT_ID_123',
143+
clientSecret: 'FAKE_CLIENT_SECRET_123',
144+
code: '123-456',
145+
});
155146
expect(res).toBe('this-is-a-token');
156147
});
157148
});
@@ -177,10 +168,7 @@ describe('renderer/utils/auth/utils.ts', () => {
177168
beforeEach(() => {
178169
fetchAuthenticatedUserDetailsSpy.mockResolvedValue({
179170
data: {
180-
viewer: {
181-
...mockGitifyUser,
182-
avatarUrl: mockGitifyUser.avatar,
183-
} as UserDetailsResponse,
171+
viewer: mockGitifyUser,
184172
},
185173
headers: {
186174
'x-oauth-scopes': Constants.OAUTH_SCOPES.RECOMMENDED.join(', '),
@@ -235,10 +223,7 @@ describe('renderer/utils/auth/utils.ts', () => {
235223
beforeEach(() => {
236224
fetchAuthenticatedUserDetailsSpy.mockResolvedValue({
237225
data: {
238-
viewer: {
239-
...mockGitifyUser,
240-
avatarUrl: mockGitifyUser.avatar,
241-
} as UserDetailsResponse,
226+
viewer: mockGitifyUser,
242227
},
243228
headers: {
244229
'x-github-enterprise-version': '3.0.0',

‎src/renderer/utils/auth/utils.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ export function performGitHubOAuth(
3434
clientId: authOptions.clientId,
3535
scopes: Constants.OAUTH_SCOPES.RECOMMENDED,
3636
allowSignup: false,
37+
request: request.defaults({
38+
baseUrl: `https://${authOptions.hostname}`,
39+
}),
3740
});
3841

3942
openExternalLink(url as Link);
@@ -152,7 +155,7 @@ export async function refreshAccount(account: Account): Promise<Account> {
152155
id: user.id,
153156
login: user.login,
154157
name: user.name,
155-
avatar: user.avatarUrl,
158+
avatar: user.avatar,
156159
};
157160

158161
account.version = extractHostVersion(

0 commit comments

Comments
 (0)