From d595bfb7f143336a21e270bd45029cacc2b3d0a8 Mon Sep 17 00:00:00 2001 From: Satindar Dhillon Date: Wed, 22 Jun 2022 17:38:23 -0700 Subject: [PATCH 01/17] add delete account mutation to schema.ts --- packages/api/src/schema.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/packages/api/src/schema.ts b/packages/api/src/schema.ts index fb8766940..6564ef9c4 100755 --- a/packages/api/src/schema.ts +++ b/packages/api/src/schema.ts @@ -189,6 +189,22 @@ const schema = gql` message: String } + enum DeleteAccountErrorCode { + USER_NOT_FOUND + UNAUTHORIZED + FORBIDDEN + } + + type DeleteAccountError { + errorCodes: [DeleteAccountErrorCode!]! + } + + type DeleteAccountSuccess { + userID: ID! + } + + union DeleteAccountResult = DeleteAccountSuccess | DeleteAccountError + union UpdateUserResult = UpdateUserSuccess | UpdateUserError input UpdateUserInput { name: String! @sanitize(maxLength: 50) @@ -1752,6 +1768,7 @@ const schema = gql` googleLogin(input: GoogleLoginInput!): LoginResult! googleSignup(input: GoogleSignupInput!): GoogleSignupResult! logOut: LogOutResult! + deleteAccount(userID: ID!): DeleteAccountResult! updateUser(input: UpdateUserInput!): UpdateUserResult! updateUserProfile(input: UpdateUserProfileInput!): UpdateUserProfileResult! createArticle(input: CreateArticleInput!): CreateArticleResult! From 02411347a77635ea9996a441e1a2ec539fc17fe9 Mon Sep 17 00:00:00 2001 From: Satindar Dhillon Date: Wed, 22 Jun 2022 17:46:04 -0700 Subject: [PATCH 02/17] regenerate graphql files --- packages/api/src/generated/graphql.ts | 49 +++++++++++++++++++++++ packages/api/src/generated/schema.graphql | 17 ++++++++ 2 files changed, 66 insertions(+) diff --git a/packages/api/src/generated/graphql.ts b/packages/api/src/generated/graphql.ts index c6ef314ee..5edea20f8 100644 --- a/packages/api/src/generated/graphql.ts +++ b/packages/api/src/generated/graphql.ts @@ -424,6 +424,24 @@ export type CreateReminderSuccess = { reminder: Reminder; }; +export type DeleteAccountError = { + __typename?: 'DeleteAccountError'; + errorCodes: Array; +}; + +export enum DeleteAccountErrorCode { + Forbidden = 'FORBIDDEN', + Unauthorized = 'UNAUTHORIZED', + UserNotFound = 'USER_NOT_FOUND' +} + +export type DeleteAccountResult = DeleteAccountError | DeleteAccountSuccess; + +export type DeleteAccountSuccess = { + __typename?: 'DeleteAccountSuccess'; + userID: Scalars['ID']; +}; + export type DeleteHighlightError = { __typename?: 'DeleteHighlightError'; errorCodes: Array; @@ -863,6 +881,7 @@ export type Mutation = { createNewsletterEmail: CreateNewsletterEmailResult; createReaction: CreateReactionResult; createReminder: CreateReminderResult; + deleteAccount: DeleteAccountResult; deleteHighlight: DeleteHighlightResult; deleteHighlightReply: DeleteHighlightReplyResult; deleteLabel: DeleteLabelResult; @@ -948,6 +967,11 @@ export type MutationCreateReminderArgs = { }; +export type MutationDeleteAccountArgs = { + userID: Scalars['ID']; +}; + + export type MutationDeleteHighlightArgs = { highlightId: Scalars['ID']; }; @@ -2437,6 +2461,10 @@ export type ResolversTypes = { CreateReminderResult: ResolversTypes['CreateReminderError'] | ResolversTypes['CreateReminderSuccess']; CreateReminderSuccess: ResolverTypeWrapper; Date: ResolverTypeWrapper; + DeleteAccountError: ResolverTypeWrapper; + DeleteAccountErrorCode: DeleteAccountErrorCode; + DeleteAccountResult: ResolversTypes['DeleteAccountError'] | ResolversTypes['DeleteAccountSuccess']; + DeleteAccountSuccess: ResolverTypeWrapper; DeleteHighlightError: ResolverTypeWrapper; DeleteHighlightErrorCode: DeleteHighlightErrorCode; DeleteHighlightReplyError: ResolverTypeWrapper; @@ -2772,6 +2800,9 @@ export type ResolversParentTypes = { CreateReminderResult: ResolversParentTypes['CreateReminderError'] | ResolversParentTypes['CreateReminderSuccess']; CreateReminderSuccess: CreateReminderSuccess; Date: Scalars['Date']; + DeleteAccountError: DeleteAccountError; + DeleteAccountResult: ResolversParentTypes['DeleteAccountError'] | ResolversParentTypes['DeleteAccountSuccess']; + DeleteAccountSuccess: DeleteAccountSuccess; DeleteHighlightError: DeleteHighlightError; DeleteHighlightReplyError: DeleteHighlightReplyError; DeleteHighlightReplyResult: ResolversParentTypes['DeleteHighlightReplyError'] | ResolversParentTypes['DeleteHighlightReplySuccess']; @@ -3274,6 +3305,20 @@ export interface DateScalarConfig extends GraphQLScalarTypeConfig = { + errorCodes?: Resolver, ParentType, ContextType>; + __isTypeOf?: IsTypeOfResolverFn; +}; + +export type DeleteAccountResultResolvers = { + __resolveType: TypeResolveFn<'DeleteAccountError' | 'DeleteAccountSuccess', ParentType, ContextType>; +}; + +export type DeleteAccountSuccessResolvers = { + userID?: Resolver; + __isTypeOf?: IsTypeOfResolverFn; +}; + export type DeleteHighlightErrorResolvers = { errorCodes?: Resolver, ParentType, ContextType>; __isTypeOf?: IsTypeOfResolverFn; @@ -3617,6 +3662,7 @@ export type MutationResolvers; createReaction?: Resolver>; createReminder?: Resolver>; + deleteAccount?: Resolver>; deleteHighlight?: Resolver>; deleteHighlightReply?: Resolver>; deleteLabel?: Resolver>; @@ -4394,6 +4440,9 @@ export type Resolvers = { CreateReminderResult?: CreateReminderResultResolvers; CreateReminderSuccess?: CreateReminderSuccessResolvers; Date?: GraphQLScalarType; + DeleteAccountError?: DeleteAccountErrorResolvers; + DeleteAccountResult?: DeleteAccountResultResolvers; + DeleteAccountSuccess?: DeleteAccountSuccessResolvers; DeleteHighlightError?: DeleteHighlightErrorResolvers; DeleteHighlightReplyError?: DeleteHighlightReplyErrorResolvers; DeleteHighlightReplyResult?: DeleteHighlightReplyResultResolvers; diff --git a/packages/api/src/generated/schema.graphql b/packages/api/src/generated/schema.graphql index 38349889d..73e5b5bb2 100644 --- a/packages/api/src/generated/schema.graphql +++ b/packages/api/src/generated/schema.graphql @@ -371,6 +371,22 @@ type CreateReminderSuccess { scalar Date +type DeleteAccountError { + errorCodes: [DeleteAccountErrorCode!]! +} + +enum DeleteAccountErrorCode { + FORBIDDEN + UNAUTHORIZED + USER_NOT_FOUND +} + +union DeleteAccountResult = DeleteAccountError | DeleteAccountSuccess + +type DeleteAccountSuccess { + userID: ID! +} + type DeleteHighlightError { errorCodes: [DeleteHighlightErrorCode!]! } @@ -766,6 +782,7 @@ type Mutation { createNewsletterEmail: CreateNewsletterEmailResult! createReaction(input: CreateReactionInput!): CreateReactionResult! createReminder(input: CreateReminderInput!): CreateReminderResult! + deleteAccount(userID: ID!): DeleteAccountResult! deleteHighlight(highlightId: ID!): DeleteHighlightResult! deleteHighlightReply(highlightReplyId: ID!): DeleteHighlightReplyResult! deleteLabel(id: ID!): DeleteLabelResult! From da76ae7aab4020281043e06defbf2e1a5dc7b881 Mon Sep 17 00:00:00 2001 From: Satindar Dhillon Date: Wed, 22 Jun 2022 20:00:37 -0700 Subject: [PATCH 03/17] attempt to write resolver functions --- packages/api/src/datalayer/user/index.ts | 11 +++++++ packages/api/src/resolvers/user/index.ts | 39 ++++++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/packages/api/src/datalayer/user/index.ts b/packages/api/src/datalayer/user/index.ts index 16bee8312..ef39505b1 100644 --- a/packages/api/src/datalayer/user/index.ts +++ b/packages/api/src/datalayer/user/index.ts @@ -327,6 +327,17 @@ class UserModel extends DataModel { } return this.kx.transaction((tx) => this.updateProfile(userId, set, tx)) } + + @logMethod + deleteUser(userId: string, tx: Knex.Transaction): boolean { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const result = super.delete(userId, tx) as any + if (result.error) { + return false + } else { + return true + } + } } export default UserModel diff --git a/packages/api/src/resolvers/user/index.ts b/packages/api/src/resolvers/user/index.ts index 3c8673c5f..90d4786d7 100644 --- a/packages/api/src/resolvers/user/index.ts +++ b/packages/api/src/resolvers/user/index.ts @@ -1,4 +1,8 @@ import { + DeleteAccountSuccess, + DeleteAccountError, + DeleteAccountErrorCode, + MutationDeleteAccountArgs, GoogleSignupResult, LoginErrorCode, LoginResult, @@ -355,3 +359,38 @@ export const signupResolver: ResolverFn< return { errorCodes: [SignupErrorCode.Unknown] } } } + +export const deleteAccountResolver = authorized< + DeleteAccountSuccess, + DeleteAccountError, + MutationDeleteAccountArgs +>(async (_, { userID }, { models, claims, log }) => { + const user = await models.user.get(userID) + + if (!user || user.id !== claims.uid) { + return { + errorCodes: [DeleteAccountErrorCode.Unauthorized], + } + } + + const deleteUserResult = await authTrx((tx) => + models.user.deleteUser(userID as string, tx) + ) + + if (!deleteUserResult) { + return { + errorCodes: [DeleteAccountErrorCode.Forbidden], + } + } + + log.info('Deleting a user account', { + userID, + labels: { + source: 'resolver', + resolver: 'deleteAccountResolver', + uid: claims.uid, + }, + }) + + return { userID } +}) From ddba50dee6257ac953d4431f4fdba32df982ef23 Mon Sep 17 00:00:00 2001 From: Satindar Dhillon Date: Wed, 22 Jun 2022 21:29:45 -0700 Subject: [PATCH 04/17] fix delete user resolver and map it to deleteAccount mutation --- packages/api/src/datalayer/user/index.ts | 15 ++++++--------- .../api/src/resolvers/function_resolvers.ts | 2 ++ packages/api/src/resolvers/user/index.ts | 19 ++++++++++--------- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/packages/api/src/datalayer/user/index.ts b/packages/api/src/datalayer/user/index.ts index ef39505b1..95e81000a 100644 --- a/packages/api/src/datalayer/user/index.ts +++ b/packages/api/src/datalayer/user/index.ts @@ -10,7 +10,7 @@ import { UpdateSet, UserData, } from './model' -import DataModel, { MAX_RECORDS_LIMIT } from '../model' +import DataModel, { DataModelError, MAX_RECORDS_LIMIT } from '../model' import Knex from 'knex' import { ENABLE_DB_REQUEST_LOGGING, globalCounter, logMethod } from '../helpers' import { Table } from '../../utils/dictionary' @@ -329,14 +329,11 @@ class UserModel extends DataModel { } @logMethod - deleteUser(userId: string, tx: Knex.Transaction): boolean { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const result = super.delete(userId, tx) as any - if (result.error) { - return false - } else { - return true - } + deleteUser( + userId: string, + tx: Knex.Transaction + ): Promise { + return super.delete(userId, tx) } } diff --git a/packages/api/src/resolvers/function_resolvers.ts b/packages/api/src/resolvers/function_resolvers.ts index a3e2de75f..910da40d2 100644 --- a/packages/api/src/resolvers/function_resolvers.ts +++ b/packages/api/src/resolvers/function_resolvers.ts @@ -30,6 +30,7 @@ import { createLabelResolver, createNewsletterEmailResolver, createReminderResolver, + deleteAccountResolver, deleteHighlightResolver, deleteLabelResolver, deleteNewsletterEmailResolver, @@ -118,6 +119,7 @@ export const functionResolvers = { googleLogin: googleLoginResolver, googleSignup: googleSignupResolver, logOut: logOutResolver, + deleteAccount: deleteAccountResolver, saveArticleReadingProgress: saveArticleReadingProgressResolver, updateUser: updateUserResolver, updateUserProfile: updateUserProfileResolver, diff --git a/packages/api/src/resolvers/user/index.ts b/packages/api/src/resolvers/user/index.ts index 90d4786d7..4ec6f1261 100644 --- a/packages/api/src/resolvers/user/index.ts +++ b/packages/api/src/resolvers/user/index.ts @@ -38,6 +38,7 @@ import { validateUsername } from '../../utils/usernamePolicy' import * as jwt from 'jsonwebtoken' import { createUser } from '../../services/create_user' import { comparePassword, hashPassword } from '../../utils/auth' +import type { UserData } from '../../datalayer/user/model' export const updateUserResolver = authorized< UpdateUserSuccess, @@ -364,7 +365,7 @@ export const deleteAccountResolver = authorized< DeleteAccountSuccess, DeleteAccountError, MutationDeleteAccountArgs ->(async (_, { userID }, { models, claims, log }) => { +>(async (_, { userID }, { models, claims, log, authTrx }) => { const user = await models.user.get(userID) if (!user || user.id !== claims.uid) { @@ -374,15 +375,9 @@ export const deleteAccountResolver = authorized< } const deleteUserResult = await authTrx((tx) => - models.user.deleteUser(userID as string, tx) + models.user.deleteUser(claims.uid, tx) ) - if (!deleteUserResult) { - return { - errorCodes: [DeleteAccountErrorCode.Forbidden], - } - } - log.info('Deleting a user account', { userID, labels: { @@ -392,5 +387,11 @@ export const deleteAccountResolver = authorized< }, }) - return { userID } + if ((deleteUserResult as UserData).id !== undefined) { + return { userID } + } else { + return { + errorCodes: [DeleteAccountErrorCode.Forbidden], + } + } }) From 75b22fbe67c3ecf452f10ca73a57f0dd3cc2226d Mon Sep 17 00:00:00 2001 From: Satindar Dhillon Date: Thu, 23 Jun 2022 13:00:40 -0700 Subject: [PATCH 05/17] update swift braphql schema --- .../Services/DataService/GQLSchema.swift | 240 ++++++++++++++++++ 1 file changed, 240 insertions(+) diff --git a/apple/OmnivoreKit/Sources/Services/DataService/GQLSchema.swift b/apple/OmnivoreKit/Sources/Services/DataService/GQLSchema.swift index d0242d354..d06651a19 100644 --- a/apple/OmnivoreKit/Sources/Services/DataService/GQLSchema.swift +++ b/apple/OmnivoreKit/Sources/Services/DataService/GQLSchema.swift @@ -3382,6 +3382,136 @@ extension Selection where TypeLock == Never, Type == Never { typealias CreateReminderSuccess = Selection } +extension Objects { + struct DeleteAccountError { + let __typename: TypeName = .deleteAccountError + let errorCodes: [String: [Enums.DeleteAccountErrorCode]] + + enum TypeName: String, Codable { + case deleteAccountError = "DeleteAccountError" + } + } +} + +extension Objects.DeleteAccountError: Decodable { + init(from decoder: Decoder) throws { + let container = try decoder.container(keyedBy: DynamicCodingKeys.self) + + var map = HashMap() + for codingKey in container.allKeys { + if codingKey.isTypenameKey { continue } + + let alias = codingKey.stringValue + let field = GraphQLField.getFieldNameFromAlias(alias) + + switch field { + case "errorCodes": + if let value = try container.decode([Enums.DeleteAccountErrorCode]?.self, forKey: codingKey) { + map.set(key: field, hash: alias, value: value as Any) + } + default: + throw DecodingError.dataCorrupted( + DecodingError.Context( + codingPath: decoder.codingPath, + debugDescription: "Unknown key \(field)." + ) + ) + } + } + + errorCodes = map["errorCodes"] + } +} + +extension Fields where TypeLock == Objects.DeleteAccountError { + func errorCodes() throws -> [Enums.DeleteAccountErrorCode] { + let field = GraphQLField.leaf( + name: "errorCodes", + arguments: [] + ) + select(field) + + switch response { + case let .decoding(data): + if let data = data.errorCodes[field.alias!] { + return data + } + throw HttpError.badpayload + case .mocking: + return [] + } + } +} + +extension Selection where TypeLock == Never, Type == Never { + typealias DeleteAccountError = Selection +} + +extension Objects { + struct DeleteAccountSuccess { + let __typename: TypeName = .deleteAccountSuccess + let userId: [String: String] + + enum TypeName: String, Codable { + case deleteAccountSuccess = "DeleteAccountSuccess" + } + } +} + +extension Objects.DeleteAccountSuccess: Decodable { + init(from decoder: Decoder) throws { + let container = try decoder.container(keyedBy: DynamicCodingKeys.self) + + var map = HashMap() + for codingKey in container.allKeys { + if codingKey.isTypenameKey { continue } + + let alias = codingKey.stringValue + let field = GraphQLField.getFieldNameFromAlias(alias) + + switch field { + case "userId": + if let value = try container.decode(String?.self, forKey: codingKey) { + map.set(key: field, hash: alias, value: value as Any) + } + default: + throw DecodingError.dataCorrupted( + DecodingError.Context( + codingPath: decoder.codingPath, + debugDescription: "Unknown key \(field)." + ) + ) + } + } + + userId = map["userId"] + } +} + +extension Fields where TypeLock == Objects.DeleteAccountSuccess { + func userId() throws -> String { + let field = GraphQLField.leaf( + name: "userID", + arguments: [] + ) + select(field) + + switch response { + case let .decoding(data): + if let data = data.userId[field.alias!] { + return data + } + throw HttpError.badpayload + case .mocking: + return String.mockValue + } + } +} + +extension Selection where TypeLock == Never, Type == Never { + typealias DeleteAccountSuccess = Selection +} + extension Objects { struct DeleteHighlightError { let __typename: TypeName = .deleteHighlightError @@ -7383,6 +7513,7 @@ extension Objects { let createNewsletterEmail: [String: Unions.CreateNewsletterEmailResult] let createReaction: [String: Unions.CreateReactionResult] let createReminder: [String: Unions.CreateReminderResult] + let deleteAccount: [String: Unions.DeleteAccountResult] let deleteHighlight: [String: Unions.DeleteHighlightResult] let deleteHighlightReply: [String: Unions.DeleteHighlightReplyResult] let deleteLabel: [String: Unions.DeleteLabelResult] @@ -7480,6 +7611,10 @@ extension Objects.Mutation: Decodable { if let value = try container.decode(Unions.CreateReminderResult?.self, forKey: codingKey) { map.set(key: field, hash: alias, value: value as Any) } + case "deleteAccount": + if let value = try container.decode(Unions.DeleteAccountResult?.self, forKey: codingKey) { + map.set(key: field, hash: alias, value: value as Any) + } case "deleteHighlight": if let value = try container.decode(Unions.DeleteHighlightResult?.self, forKey: codingKey) { map.set(key: field, hash: alias, value: value as Any) @@ -7667,6 +7802,7 @@ extension Objects.Mutation: Decodable { createNewsletterEmail = map["createNewsletterEmail"] createReaction = map["createReaction"] createReminder = map["createReminder"] + deleteAccount = map["deleteAccount"] deleteHighlight = map["deleteHighlight"] deleteHighlightReply = map["deleteHighlightReply"] deleteLabel = map["deleteLabel"] @@ -7884,6 +8020,25 @@ extension Fields where TypeLock == Objects.Mutation { } } + func deleteAccount(userId: String, selection: Selection) throws -> Type { + let field = GraphQLField.composite( + name: "deleteAccount", + arguments: [Argument(name: "userID", type: "ID!", value: userId)], + selection: selection.selection + ) + select(field) + + switch response { + case let .decoding(data): + if let data = data.deleteAccount[field.alias!] { + return try selection.decode(data: data) + } + throw HttpError.badpayload + case .mocking: + return selection.mock() + } + } + func deleteHighlight(highlightId: String, selection: Selection) throws -> Type { let field = GraphQLField.composite( name: "deleteHighlight", @@ -18211,6 +18366,80 @@ extension Selection where TypeLock == Never, Type == Never { typealias CreateReminderResult = Selection } +extension Unions { + struct DeleteAccountResult { + let __typename: TypeName + let errorCodes: [String: [Enums.DeleteAccountErrorCode]] + let userId: [String: String] + + enum TypeName: String, Codable { + case deleteAccountError = "DeleteAccountError" + case deleteAccountSuccess = "DeleteAccountSuccess" + } + } +} + +extension Unions.DeleteAccountResult: Decodable { + init(from decoder: Decoder) throws { + let container = try decoder.container(keyedBy: DynamicCodingKeys.self) + + var map = HashMap() + for codingKey in container.allKeys { + if codingKey.isTypenameKey { continue } + + let alias = codingKey.stringValue + let field = GraphQLField.getFieldNameFromAlias(alias) + + switch field { + case "errorCodes": + if let value = try container.decode([Enums.DeleteAccountErrorCode]?.self, forKey: codingKey) { + map.set(key: field, hash: alias, value: value as Any) + } + case "userId": + if let value = try container.decode(String?.self, forKey: codingKey) { + map.set(key: field, hash: alias, value: value as Any) + } + default: + throw DecodingError.dataCorrupted( + DecodingError.Context( + codingPath: decoder.codingPath, + debugDescription: "Unknown key \(field)." + ) + ) + } + } + + __typename = try container.decode(TypeName.self, forKey: DynamicCodingKeys(stringValue: "__typename")!) + + errorCodes = map["errorCodes"] + userId = map["userId"] + } +} + +extension Fields where TypeLock == Unions.DeleteAccountResult { + func on(deleteAccountError: Selection, deleteAccountSuccess: Selection) throws -> Type { + select([GraphQLField.fragment(type: "DeleteAccountError", selection: deleteAccountError.selection), GraphQLField.fragment(type: "DeleteAccountSuccess", selection: deleteAccountSuccess.selection)]) + + switch response { + case let .decoding(data): + switch data.__typename { + case .deleteAccountError: + let data = Objects.DeleteAccountError(errorCodes: data.errorCodes) + return try deleteAccountError.decode(data: data) + case .deleteAccountSuccess: + let data = Objects.DeleteAccountSuccess(userId: data.userId) + return try deleteAccountSuccess.decode(data: data) + } + case .mocking: + return deleteAccountError.mock() + } + } +} + +extension Selection where TypeLock == Never, Type == Never { + typealias DeleteAccountResult = Selection +} + extension Unions { struct DeleteHighlightReplyResult { let __typename: TypeName @@ -22234,6 +22463,17 @@ extension Enums { } } +extension Enums { + /// DeleteAccountErrorCode + enum DeleteAccountErrorCode: String, CaseIterable, Codable { + case forbidden = "FORBIDDEN" + + case unauthorized = "UNAUTHORIZED" + + case userNotFound = "USER_NOT_FOUND" + } +} + extension Enums { /// DeleteHighlightErrorCode enum DeleteHighlightErrorCode: String, CaseIterable, Codable { From 02f0e22cff78132119cce3e4e0eac3e26e172b1b Mon Sep 17 00:00:00 2001 From: Satindar Dhillon Date: Thu, 23 Jun 2022 13:55:12 -0700 Subject: [PATCH 06/17] add UI and API call to delete account from ios app --- .../Sources/App/Views/DebugMenuView.swift | 2 +- .../App/Views/Profile/ManageAccountView.swift | 61 +++++++++++++++++++ .../App/Views/Profile/ProfileView.swift | 29 ++++++--- .../App/Views/RootView/RootViewModel.swift | 2 +- .../Authentication/Authenticator.swift | 3 +- .../Services/DataService/DataService.swift | 2 +- .../DataService/Mutations/DeleteAccount.swift | 46 ++++++++++++++ .../Sources/Utils/FeatureFlags.swift | 1 - .../UserSettings/ManageAccountView.swift | 28 --------- 9 files changed, 132 insertions(+), 42 deletions(-) create mode 100644 apple/OmnivoreKit/Sources/App/Views/Profile/ManageAccountView.swift create mode 100644 apple/OmnivoreKit/Sources/Services/DataService/Mutations/DeleteAccount.swift delete mode 100644 apple/OmnivoreKit/Sources/Views/UserSettings/ManageAccountView.swift diff --git a/apple/OmnivoreKit/Sources/App/Views/DebugMenuView.swift b/apple/OmnivoreKit/Sources/App/Views/DebugMenuView.swift index 6b6f83eae..abff9974b 100644 --- a/apple/OmnivoreKit/Sources/App/Views/DebugMenuView.swift +++ b/apple/OmnivoreKit/Sources/App/Views/DebugMenuView.swift @@ -26,7 +26,7 @@ struct DebugMenuView: View { Button( action: { - authenticator.logout() + authenticator.logout(dataService: dataService) dataService.switchAppEnvironment(appEnvironment: selectedEnvironment) }, label: { Text("Apply Changes") } diff --git a/apple/OmnivoreKit/Sources/App/Views/Profile/ManageAccountView.swift b/apple/OmnivoreKit/Sources/App/Views/Profile/ManageAccountView.swift new file mode 100644 index 000000000..940c5059c --- /dev/null +++ b/apple/OmnivoreKit/Sources/App/Views/Profile/ManageAccountView.swift @@ -0,0 +1,61 @@ +import Services +import SwiftUI +import Views + +struct ManageAccountView: View { + @EnvironmentObject var authenticator: Authenticator + @EnvironmentObject var dataService: DataService + + @State private var showDeleteAccountConfirmation = false + @StateObject private var viewModel = ProfileContainerViewModel() + + var body: some View { + #if os(iOS) + Form { + innerBody + } + #elseif os(macOS) + List { + innerBody + } + .listStyle(InsetListStyle()) + #endif + } + + var innerBody: some View { + Group { + Section { + ProfileCard(data: viewModel.profileCardData) + .task { + await viewModel.loadProfileData(dataService: dataService) + } + } + Section { + Button( + action: { + showDeleteAccountConfirmation = true + }, + label: { Text("Delete Account") } + ) + .alert(isPresented: $showDeleteAccountConfirmation) { + Alert( + title: Text("Are you sure you want to delete your account? This action can't be undone."), + primaryButton: .destructive(Text("Delete Account")) { + Task { + await viewModel.deleteAccount(dataService: dataService, authenticator: authenticator) + } + }, + secondaryButton: .cancel() + ) + } + } + + if let errorMessage = viewModel.deleteAccountErrorMessage { + Text(errorMessage) + .font(.appBody) + .foregroundColor(.red) + .multilineTextAlignment(.leading) + } + } + } +} diff --git a/apple/OmnivoreKit/Sources/App/Views/Profile/ProfileView.swift b/apple/OmnivoreKit/Sources/App/Views/Profile/ProfileView.swift index 2d4eef29d..8a5c1688c 100644 --- a/apple/OmnivoreKit/Sources/App/Views/Profile/ProfileView.swift +++ b/apple/OmnivoreKit/Sources/App/Views/Profile/ProfileView.swift @@ -7,6 +7,7 @@ import Views @MainActor final class ProfileContainerViewModel: ObservableObject { @Published var isLoading = false @Published var profileCardData = ProfileCardData() + @Published var deleteAccountErrorMessage: String? var appVersionString: String { if let appVersion = Bundle.main.object(forInfoDictionaryKey: "CFBundleShortVersionString") as? String { @@ -31,6 +32,20 @@ import Views } } + func deleteAccount(dataService: DataService, authenticator: Authenticator) async { + guard let currentViewer = dataService.currentViewer else { + deleteAccountErrorMessage = "Unable to load account information." + return + } + + do { + try await dataService.deleteAccount(userID: currentViewer.unwrappedUserID) + authenticator.logout(dataService: dataService) + } catch { + deleteAccountErrorMessage = "We were unable to delete your account." + } + } + private func loadProfileCardData(viewer: Viewer) { profileCardData = ProfileCardData( name: viewer.unwrappedName, @@ -106,14 +121,10 @@ struct ProfileView: View { } Section(footer: Text(viewModel.appVersionString)) { - if FeatureFlag.showAccountDeletion { - NavigationLink( - destination: ManageAccountView(handleAccountDeletion: { - print("delete account") - }) - ) { - Text("Manage Account") - } + NavigationLink( + destination: ManageAccountView() + ) { + Text("Manage Account") } Text("Logout") @@ -124,7 +135,7 @@ struct ProfileView: View { Alert( title: Text("Are you sure you want to logout?"), primaryButton: .destructive(Text("Confirm")) { - authenticator.logout() + authenticator.logout(dataService: dataService) }, secondaryButton: .cancel() ) diff --git a/apple/OmnivoreKit/Sources/App/Views/RootView/RootViewModel.swift b/apple/OmnivoreKit/Sources/App/Views/RootView/RootViewModel.swift index 9acf9a501..033f6f5e4 100644 --- a/apple/OmnivoreKit/Sources/App/Views/RootView/RootViewModel.swift +++ b/apple/OmnivoreKit/Sources/App/Views/RootView/RootViewModel.swift @@ -27,7 +27,7 @@ public final class RootViewModel: ObservableObject { #if DEBUG if CommandLine.arguments.contains("--uitesting") { - services.authenticator.logout() + services.authenticator.logout(dataService: services.dataService) } #endif } diff --git a/apple/OmnivoreKit/Sources/Services/Authentication/Authenticator.swift b/apple/OmnivoreKit/Sources/Services/Authentication/Authenticator.swift index cd2f0c66b..432508e68 100644 --- a/apple/OmnivoreKit/Sources/Services/Authentication/Authenticator.swift +++ b/apple/OmnivoreKit/Sources/Services/Authentication/Authenticator.swift @@ -36,7 +36,8 @@ public final class Authenticator: ObservableObject { ValetKey.authToken.value() } - public func logout() { + public func logout(dataService: DataService) { + dataService.resetCoreData() clearCreds() Authenticator.unregisterIntercomUser?() isLoggedIn = false diff --git a/apple/OmnivoreKit/Sources/Services/DataService/DataService.swift b/apple/OmnivoreKit/Sources/Services/DataService/DataService.swift index 078a6ffea..483cb3521 100644 --- a/apple/OmnivoreKit/Sources/Services/DataService/DataService.swift +++ b/apple/OmnivoreKit/Sources/Services/DataService/DataService.swift @@ -95,7 +95,7 @@ public final class DataService: ObservableObject { } } - private func resetCoreData() { + func resetCoreData() { clearCoreData() persistentContainer = PersistentContainer.make() diff --git a/apple/OmnivoreKit/Sources/Services/DataService/Mutations/DeleteAccount.swift b/apple/OmnivoreKit/Sources/Services/DataService/Mutations/DeleteAccount.swift new file mode 100644 index 000000000..197de9219 --- /dev/null +++ b/apple/OmnivoreKit/Sources/Services/DataService/Mutations/DeleteAccount.swift @@ -0,0 +1,46 @@ +import Foundation +import Models +import SwiftGraphQL + +public extension DataService { + func deleteAccount(userID: String) async throws { + enum MutationResult { + case success(id: String) + case error(errorMessage: String) + } + + let selection = Selection { + try $0.on( + deleteAccountError: .init { + .error(errorMessage: (try $0.errorCodes().first ?? .forbidden).rawValue) + }, + deleteAccountSuccess: .init { + .success(id: try $0.userId()) + } + ) + } + + let mutation = Selection.Mutation { + try $0.deleteAccount(userId: userID, selection: selection) + } + + let path = appEnvironment.graphqlPath + let headers = networker.defaultHeaders + + return try await withCheckedThrowingContinuation { continuation in + send(mutation, to: path, headers: headers) { mutationResult in + guard let payload = try? mutationResult.get() else { + continuation.resume(throwing: BasicError.message(messageText: "failed to delete user")) + return + } + + switch payload.data { + case .success: + continuation.resume() + case .error: + continuation.resume(throwing: BasicError.message(messageText: "failed to delete user")) + } + } + } + } +} diff --git a/apple/OmnivoreKit/Sources/Utils/FeatureFlags.swift b/apple/OmnivoreKit/Sources/Utils/FeatureFlags.swift index 5642e89eb..5c0aab9af 100644 --- a/apple/OmnivoreKit/Sources/Utils/FeatureFlags.swift +++ b/apple/OmnivoreKit/Sources/Utils/FeatureFlags.swift @@ -7,7 +7,6 @@ import Foundation #endif public enum FeatureFlag { - public static let showAccountDeletion = false public static let enableSnoozeFromShareExtension = false public static let enableRemindersFromShareExtension = false public static let enableReadNow = false diff --git a/apple/OmnivoreKit/Sources/Views/UserSettings/ManageAccountView.swift b/apple/OmnivoreKit/Sources/Views/UserSettings/ManageAccountView.swift deleted file mode 100644 index 1b6d589ca..000000000 --- a/apple/OmnivoreKit/Sources/Views/UserSettings/ManageAccountView.swift +++ /dev/null @@ -1,28 +0,0 @@ -import SwiftUI - -public struct ManageAccountView: View { - let handleAccountDeletion: () -> Void - @State private var showDeleteAccountConfirmation = false - - public init(handleAccountDeletion: @escaping () -> Void) { - self.handleAccountDeletion = handleAccountDeletion - } - - public var body: some View { - Button( - action: { - showDeleteAccountConfirmation = true - }, - label: { Text("Delete my account") } - ) - .alert(isPresented: $showDeleteAccountConfirmation) { - Alert( - title: Text("Are you sure you want to delete your account? This action can't be undone."), - primaryButton: .destructive(Text("Delete Account")) { - handleAccountDeletion() - }, - secondaryButton: .cancel() - ) - } - } -} From 7d7b51500c11db6d8b3236aa19b8861309deb5d3 Mon Sep 17 00:00:00 2001 From: Satindar Dhillon Date: Thu, 30 Jun 2022 08:04:23 -0700 Subject: [PATCH 07/17] migration to allow user to delete themseleves --- .../db/migrations/0086.do.grant_delete_on_user_table.sql | 9 +++++++++ .../migrations/0086.undo.grant_delete_on_user_table.sql | 9 +++++++++ 2 files changed, 18 insertions(+) create mode 100755 packages/db/migrations/0086.do.grant_delete_on_user_table.sql create mode 100755 packages/db/migrations/0086.undo.grant_delete_on_user_table.sql diff --git a/packages/db/migrations/0086.do.grant_delete_on_user_table.sql b/packages/db/migrations/0086.do.grant_delete_on_user_table.sql new file mode 100755 index 000000000..c3d235ced --- /dev/null +++ b/packages/db/migrations/0086.do.grant_delete_on_user_table.sql @@ -0,0 +1,9 @@ +-- Type: DO +-- Name: grant_delete_on_user_table +-- Description: Allows the Omnivore User to delete themselves (for delete account app feature) + +BEGIN; + +GRANT DELETE ON omnivore.user TO omnivore_user; + +COMMIT; diff --git a/packages/db/migrations/0086.undo.grant_delete_on_user_table.sql b/packages/db/migrations/0086.undo.grant_delete_on_user_table.sql new file mode 100755 index 000000000..0dd09b6ff --- /dev/null +++ b/packages/db/migrations/0086.undo.grant_delete_on_user_table.sql @@ -0,0 +1,9 @@ +-- Type: UNDO +-- Name: grant_delete_on_user_table +-- Description: Allows the Omnivore User to delete themselves (for delete account app feature) + +BEGIN; + +-- do nothing here, there's no reason to undo this migration. + +COMMIT; From e5d6386b9668dfe524f86eae5a2c87ce423dc082 Mon Sep 17 00:00:00 2001 From: Satindar Dhillon Date: Wed, 6 Jul 2022 09:49:01 -0700 Subject: [PATCH 08/17] return UserNotFound error if user does not exist in delete account trx --- packages/api/src/resolvers/user/index.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/api/src/resolvers/user/index.ts b/packages/api/src/resolvers/user/index.ts index 4ec6f1261..19ea0ddbe 100644 --- a/packages/api/src/resolvers/user/index.ts +++ b/packages/api/src/resolvers/user/index.ts @@ -368,7 +368,13 @@ export const deleteAccountResolver = authorized< >(async (_, { userID }, { models, claims, log, authTrx }) => { const user = await models.user.get(userID) - if (!user || user.id !== claims.uid) { + if (!user) { + return { + errorCodes: [DeleteAccountErrorCode.UserNotFound], + } + } + + if (user.id !== claims.uid) { return { errorCodes: [DeleteAccountErrorCode.Unauthorized], } From 5db87db1c699c633cee6cbc23b2ccac4a59b9832 Mon Sep 17 00:00:00 2001 From: Satindar Dhillon Date: Wed, 6 Jul 2022 10:23:52 -0700 Subject: [PATCH 09/17] add api tests for deleting a user --- .../resolvers/user_delete_account.test.ts | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 packages/api/test/resolvers/user_delete_account.test.ts diff --git a/packages/api/test/resolvers/user_delete_account.test.ts b/packages/api/test/resolvers/user_delete_account.test.ts new file mode 100644 index 000000000..12a2b52a5 --- /dev/null +++ b/packages/api/test/resolvers/user_delete_account.test.ts @@ -0,0 +1,69 @@ +import { createTestUser } from '../db' +import { graphqlRequest, request } from '../util' +import * as chai from 'chai' +import { expect } from 'chai' +import 'mocha' +import { User } from '../../src/entity/user' +import chaiString from 'chai-string' +import { DeleteAccountErrorCode } from '../../src/generated/graphql' + +chai.use(chaiString) + +const deleteAccountRequest = async (authToken: string, userId: string) => { + const mutation = ` + mutation { + deleteAccount( + input: { + userId: "${userId}", + } + ) { + ... on DeleteAccountSuccess { + userId + } + ... on DeleteAccountError { + errorCodes + } + } + } + ` + return graphqlRequest(mutation, authToken).expect(200) +} + +describe('the deleteAccount API', () => { + const username = 'fakeUser' + let authToken: string + let user: User + + before(async () => { + // create test user and login + user = await createTestUser(username) + const res = await request + .post('/local/debug/fake-user-login') + .send({ fakeEmail: user.email }) + + authToken = res.body.authToken + }) + + context('deleting a user that exists', () => { + it('should return a unauthorized error if authToken is invalid', async () => { + const res = await deleteAccountRequest('invalid-auth-token', user.id) + expect(res.body.data.deleteAccount.errorCodes).to.contain( + DeleteAccountErrorCode.Unauthorized + ) + }) + + it('should return the user id after a successful user deletion', async () => { + const res = await deleteAccountRequest(authToken, user.id) + expect(res.body.data.deleteAccount.userId).to.eql(user.id) + }) + }) + + context('deleting a user that does not exist', () => { + it('should return a user not found error if user id is invalid', async () => { + const res = await deleteAccountRequest(authToken, 'invalid-user-id') + expect(res.body.data.deleteAccount.errorCodes).to.contain( + DeleteAccountErrorCode.UserNotFound + ) + }) + }) +}) From b898f55bb945b9f40429a875510d651a7d85bfaa Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 7 Jul 2022 10:56:20 +0800 Subject: [PATCH 10/17] add delete account result in resolvers --- packages/api/src/resolvers/function_resolvers.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/api/src/resolvers/function_resolvers.ts b/packages/api/src/resolvers/function_resolvers.ts index 910da40d2..f0aea7a44 100644 --- a/packages/api/src/resolvers/function_resolvers.ts +++ b/packages/api/src/resolvers/function_resolvers.ts @@ -588,4 +588,5 @@ export const functionResolvers = { ...resultResolveTypeResolver('Webhook'), ...resultResolveTypeResolver('ApiKeys'), ...resultResolveTypeResolver('RevokeApiKey'), + ...resultResolveTypeResolver('DeleteAccount'), } From efd47f3f831c75357a2646a3744792d568f98618 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 7 Jul 2022 11:07:00 +0800 Subject: [PATCH 11/17] add function to delete pages by param --- packages/api/src/elastic/pages.ts | 32 ++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/packages/api/src/elastic/pages.ts b/packages/api/src/elastic/pages.ts index 64e86d63c..60fd7247e 100644 --- a/packages/api/src/elastic/pages.ts +++ b/packages/api/src/elastic/pages.ts @@ -309,7 +309,7 @@ export const getPageByParam = async ( id: body.hits.hits[0]._id, } as Page } catch (e) { - console.error('failed to get pages by param in elastic', e) + console.error('failed to get page by param in elastic', e) return undefined } } @@ -509,3 +509,33 @@ export const countByCreatedAt = async ( return 0 } } + +export const deletePagesByParam = async ( + param: Record +): Promise => { + try { + const params = { + query: { + bool: { + filter: Object.keys(param).map((key) => { + return { + term: { + [key]: param[key as K], + }, + } + }), + }, + }, + } + + const { body } = await client.deleteByQuery({ + index: INDEX_ALIAS, + body: params, + }) + + return body.deleted > 0 + } catch (e) { + console.error('failed to delete pages by param in elastic', e) + return false + } +} From 94d8891c06f7d4b7a5e60abf528fe943d70abe30 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 7 Jul 2022 12:20:29 +0800 Subject: [PATCH 12/17] add test --- packages/api/test/elastic/index.test.ts | 35 +++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/packages/api/test/elastic/index.test.ts b/packages/api/test/elastic/index.test.ts index 97fba7520..7fbdeef44 100644 --- a/packages/api/test/elastic/index.test.ts +++ b/packages/api/test/elastic/index.test.ts @@ -14,6 +14,7 @@ import { countByCreatedAt, createPage, deletePage, + deletePagesByParam, getPageById, getPageByParam, searchPages, @@ -301,4 +302,38 @@ describe('elastic api', () => { expect(result).to.be.true }) }) + + describe('deletePagesByParam', () => { + const userId = 'test user id' + + before(async () => { + // create a testing page + await createPage( + { + content: 'deletePagesByParam content', + createdAt: new Date(), + hash: '', + id: '', + pageType: PageType.Article, + readingProgressAnchorIndex: 0, + readingProgressPercent: 0, + savedAt: new Date(), + slug: 'deletePagesByParam slug', + state: ArticleSavingRequestStatus.Succeeded, + title: 'deletePagesByParam title', + url: 'https://localhost/deletePagesByParam', + userId, + }, + ctx + ) + }) + + it('deletes page by userId', async () => { + const deleted = await deletePagesByParam({ + userId, + }) + + expect(deleted).to.be.true + }) + }) }) From 41f43b54ab97d435eafe7245ca39176da5ac2178 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 7 Jul 2022 12:31:46 +0800 Subject: [PATCH 13/17] Publish delete event --- packages/api/src/elastic/pages.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/api/src/elastic/pages.ts b/packages/api/src/elastic/pages.ts index 60fd7247e..2b53c9509 100644 --- a/packages/api/src/elastic/pages.ts +++ b/packages/api/src/elastic/pages.ts @@ -511,7 +511,8 @@ export const countByCreatedAt = async ( } export const deletePagesByParam = async ( - param: Record + param: Record, + ctx: PageContext ): Promise => { try { const params = { @@ -533,7 +534,14 @@ export const deletePagesByParam = async ( body: params, }) - return body.deleted > 0 + if (body.deleted > 0) { + // * means deleting all pages of the same user + await ctx.pubsub.entityDeleted(EntityType.PAGE, '*', ctx.uid) + + return true + } + + return false } catch (e) { console.error('failed to delete pages by param in elastic', e) return false From 322e135ead0031fd166409f8c41de525d2494454 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 7 Jul 2022 12:32:55 +0800 Subject: [PATCH 14/17] delete this user pages in elastic --- packages/api/src/resolvers/user/index.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/api/src/resolvers/user/index.ts b/packages/api/src/resolvers/user/index.ts index 19ea0ddbe..c39524338 100644 --- a/packages/api/src/resolvers/user/index.ts +++ b/packages/api/src/resolvers/user/index.ts @@ -1,13 +1,13 @@ import { - DeleteAccountSuccess, DeleteAccountError, DeleteAccountErrorCode, - MutationDeleteAccountArgs, + DeleteAccountSuccess, GoogleSignupResult, LoginErrorCode, LoginResult, LogOutErrorCode, LogOutResult, + MutationDeleteAccountArgs, MutationGoogleLoginArgs, MutationGoogleSignupArgs, MutationLoginArgs, @@ -39,6 +39,7 @@ import * as jwt from 'jsonwebtoken' import { createUser } from '../../services/create_user' import { comparePassword, hashPassword } from '../../utils/auth' import type { UserData } from '../../datalayer/user/model' +import { deletePagesByParam } from '../../elastic/pages' export const updateUserResolver = authorized< UpdateUserSuccess, @@ -365,7 +366,7 @@ export const deleteAccountResolver = authorized< DeleteAccountSuccess, DeleteAccountError, MutationDeleteAccountArgs ->(async (_, { userID }, { models, claims, log, authTrx }) => { +>(async (_, { userID }, { models, claims, log, authTrx, pubsub }) => { const user = await models.user.get(userID) if (!user) { @@ -384,6 +385,9 @@ export const deleteAccountResolver = authorized< models.user.deleteUser(claims.uid, tx) ) + // delete this user's pages in elastic + await deletePagesByParam({ userId: userID }, { uid: userID, pubsub }) + log.info('Deleting a user account', { userID, labels: { From 583fee7f732695ca12c9116e3620da1c7a243013 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 7 Jul 2022 14:19:28 +0800 Subject: [PATCH 15/17] fix tests --- packages/api/test/elastic/index.test.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/api/test/elastic/index.test.ts b/packages/api/test/elastic/index.test.ts index 7fbdeef44..8b6665d02 100644 --- a/packages/api/test/elastic/index.test.ts +++ b/packages/api/test/elastic/index.test.ts @@ -329,9 +329,12 @@ describe('elastic api', () => { }) it('deletes page by userId', async () => { - const deleted = await deletePagesByParam({ - userId, - }) + const deleted = await deletePagesByParam( + { + userId, + }, + ctx + ) expect(deleted).to.be.true }) From 1b9d22cb6fc34934b746362575f62965e4788c02 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 7 Jul 2022 15:08:34 +0800 Subject: [PATCH 16/17] update elastic-test docker volumn --- docker-compose-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker-compose-test.yml b/docker-compose-test.yml index 341803d46..11500a2eb 100644 --- a/docker-compose-test.yml +++ b/docker-compose-test.yml @@ -32,7 +32,7 @@ services: - http.cors.allow-credentials=true - http.port=9201 volumes: - - ./.docker/elastic-test-data:/usr/share/elasticsearch-test/data + - ./.docker/elastic-test-data:/usr/share/elasticsearch/data ports: - "9201:9201" From de803ebbd5294ff3145bf4a333f39cd3778246bf Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Fri, 8 Jul 2022 11:59:55 +0800 Subject: [PATCH 17/17] fix auth error when deleting user in db --- packages/api/src/datalayer/user/index.ts | 10 +++++--- packages/api/src/resolvers/article/index.ts | 2 +- packages/api/src/resolvers/user/index.ts | 25 ++++++++----------- .../resolvers/user_delete_account.test.ts | 16 ++++++------ 4 files changed, 28 insertions(+), 25 deletions(-) diff --git a/packages/api/src/datalayer/user/index.ts b/packages/api/src/datalayer/user/index.ts index 95e81000a..a4f6058b2 100644 --- a/packages/api/src/datalayer/user/index.ts +++ b/packages/api/src/datalayer/user/index.ts @@ -329,11 +329,15 @@ class UserModel extends DataModel { } @logMethod - deleteUser( + async delete( userId: string, - tx: Knex.Transaction + tx?: Knex.Transaction ): Promise { - return super.delete(userId, tx) + if (tx) { + return super.delete(userId, tx) + } + + return this.kx.transaction((tx) => super.delete(userId, tx)) } } diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts index 2a1d39641..013cab0d1 100644 --- a/packages/api/src/resolvers/article/index.ts +++ b/packages/api/src/resolvers/article/index.ts @@ -306,7 +306,7 @@ export const createArticleResolver = authorized< let uploadFileUrlOverride = '' if (uploadFileId) { const uploadFileData = await authTrx(async (tx) => { - return await models.uploadFile.setFileUploadComplete(uploadFileId, tx) + return models.uploadFile.setFileUploadComplete(uploadFileId, tx) }) if (!uploadFileData || !uploadFileData.id || !uploadFileData.fileName) { return pageError( diff --git a/packages/api/src/resolvers/user/index.ts b/packages/api/src/resolvers/user/index.ts index c39524338..cf57a201a 100644 --- a/packages/api/src/resolvers/user/index.ts +++ b/packages/api/src/resolvers/user/index.ts @@ -38,7 +38,6 @@ import { validateUsername } from '../../utils/usernamePolicy' import * as jwt from 'jsonwebtoken' import { createUser } from '../../services/create_user' import { comparePassword, hashPassword } from '../../utils/auth' -import type { UserData } from '../../datalayer/user/model' import { deletePagesByParam } from '../../elastic/pages' export const updateUserResolver = authorized< @@ -366,9 +365,8 @@ export const deleteAccountResolver = authorized< DeleteAccountSuccess, DeleteAccountError, MutationDeleteAccountArgs ->(async (_, { userID }, { models, claims, log, authTrx, pubsub }) => { +>(async (_, { userID }, { models, claims, log, pubsub }) => { const user = await models.user.get(userID) - if (!user) { return { errorCodes: [DeleteAccountErrorCode.UserNotFound], @@ -381,13 +379,6 @@ export const deleteAccountResolver = authorized< } } - const deleteUserResult = await authTrx((tx) => - models.user.deleteUser(claims.uid, tx) - ) - - // delete this user's pages in elastic - await deletePagesByParam({ userId: userID }, { uid: userID, pubsub }) - log.info('Deleting a user account', { userID, labels: { @@ -397,11 +388,17 @@ export const deleteAccountResolver = authorized< }, }) - if ((deleteUserResult as UserData).id !== undefined) { - return { userID } - } else { + const deletedUser = await models.user.delete(userID) + if ('error' in deletedUser) { + log.error('Error deleting user account', deletedUser.error) + return { - errorCodes: [DeleteAccountErrorCode.Forbidden], + errorCodes: [DeleteAccountErrorCode.UserNotFound], } } + + // delete this user's pages in elastic + await deletePagesByParam({ userId: userID }, { uid: userID, pubsub }) + + return { userID } }) diff --git a/packages/api/test/resolvers/user_delete_account.test.ts b/packages/api/test/resolvers/user_delete_account.test.ts index 12a2b52a5..6859217ca 100644 --- a/packages/api/test/resolvers/user_delete_account.test.ts +++ b/packages/api/test/resolvers/user_delete_account.test.ts @@ -1,4 +1,4 @@ -import { createTestUser } from '../db' +import { createTestUser, deleteTestUser } from '../db' import { graphqlRequest, request } from '../util' import * as chai from 'chai' import { expect } from 'chai' @@ -13,12 +13,10 @@ const deleteAccountRequest = async (authToken: string, userId: string) => { const mutation = ` mutation { deleteAccount( - input: { - userId: "${userId}", - } + userID: "${userId}", ) { ... on DeleteAccountSuccess { - userId + userID } ... on DeleteAccountError { errorCodes @@ -30,7 +28,7 @@ const deleteAccountRequest = async (authToken: string, userId: string) => { } describe('the deleteAccount API', () => { - const username = 'fakeUser' + const username = 'newFakeUser' let authToken: string let user: User @@ -44,6 +42,10 @@ describe('the deleteAccount API', () => { authToken = res.body.authToken }) + after(async () => { + await deleteTestUser(username) + }) + context('deleting a user that exists', () => { it('should return a unauthorized error if authToken is invalid', async () => { const res = await deleteAccountRequest('invalid-auth-token', user.id) @@ -54,7 +56,7 @@ describe('the deleteAccount API', () => { it('should return the user id after a successful user deletion', async () => { const res = await deleteAccountRequest(authToken, user.id) - expect(res.body.data.deleteAccount.userId).to.eql(user.id) + expect(res.body.data.deleteAccount.userID).to.eql(user.id) }) })