From d1c2721b9519e06c490a22f87147cc13c4612a92 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 28 Apr 2022 09:09:40 +0800 Subject: [PATCH] Add expiration date to api key generation (#495) * Add exp to jwt token * Add expiredAt in generate api key input * Check exp in claim * Add tests --- packages/api/src/generated/graphql.ts | 23 ++++++- packages/api/src/generated/schema.graphql | 13 +++- packages/api/src/resolvers/api_key/index.ts | 19 +++--- packages/api/src/resolvers/types.ts | 1 + packages/api/src/schema.ts | 9 ++- packages/api/test/resolvers/api_key.test.ts | 70 ++++++++++++++++++--- 6 files changed, 115 insertions(+), 20 deletions(-) diff --git a/packages/api/src/generated/graphql.ts b/packages/api/src/generated/graphql.ts index bf4fdb5ff..779435430 100644 --- a/packages/api/src/generated/graphql.ts +++ b/packages/api/src/generated/graphql.ts @@ -69,7 +69,10 @@ export type Article = { siteIcon?: Maybe; siteName?: Maybe; slug: Scalars['String']; + subscription?: Maybe; title: Scalars['String']; + unsubHttpUrl?: Maybe; + unsubMailTo?: Maybe; uploadFileId?: Maybe; url: Scalars['String']; }; @@ -530,6 +533,11 @@ export enum GenerateApiKeyErrorCode { BadRequest = 'BAD_REQUEST' } +export type GenerateApiKeyInput = { + expiredAt?: InputMaybe; + scope?: InputMaybe; +}; + export type GenerateApiKeyResult = GenerateApiKeyError | GenerateApiKeySuccess; export type GenerateApiKeySuccess = { @@ -885,7 +893,7 @@ export type MutationDeleteReminderArgs = { export type MutationGenerateApiKeyArgs = { - scope?: InputMaybe; + input: GenerateApiKeyInput; }; @@ -1373,7 +1381,10 @@ export type SearchItem = { readingProgressPercent?: Maybe; shortId?: Maybe; slug: Scalars['String']; + subscription?: Maybe; title: Scalars['String']; + unsubHttpUrl?: Maybe; + unsubMailTo?: Maybe; uploadFileId?: Maybe; url: Scalars['String']; }; @@ -2139,6 +2150,7 @@ export type ResolversTypes = { Float: ResolverTypeWrapper; GenerateApiKeyError: ResolverTypeWrapper; GenerateApiKeyErrorCode: GenerateApiKeyErrorCode; + GenerateApiKeyInput: GenerateApiKeyInput; GenerateApiKeyResult: ResolversTypes['GenerateApiKeyError'] | ResolversTypes['GenerateApiKeySuccess']; GenerateApiKeySuccess: ResolverTypeWrapper; GetFollowersError: ResolverTypeWrapper; @@ -2421,6 +2433,7 @@ export type ResolversParentTypes = { FeedArticlesSuccess: FeedArticlesSuccess; Float: Scalars['Float']; GenerateApiKeyError: GenerateApiKeyError; + GenerateApiKeyInput: GenerateApiKeyInput; GenerateApiKeyResult: ResolversParentTypes['GenerateApiKeyError'] | ResolversParentTypes['GenerateApiKeySuccess']; GenerateApiKeySuccess: GenerateApiKeySuccess; GetFollowersError: GetFollowersError; @@ -2636,7 +2649,10 @@ export type ArticleResolvers, ParentType, ContextType>; siteName?: Resolver, ParentType, ContextType>; slug?: Resolver; + subscription?: Resolver, ParentType, ContextType>; title?: Resolver; + unsubHttpUrl?: Resolver, ParentType, ContextType>; + unsubMailTo?: Resolver, ParentType, ContextType>; uploadFileId?: Resolver, ParentType, ContextType>; url?: Resolver; __isTypeOf?: IsTypeOfResolverFn; @@ -3154,7 +3170,7 @@ export type MutationResolvers>; deleteReaction?: Resolver>; deleteReminder?: Resolver>; - generateApiKey?: Resolver>; + generateApiKey?: Resolver>; googleLogin?: Resolver>; googleSignup?: Resolver>; login?: Resolver>; @@ -3362,7 +3378,10 @@ export type SearchItemResolvers, ParentType, ContextType>; shortId?: Resolver, ParentType, ContextType>; slug?: Resolver; + subscription?: Resolver, ParentType, ContextType>; title?: Resolver; + unsubHttpUrl?: Resolver, ParentType, ContextType>; + unsubMailTo?: Resolver, ParentType, ContextType>; uploadFileId?: Resolver, ParentType, ContextType>; url?: Resolver; __isTypeOf?: IsTypeOfResolverFn; diff --git a/packages/api/src/generated/schema.graphql b/packages/api/src/generated/schema.graphql index 54093421e..703d79014 100644 --- a/packages/api/src/generated/schema.graphql +++ b/packages/api/src/generated/schema.graphql @@ -50,7 +50,10 @@ type Article { siteIcon: String siteName: String slug: String! + subscription: String title: String! + unsubHttpUrl: String + unsubMailTo: String uploadFileId: ID url: String! } @@ -465,6 +468,11 @@ enum GenerateApiKeyErrorCode { BAD_REQUEST } +input GenerateApiKeyInput { + expiredAt: Date + scope: String +} + union GenerateApiKeyResult = GenerateApiKeyError | GenerateApiKeySuccess type GenerateApiKeySuccess { @@ -697,7 +705,7 @@ type Mutation { deleteNewsletterEmail(newsletterEmailId: ID!): DeleteNewsletterEmailResult! deleteReaction(id: ID!): DeleteReactionResult! deleteReminder(id: ID!): DeleteReminderResult! - generateApiKey(scope: String): GenerateApiKeyResult! + generateApiKey(input: GenerateApiKeyInput!): GenerateApiKeyResult! googleLogin(input: GoogleLoginInput!): LoginResult! googleSignup(input: GoogleSignupInput!): GoogleSignupResult! login(input: LoginInput!): LoginResult! @@ -982,7 +990,10 @@ type SearchItem { readingProgressPercent: Float shortId: String slug: String! + subscription: String title: String! + unsubHttpUrl: String + unsubMailTo: String uploadFileId: ID url: String! } diff --git a/packages/api/src/resolvers/api_key/index.ts b/packages/api/src/resolvers/api_key/index.ts index 9b70b59b6..538a7c837 100644 --- a/packages/api/src/resolvers/api_key/index.ts +++ b/packages/api/src/resolvers/api_key/index.ts @@ -13,25 +13,28 @@ export const generateApiKeyResolver = authorized< GenerateApiKeySuccess, GenerateApiKeyError, MutationGenerateApiKeyArgs ->((_, { scope }, { claims }) => { +>((_, { input: { scope, expiredAt } }, { claims }) => { try { - console.log('generateApiKeyResolver', scope) + console.log('generateApiKeyResolver', scope, expiredAt) + + const exp = expiredAt ? new Date(expiredAt).getTime() / 1000 : null + const apiKey = generateApiKey({ + iat: new Date().getTime(), + scope: scope || 'all', + uid: claims.uid, + ...(exp && { exp }), + }) analytics.track({ userId: claims.uid, event: 'generate_api_key', properties: { scope, + expiredAt: exp, env: env.server.apiEnv, }, }) - const apiKey = generateApiKey({ - iat: new Date().getTime(), - scope: scope || 'all', - uid: claims.uid, - }) - return { apiKey } } catch (error) { console.error(error) diff --git a/packages/api/src/resolvers/types.ts b/packages/api/src/resolvers/types.ts index 723346ff2..00e11b978 100644 --- a/packages/api/src/resolvers/types.ts +++ b/packages/api/src/resolvers/types.ts @@ -21,6 +21,7 @@ export interface Claims { iat: number userRole?: string scope?: string // scope is used for api key like page:search + exp?: number } export type ClaimsToSet = { diff --git a/packages/api/src/schema.ts b/packages/api/src/schema.ts index eb5fa5cf4..f8c27e5b8 100755 --- a/packages/api/src/schema.ts +++ b/packages/api/src/schema.ts @@ -1383,6 +1383,11 @@ const schema = gql` NOT_FOUND } + input GenerateApiKeyInput { + scope: String + expiredAt: Date + } + union GenerateApiKeyResult = GenerateApiKeySuccess | GenerateApiKeyError type GenerateApiKeySuccess { @@ -1562,11 +1567,11 @@ const schema = gql` login(input: LoginInput!): LoginResult! signup(input: SignupInput!): SignupResult! setLabels(input: SetLabelsInput!): SetLabelsResult! - generateApiKey(scope: String): GenerateApiKeyResult! + generateApiKey(input: GenerateApiKeyInput!): GenerateApiKeyResult! unsubscribe(name: String!): UnsubscribeResult! } - # FIXME: remove sort from feedArticles after all cahced tabs are closed + # FIXME: remove sort from feedArticles after all cached tabs are closed # FIXME: sharedOnly is legacy type Query { hello: String diff --git a/packages/api/test/resolvers/api_key.test.ts b/packages/api/test/resolvers/api_key.test.ts index 8045b7ad7..32aac8eab 100644 --- a/packages/api/test/resolvers/api_key.test.ts +++ b/packages/api/test/resolvers/api_key.test.ts @@ -2,12 +2,33 @@ import { User } from '../../src/entity/user' import { createTestUser, deleteTestUser } from '../db' import { graphqlRequest, request } from '../util' import { expect } from 'chai' +import supertest from 'supertest' + +const testAPIKey = (apiKey: string): supertest.Test => { + const query = ` + query { + articles(first: 1) { + ... on ArticlesSuccess { + edges { + cursor + } + } + ... on ArticlesError { + errorCodes + } + } + } + ` + return graphqlRequest(query, apiKey) +} describe('generate api key', () => { const username = 'fake_user' let authToken: string let user: User + let query: string + let expiredAt: string before(async () => { // create test user and login @@ -24,10 +45,12 @@ describe('generate api key', () => { await deleteTestUser(username) }) - it('should return api key which could be used to make api calls', async () => { - const query = ` + beforeEach(() => { + query = ` mutation { - generateApiKey { + generateApiKey(input: { + expiredAt: "${expiredAt}" + }) { ... on GenerateApiKeySuccess { apiKey } @@ -37,11 +60,44 @@ describe('generate api key', () => { } } ` - const response = await graphqlRequest(query, authToken).expect(200) + }) - expect(response.body.data.generateApiKey.apiKey).to.be.a('string') + context('when no expiredAt is specified', () => { + before(() => { + expiredAt = '' + }) - const apiKey = response.body.data.generateApiKey.apiKey - return graphqlRequest(query, apiKey).expect(200) + it('should generate an api key with no expiration date', async () => { + const response = await graphqlRequest(query, authToken) + expect(response.body.data.generateApiKey.apiKey).to.be.a('string') + + return testAPIKey(response.body.data.generateApiKey.apiKey).expect(200) + }) + }) + + context('when api key is not expired', () => { + before(() => { + expiredAt = new Date(Date.now() + 1000 * 60 * 60 * 24).toISOString() + }) + + it('should generate an api key', async () => { + const response = await graphqlRequest(query, authToken) + expect(response.body.data.generateApiKey.apiKey).to.be.a('string') + + return testAPIKey(response.body.data.generateApiKey.apiKey).expect(200) + }) + }) + + context('when api key is expired', () => { + before(() => { + expiredAt = new Date(Date.now() - 1000 * 60 * 60 * 24).toISOString() + }) + + it('should generate an expired api key', async () => { + const response = await graphqlRequest(query, authToken) + expect(response.body.data.generateApiKey.apiKey).to.be.a('string') + + return testAPIKey(response.body.data.generateApiKey.apiKey).expect(500) + }) }) })