From fbc7e7f00a328e40b79f70ad94cacba3d7aec9a9 Mon Sep 17 00:00:00 2001 From: Darcy Ye Date: Wed, 17 Apr 2024 16:12:09 +0800 Subject: [PATCH] feat(connector): can access all user email even if no public email is set --- .changeset/pretty-mirrors-peel.md | 5 + .../connectors/connector-github/package.json | 4 +- .../connector-github/src/constant.ts | 8 +- .../connector-github/src/index.test.ts | 119 +++++++++++++----- .../connectors/connector-github/src/index.ts | 92 ++++++++------ .../connectors/connector-github/src/types.ts | 11 ++ pnpm-lock.yaml | 32 ++--- 7 files changed, 180 insertions(+), 91 deletions(-) create mode 100644 .changeset/pretty-mirrors-peel.md diff --git a/.changeset/pretty-mirrors-peel.md b/.changeset/pretty-mirrors-peel.md new file mode 100644 index 00000000000..c6f90aae76d --- /dev/null +++ b/.changeset/pretty-mirrors-peel.md @@ -0,0 +1,5 @@ +--- +"@logto/connector-github": minor +--- + +Can skip email completion step when not public email available diff --git a/packages/connectors/connector-github/package.json b/packages/connectors/connector-github/package.json index b0e35afbd78..83fa7cbc5f6 100644 --- a/packages/connectors/connector-github/package.json +++ b/packages/connectors/connector-github/package.json @@ -6,7 +6,7 @@ "dependencies": { "@logto/connector-kit": "workspace:^3.0.0", "@silverhand/essentials": "^2.9.0", - "got": "^14.0.0", + "ky": "^1.2.3", "query-string": "^9.0.0", "snakecase-keys": "^8.0.0", "zod": "^3.22.4" @@ -64,7 +64,7 @@ "@vitest/coverage-v8": "^1.4.0", "eslint": "^8.56.0", "lint-staged": "^15.0.2", - "nock": "^13.3.1", + "nock": "14.0.0-beta.6", "prettier": "^3.0.0", "rollup": "^4.12.0", "rollup-plugin-output-size": "^1.3.0", diff --git a/packages/connectors/connector-github/src/constant.ts b/packages/connectors/connector-github/src/constant.ts index 4d9445e16fa..798546e4027 100644 --- a/packages/connectors/connector-github/src/constant.ts +++ b/packages/connectors/connector-github/src/constant.ts @@ -2,9 +2,15 @@ import type { ConnectorMetadata } from '@logto/connector-kit'; import { ConnectorPlatform, ConnectorConfigFormItemType } from '@logto/connector-kit'; export const authorizationEndpoint = 'https://github.com/login/oauth/authorize'; -export const scope = 'read:user'; +/** + * `read:user` read user profile data; `user:email` read user email addresses (including private email addresses). + * Ref: https://docs.github.com/en/apps/oauth-apps/building-oauth-apps/scopes-for-oauth-apps + */ +export const scope = 'read:user user:email'; export const accessTokenEndpoint = 'https://github.com/login/oauth/access_token'; export const userInfoEndpoint = 'https://api.github.com/user'; +// Ref: https://docs.github.com/en/rest/users/emails?apiVersion=2022-11-28#list-email-addresses-for-the-authenticated-user +export const userEmailsEndpoint = 'https://api.github.com/user/emails'; export const defaultMetadata: ConnectorMetadata = { id: 'github-universal', diff --git a/packages/connectors/connector-github/src/index.test.ts b/packages/connectors/connector-github/src/index.test.ts index 0656245e073..f7d0793ce2f 100644 --- a/packages/connectors/connector-github/src/index.test.ts +++ b/packages/connectors/connector-github/src/index.test.ts @@ -1,9 +1,13 @@ import nock from 'nock'; import { ConnectorError, ConnectorErrorCodes } from '@logto/connector-kit'; -import qs from 'query-string'; -import { accessTokenEndpoint, authorizationEndpoint, userInfoEndpoint } from './constant.js'; +import { + accessTokenEndpoint, + authorizationEndpoint, + userEmailsEndpoint, + userInfoEndpoint, +} from './constant.js'; import createConnector, { getAccessToken } from './index.js'; import { mockedConfig } from './mock.js'; @@ -28,7 +32,7 @@ describe('getAuthorizationUri', () => { vi.fn() ); expect(authorizationUri).toEqual( - `${authorizationEndpoint}?client_id=%3Cclient-id%3E&redirect_uri=http%3A%2F%2Flocalhost%3A3000%2Fcallback&state=some_state&scope=read%3Auser` + `${authorizationEndpoint}?client_id=%3Cclient-id%3E&redirect_uri=http%3A%2F%2Flocalhost%3A3000%2Fcallback&state=some_state&scope=read%3Auser+user%3Aemail` ); }); }); @@ -40,16 +44,11 @@ describe('getAccessToken', () => { }); it('should get an accessToken by exchanging with code', async () => { - nock(accessTokenEndpoint) - .post('') - .reply( - 200, - qs.stringify({ - access_token: 'access_token', - scope: 'scope', - token_type: 'token_type', - }) - ); + nock(accessTokenEndpoint).post('').reply(200, { + access_token: 'access_token', + scope: 'scope', + token_type: 'token_type', + }); const { accessToken } = await getAccessToken(mockedConfig, { code: 'code' }); expect(accessToken).toEqual('access_token'); }); @@ -57,7 +56,7 @@ describe('getAccessToken', () => { it('throws SocialAuthCodeInvalid error if accessToken not found in response', async () => { nock(accessTokenEndpoint) .post('') - .reply(200, qs.stringify({ access_token: '', scope: 'scope', token_type: 'token_type' })); + .reply(200, { access_token: '', scope: 'scope', token_type: 'token_type' }); await expect(getAccessToken(mockedConfig, { code: 'code' })).rejects.toStrictEqual( new ConnectorError(ConnectorErrorCodes.SocialAuthCodeInvalid) ); @@ -66,16 +65,11 @@ describe('getAccessToken', () => { describe('getUserInfo', () => { beforeEach(() => { - nock(accessTokenEndpoint) - .post('') - .reply( - 200, - qs.stringify({ - access_token: 'access_token', - scope: 'scope', - token_type: 'token_type', - }) - ); + nock(accessTokenEndpoint).post('').query(true).reply(200, { + access_token: 'access_token', + scope: 'scope', + token_type: 'token_type', + }); }); afterEach(() => { @@ -91,6 +85,7 @@ describe('getUserInfo', () => { email: 'octocat@github.com', foo: 'bar', }); + nock(userEmailsEndpoint).get('').reply(200, []); const connector = await createConnector({ getConfig }); const socialUserInfo = await connector.getUserInfo({ code: 'code' }, vi.fn()); expect(socialUserInfo).toStrictEqual({ @@ -99,11 +94,70 @@ describe('getUserInfo', () => { name: 'monalisa octocat', email: 'octocat@github.com', rawData: { - id: 1, - avatar_url: 'https://github.com/images/error/octocat_happy.gif', - name: 'monalisa octocat', - email: 'octocat@github.com', - foo: 'bar', + userInfo: { + id: 1, + avatar_url: 'https://github.com/images/error/octocat_happy.gif', + name: 'monalisa octocat', + email: 'octocat@github.com', + foo: 'bar', + }, + userEmails: [], + }, + }); + }); + + it('should fallback to verified primary email if not public is available', async () => { + nock(userInfoEndpoint).get('').reply(200, { + id: 1, + avatar_url: 'https://github.com/images/error/octocat_happy.gif', + name: 'monalisa octocat', + email: undefined, + foo: 'bar', + }); + nock(userEmailsEndpoint) + .get('') + .reply(200, [ + { + email: 'foo@logto.io', + verified: true, + primary: true, + visibility: 'public', + }, + { + email: 'foo1@logto.io', + verified: true, + primary: false, + visibility: null, + }, + ]); + const connector = await createConnector({ getConfig }); + const socialUserInfo = await connector.getUserInfo({ code: 'code' }, vi.fn()); + expect(socialUserInfo).toStrictEqual({ + id: '1', + avatar: 'https://github.com/images/error/octocat_happy.gif', + name: 'monalisa octocat', + email: 'foo@logto.io', + rawData: { + userInfo: { + id: 1, + avatar_url: 'https://github.com/images/error/octocat_happy.gif', + name: 'monalisa octocat', + foo: 'bar', + }, + userEmails: [ + { + email: 'foo@logto.io', + verified: true, + primary: true, + visibility: 'public', + }, + { + email: 'foo1@logto.io', + verified: true, + primary: false, + visibility: null, + }, + ], }, }); }); @@ -115,15 +169,14 @@ describe('getUserInfo', () => { name: null, email: null, }); + nock(userEmailsEndpoint).get('').reply(200, []); const connector = await createConnector({ getConfig }); const socialUserInfo = await connector.getUserInfo({ code: 'code' }, vi.fn()); expect(socialUserInfo).toMatchObject({ id: '1', rawData: { - id: 1, - avatar_url: null, - name: null, - email: null, + userInfo: { id: 1, avatar_url: null, name: null, email: null }, + userEmails: [], }, }); }); diff --git a/packages/connectors/connector-github/src/index.ts b/packages/connectors/connector-github/src/index.ts index 52a0e97f182..8c36bdc36ba 100644 --- a/packages/connectors/connector-github/src/index.ts +++ b/packages/connectors/connector-github/src/index.ts @@ -1,6 +1,12 @@ import { assert, conditional } from '@silverhand/essentials'; -import { got, HTTPError } from 'got'; +import { + ConnectorError, + ConnectorErrorCodes, + validateConfig, + ConnectorType, + jsonGuard, +} from '@logto/connector-kit'; import type { GetAuthorizationUri, GetUserInfo, @@ -8,20 +14,14 @@ import type { CreateConnector, GetConnectorConfig, } from '@logto/connector-kit'; -import { - ConnectorError, - ConnectorErrorCodes, - validateConfig, - ConnectorType, - parseJson, -} from '@logto/connector-kit'; -import qs from 'query-string'; +import ky, { HTTPError } from 'ky'; import { authorizationEndpoint, accessTokenEndpoint, scope as defaultScope, userInfoEndpoint, + userEmailsEndpoint, defaultMetadata, defaultTimeout, } from './constant.js'; @@ -29,6 +29,7 @@ import type { GithubConfig } from './types.js'; import { authorizationCallbackErrorGuard, githubConfigGuard, + emailAddressGuard, accessTokenResponseGuard, userInfoResponseGuard, authResponseGuard, @@ -79,17 +80,18 @@ export const getAccessToken = async (config: GithubConfig, codeObject: { code: s const { code } = codeObject; const { clientId: client_id, clientSecret: client_secret } = config; - const httpResponse = await got.post({ - url: accessTokenEndpoint, - json: { - client_id, - client_secret, - code, - }, - timeout: { request: defaultTimeout }, - }); + const httpResponse = await ky + .post(accessTokenEndpoint, { + body: new URLSearchParams({ + client_id, + client_secret, + code, + }), + timeout: defaultTimeout, + }) + .json(); - const result = accessTokenResponseGuard.safeParse(qs.parse(httpResponse.body)); + const result = accessTokenResponseGuard.safeParse(httpResponse); if (!result.success) { throw new ConnectorError(ConnectorErrorCodes.InvalidResponse, result.error); @@ -110,34 +112,54 @@ const getUserInfo = validateConfig(config, githubConfigGuard); const { accessToken } = await getAccessToken(config, { code }); + const authedApi = ky.create({ + timeout: defaultTimeout, + hooks: { + beforeRequest: [ + (request) => { + request.headers.set('Authorization', `Bearer ${accessToken}`); + }, + ], + }, + }); + try { - const httpResponse = await got.get(userInfoEndpoint, { - headers: { - authorization: `token ${accessToken}`, - }, - timeout: { request: defaultTimeout }, - }); - const rawData = parseJson(httpResponse.body); - const result = userInfoResponseGuard.safeParse(rawData); - - if (!result.success) { - throw new ConnectorError(ConnectorErrorCodes.InvalidResponse, result.error); + const [userInfo, userEmails] = await Promise.all([ + authedApi.get(userInfoEndpoint).json(), + authedApi.get(userEmailsEndpoint).json(), + ]); + + const userInfoResult = userInfoResponseGuard.safeParse(userInfo); + const userEmailsResult = emailAddressGuard.array().safeParse(userEmails); + + if (!userInfoResult.success) { + throw new ConnectorError(ConnectorErrorCodes.InvalidResponse, userInfoResult.error); + } + + if (!userEmailsResult.success) { + throw new ConnectorError(ConnectorErrorCodes.InvalidResponse, userEmailsResult.error); } - const { id, avatar_url: avatar, email, name } = result.data; + const { id, avatar_url: avatar, email: publicEmail, name } = userInfoResult.data; return { id: String(id), avatar: conditional(avatar), - email: conditional(email), + email: conditional( + publicEmail ?? + userEmailsResult.data.find(({ verified, primary }) => verified && primary)?.email + ), name: conditional(name), - rawData, + rawData: jsonGuard.parse({ + userInfo, + userEmails, + }), }; } catch (error: unknown) { if (error instanceof HTTPError) { - const { statusCode, body: rawBody } = error.response; + const { status, body: rawBody } = error.response; - if (statusCode === 401) { + if (status === 401) { throw new ConnectorError(ConnectorErrorCodes.SocialAccessTokenInvalid); } diff --git a/packages/connectors/connector-github/src/types.ts b/packages/connectors/connector-github/src/types.ts index fcd8c30915a..7906e4e8be9 100644 --- a/packages/connectors/connector-github/src/types.ts +++ b/packages/connectors/connector-github/src/types.ts @@ -8,6 +8,17 @@ export const githubConfigGuard = z.object({ export type GithubConfig = z.infer; +/** + * This guard is used to validate the response from the GitHub API when requesting the user's email addresses. + * Ref: https://docs.github.com/en/rest/users/emails?apiVersion=2022-11-28#list-email-addresses-for-the-authenticated-user + */ +export const emailAddressGuard = z.object({ + email: z.string(), + primary: z.boolean(), + verified: z.boolean(), + visibility: z.string().nullable(), +}); + export const accessTokenResponseGuard = z.object({ access_token: z.string(), scope: z.string(), diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 206291d5b4c..c99ffe836e1 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -981,9 +981,9 @@ importers: '@silverhand/essentials': specifier: ^2.9.0 version: 2.9.0 - got: - specifier: ^14.0.0 - version: 14.0.0 + ky: + specifier: ^1.2.3 + version: 1.2.3 query-string: specifier: ^9.0.0 version: 9.0.0 @@ -1028,8 +1028,8 @@ importers: specifier: ^15.0.2 version: 15.0.2 nock: - specifier: ^13.3.1 - version: 13.3.1 + specifier: 14.0.0-beta.6 + version: 14.0.0-beta.6 prettier: specifier: ^3.0.0 version: 3.0.0 @@ -10186,7 +10186,7 @@ packages: strip-literal: 2.0.0 test-exclude: 6.0.0 v8-to-istanbul: 9.2.0 - vitest: 1.4.0(@types/node@20.12.7) + vitest: 1.4.0(@types/node@20.10.4) transitivePeerDependencies: - supports-color dev: true @@ -11930,17 +11930,6 @@ packages: /dayjs@1.11.6: resolution: {integrity: sha512-zZbY5giJAinCG+7AGaw0wIhNZ6J8AhWuSXKvuc1KAyMiRsvGQWqh4L+MomvhdAYjN+lqvVCMq1I41e3YHvXkyQ==} - /debug@3.2.7: - resolution: {integrity: sha512-CFjzYYAi4ThfiQvizrFQevTTXHtnCqWfe7x1AhgEscTz6ZbLbfoLRLPugTQyBth6f8ZERVUSyWHFD/7Wu4t1XQ==} - peerDependencies: - supports-color: '*' - peerDependenciesMeta: - supports-color: - optional: true - dependencies: - ms: 2.1.3 - dev: true - /debug@3.2.7(supports-color@5.5.0): resolution: {integrity: sha512-CFjzYYAi4ThfiQvizrFQevTTXHtnCqWfe7x1AhgEscTz6ZbLbfoLRLPugTQyBth6f8ZERVUSyWHFD/7Wu4t1XQ==} peerDependencies: @@ -12705,7 +12694,7 @@ packages: /eslint-import-resolver-node@0.3.9: resolution: {integrity: sha512-WFj2isz22JahUv+B788TlO3N6zL3nNJGU8CcZbPZvVEkBPaJdCV4vy5wyghty5ROFbCRnm132v8BScu5/1BQ8g==} dependencies: - debug: 3.2.7 + debug: 3.2.7(supports-color@5.5.0) is-core-module: 2.13.1 resolve: 1.22.8 transitivePeerDependencies: @@ -12757,7 +12746,7 @@ packages: optional: true dependencies: '@typescript-eslint/parser': 7.7.0(eslint@8.57.0)(typescript@5.3.3) - debug: 3.2.7 + debug: 3.2.7(supports-color@5.5.0) eslint: 8.57.0 eslint-import-resolver-node: 0.3.9 eslint-import-resolver-typescript: 3.6.1(@typescript-eslint/parser@7.7.0)(eslint-plugin-import@2.29.1)(eslint@8.57.0) @@ -12811,7 +12800,7 @@ packages: array.prototype.findlastindex: 1.2.5 array.prototype.flat: 1.3.2 array.prototype.flatmap: 1.3.2 - debug: 3.2.7 + debug: 3.2.7(supports-color@5.5.0) doctrine: 2.1.0 eslint: 8.57.0 eslint-import-resolver-node: 0.3.9 @@ -18004,6 +17993,9 @@ packages: resolution: {integrity: sha512-2GTVocFkwblV/TIg9AmT7TI2fO4xdWkyN8aFUEVtiVNWt96GTR3FgQyHFValfCbcj1k9Xf962Ws2hYXYUr9k1Q==} engines: {node: '>= 12.0.0'} hasBin: true + peerDependenciesMeta: + '@parcel/core': + optional: true dependencies: '@parcel/config-default': 2.9.3(@parcel/core@2.9.3)(postcss@8.4.31) '@parcel/core': 2.9.3