Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/User email verification #1874

Merged
merged 12 commits into from
Nov 22, 2024
Merged

Feat/User email verification #1874

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
0c4082d
Feat/User email verification
kkatusic Nov 14, 2024
d8897fa
added confirmation flow for use inputed code
kkatusic Nov 18, 2024
a7b30f6
added restriction for project and solved verification process
kkatusic Nov 19, 2024
a006f8a
added tests for verification and bypassing first user update
kkatusic Nov 20, 2024
c432106
fixing project resolver test
kkatusic Nov 20, 2024
f649763
switch back variables
kkatusic Nov 20, 2024
f047276
update jwt token with isEmailVerified
kkatusic Nov 20, 2024
4b5a37d
fixing project resolver test
kkatusic Nov 21, 2024
7708ba1
remove bypassing
kkatusic Nov 21, 2024
105550a
fixing tests
kkatusic Nov 21, 2024
ae87169
fixing project resolver test
kkatusic Nov 21, 2024
ddaef95
fixing user resolver test
kkatusic Nov 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion config/test.env
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,4 +257,4 @@ STELLAR_HORIZON_API_URL=https://horizon.stellar.org
STELLAR_SCAN_API_URL=https://stellar.expert/explorer/public

ENDAOMENT_ADMIN_WALLET_ADDRESS=0xfE3524e04E4e564F9935D34bB5e80c5CaB07F5b4
SYNC_USER_MODEL_SCORE_CRONJOB_EXPRESSION=0 0 */3 * *
SYNC_USER_MODEL_SCORE_CRONJOB_EXPRESSION=0 0 */3 * *
21 changes: 21 additions & 0 deletions migration/1731071653657-addUserEmailVerificationColumns.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { MigrationInterface, QueryRunner } from 'typeorm';

export class AddUserEmailVerificationColumns1731071653657
implements MigrationInterface
{
public async up(queryRunner: QueryRunner): Promise<void> {
queryRunner.query(`
ALTER TABLE "user"
ADD COLUMN IF NOT EXISTS "emailVerificationCode" VARCHAR,
kkatusic marked this conversation as resolved.
Show resolved Hide resolved
ADD COLUMN IF NOT EXISTS "isEmailVerified" BOOLEAN DEFAULT false;
`);
}
kkatusic marked this conversation as resolved.
Show resolved Hide resolved

public async down(queryRunner: QueryRunner): Promise<void> {
queryRunner.query(`
ALTER TABLE "user"
DROP COLUMN IF EXISTS "emailVerificationCode",
DROP COLUMN IF EXISTS "isEmailVerified";
`);
}
Comment on lines +14 to +20
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add index cleanup to down migration.

If implementing the suggested index in the up migration, ensure it's properly cleaned up in the down migration.

Apply this diff to include index cleanup:

 public async down(queryRunner: QueryRunner): Promise<void> {
     queryRunner.query(`
+        DROP INDEX IF EXISTS "IDX_USER_EMAIL_VERIFICATION_CODE";
         ALTER TABLE "user"
         DROP COLUMN IF EXISTS "emailVerificationCode",
         DROP COLUMN IF EXISTS "isEmailVerified";
     `);
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public async down(queryRunner: QueryRunner): Promise<void> {
queryRunner.query(`
ALTER TABLE "user"
DROP COLUMN IF EXISTS "emailVerificationCode",
DROP COLUMN IF EXISTS "isEmailVerified";
`);
}
public async down(queryRunner: QueryRunner): Promise<void> {
queryRunner.query(`
DROP INDEX IF EXISTS "IDX_USER_EMAIL_VERIFICATION_CODE";
ALTER TABLE "user"
DROP COLUMN IF EXISTS "emailVerificationCode",
DROP COLUMN IF EXISTS "isEmailVerified";
`);
}

}
8 changes: 8 additions & 0 deletions src/adapters/notifications/MockNotificationAdapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,14 @@ export class MockNotificationAdapter implements NotificationAdapterInterface {
return Promise.resolve(undefined);
}

sendUserEmailConfirmationCodeFlow(params: { email: string }): Promise<void> {
logger.debug(
'MockNotificationAdapter sendUserEmailConfirmationCodeFlow',
params,
);
return Promise.resolve(undefined);
}
Comment on lines +38 to +44
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Security: Avoid logging sensitive information

The debug logging includes the full params object which contains the email address. This could expose sensitive PII in logs.

Consider:

  1. Removing the params from logging
  2. Or logging only non-sensitive information like userId
  3. Or masking the email address

🛠️ Refactor suggestion

Add input validation

The method should validate the email parameter to ensure it's not undefined or empty.

Apply this diff to add validation:

   sendUserEmailConfirmationCodeFlow(params: { email: string }): Promise<void> {
+    if (!params.email?.trim()) {
+      throw new Error('Email is required');
+    }
     logger.debug(
       'MockNotificationAdapter sendUserEmailConfirmationCodeFlow',
       params,
     );
     return Promise.resolve(undefined);
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
sendUserEmailConfirmationCodeFlow(params: { email: string }): Promise<void> {
logger.debug(
'MockNotificationAdapter sendUserEmailConfirmationCodeFlow',
params,
);
return Promise.resolve(undefined);
}
sendUserEmailConfirmationCodeFlow(params: { email: string }): Promise<void> {
if (!params.email?.trim()) {
throw new Error('Email is required');
}
logger.debug(
'MockNotificationAdapter sendUserEmailConfirmationCodeFlow',
params,
);
return Promise.resolve(undefined);
}

⚠️ Potential issue

Fix interface compliance: Add missing user parameter

The method signature doesn't match the NotificationAdapterInterface. According to the AI summary, the interface requires both email and user properties in the params object.

Apply this diff to align with the interface:

-  sendUserEmailConfirmationCodeFlow(params: { email: string }): Promise<void> {
+  sendUserEmailConfirmationCodeFlow(params: { email: string; user: User }): Promise<void> {
     logger.debug(
       'MockNotificationAdapter sendUserEmailConfirmationCodeFlow',
-      params,
+      { email: params.email, userId: params.user.id },
     );
     return Promise.resolve(undefined);
   }

Committable suggestion skipped: line range outside the PR's diff.

💡 Codebase verification

Implementation mismatch detected between interface and mock adapter

The mock adapter implementation doesn't match the interface definition:

  • Interface defines: sendUserEmailConfirmationCodeFlow(params: { email: string; user: User }): Promise<void>
  • Mock implements: sendUserEmailConfirmationCodeFlow(params: { email: string }): Promise<void>

The mock adapter is missing the required user parameter in its implementation, while the resolver is correctly passing both parameters. This needs to be fixed to maintain consistency with the interface contract.

🔗 Analysis chain

Verify implementation consistency

Let's verify the interface definition and usage across the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check interface definition and usage

# Check interface definition
echo "Checking NotificationAdapterInterface definition..."
rg -A 5 "sendUserEmailConfirmationCodeFlow.*Promise" src/adapters/notifications/NotificationAdapterInterface.ts

# Check usage in resolvers
echo "Checking usage in resolvers..."
rg -A 10 "sendUserEmailConfirmationCodeFlow" src/resolvers/

# Check test coverage
echo "Checking test coverage..."
rg -A 10 "sendUserEmailConfirmationCodeFlow" "src/**/*.test.ts"

Length of output: 6007

Script:

#!/bin/bash
# Let's check the interface definition and implementation details

# Check interface definition
echo "Checking NotificationAdapterInterface definition..."
rg -A 5 "interface NotificationAdapterInterface" src/adapters/notifications/NotificationAdapterInterface.ts

# Check implementation in mock adapter
echo "Checking mock adapter implementation..."
rg -A 5 "class MockNotificationAdapter" src/adapters/notifications/MockNotificationAdapter.ts

# Check the actual usage parameters
echo "Checking actual parameters in usage..."
rg -C 5 "sendUserEmailConfirmationCodeFlow.*email" src/resolvers/userResolver.ts

# Check test implementation
echo "Checking test implementation..."
rg -C 5 "sendUserEmailConfirmationCodeFlow" src/adapters/notifications/MockNotificationAdapter.test.ts

Length of output: 1345

Script:

#!/bin/bash
# Let's check the complete interface file and implementation

# Check complete interface file
echo "Checking complete interface file..."
cat src/adapters/notifications/NotificationAdapterInterface.ts

# Check complete mock adapter implementation
echo "Checking complete mock adapter implementation..."
cat src/adapters/notifications/MockNotificationAdapter.ts

# Check the complete resolver implementation
echo "Checking complete resolver implementation..."
rg -C 10 "sendUserEmailConfirmationCodeFlow" src/resolvers/userResolver.ts

Length of output: 14412


userSuperTokensCritical(): Promise<void> {
return Promise.resolve(undefined);
}
Expand Down
5 changes: 5 additions & 0 deletions src/adapters/notifications/NotificationAdapterInterface.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ export interface NotificationAdapterInterface {
networkName: string;
}): Promise<void>;

sendUserEmailConfirmationCodeFlow(params: {
email: string;
user: User;
}): Promise<void>;
kkatusic marked this conversation as resolved.
Show resolved Hide resolved

projectVerified(params: { project: Project }): Promise<void>;
projectBoosted(params: { projectId: number; userId: number }): Promise<void>;
projectBoostedBatch(params: {
Expand Down
22 changes: 22 additions & 0 deletions src/adapters/notifications/NotificationCenterAdapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,28 @@ export class NotificationCenterAdapter implements NotificationAdapterInterface {
}
}

async sendUserEmailConfirmationCodeFlow(params: {
email: string;
user: User;
}): Promise<void> {
const { email, user } = params;
try {
await callSendNotification({
eventName:
NOTIFICATIONS_EVENT_NAMES.SEND_USER_EMAIL_CONFIRMATION_CODE_FLOW,
segment: {
payload: {
email,
verificationCode: user.emailVerificationCode,
userId: user.id,
},
},
});
} catch (e) {
logger.error('sendUserEmailConfirmationCodeFlow >> error', e);
}
}

async userSuperTokensCritical(params: {
user: User;
eventName: UserStreamBalanceWarning;
Expand Down
2 changes: 2 additions & 0 deletions src/analytics/analytics.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,6 @@ export enum NOTIFICATIONS_EVENT_NAMES {
SUBSCRIBE_ONBOARDING = 'Subscribe onboarding',
CREATE_ORTTO_PROFILE = 'Create Ortto profile',
SEND_EMAIL_CONFIRMATION = 'Send email confirmation',

SEND_USER_EMAIL_CONFIRMATION_CODE_FLOW = 'Send email confirmation code flow',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

MockNotificationAdapter needs to be updated to match the interface

The MockNotificationAdapter implementation of sendUserEmailConfirmationCodeFlow doesn't match the interface definition. The interface requires both email and user parameters, but the mock implementation only accepts email.

  • src/adapters/notifications/MockNotificationAdapter.ts: Update the method signature to sendUserEmailConfirmationCodeFlow(params: { email: string; user: User }): Promise<void>
🔗 Analysis chain

LGTM! Well-structured enum addition.

The new notification event name follows the established naming convention and clearly describes its purpose within the email verification flow.

Let's verify the usage of this new enum value in the notification adapters:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the implementation of email confirmation code flow in notification adapters
# Expected: Find implementations in MockNotificationAdapter and NotificationCenterAdapter

# Search for the new enum usage
rg -A 5 "SEND_USER_EMAIL_CONFIRMATION_CODE_FLOW"

# Search for related method implementations
ast-grep --pattern 'sendUserEmailConfirmationCodeFlow($_) {
  $$$
}'

Length of output: 816

Script:

#!/bin/bash
# Let's verify if there are any other notification adapters in the codebase
fd ".*Adapter.ts" src/adapters/notifications

# Check for any test files related to notification adapters
fd ".*Adapter.test.ts" src/adapters/notifications

# Search for any mock adapter implementations
rg -A 5 "class Mock.*Adapter"

Length of output: 1397

Script:

#!/bin/bash
# Let's check the implementation in MockNotificationAdapter for the new enum
rg -A 10 "SEND_USER_EMAIL_CONFIRMATION_CODE_FLOW" src/adapters/notifications/MockNotificationAdapter.ts

# Also check the interface definition to ensure the method is properly defined
ast-grep --pattern 'interface NotificationAdapterInterface {
  $$$
}'

Length of output: 8501

Script:

#!/bin/bash
# Let's check the implementation of sendUserEmailConfirmationCodeFlow in both adapters
rg -B 2 -A 10 "sendUserEmailConfirmationCodeFlow" src/adapters/notifications/MockNotificationAdapter.ts src/adapters/notifications/NotificationCenterAdapter.ts

Length of output: 3415

}
8 changes: 8 additions & 0 deletions src/entities/user.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export const publicSelectionFields = [
'user.totalReceived',
'user.passportScore',
'user.passportStamps',
'user.isEmailVerified',
];

export enum UserRole {
Expand Down Expand Up @@ -195,6 +196,13 @@ export class User extends BaseEntity {
@Field(_type => Float, { nullable: true })
activeQFMBDScore?: number;

@Field(_type => Boolean, { nullable: true })
@Column('bool', { default: false })
isEmailVerified: boolean;

@Column('varchar', { nullable: true, default: null })
emailVerificationCode?: string | null;

@Field(_type => Int, { nullable: true })
async donationsCount() {
// Count for non-recurring donations
Expand Down
15 changes: 15 additions & 0 deletions src/resolvers/projectResolver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1079,6 +1079,14 @@ export class ProjectResolver {
throw new Error(
i18n.__(translationErrorMessagesKeys.AUTHENTICATION_REQUIRED),
);

const dbUser = await findUserById(user.userId);

// Check if user email is verified
if (!dbUser || !dbUser.isEmailVerified) {
throw new Error(i18n.__(translationErrorMessagesKeys.EMAIL_NOT_VERIFIED));
}

const { image } = newProjectData;

// const project = await Project.findOne({ id: projectId });
Expand Down Expand Up @@ -1362,6 +1370,13 @@ export class ProjectResolver {
const user = await getLoggedInUser(ctx);
const { image, description } = projectInput;

const dbUser = await findUserById(user.id);

// Check if user email is verified
if (!dbUser || !dbUser.isEmailVerified) {
throw new Error(i18n.__(translationErrorMessagesKeys.EMAIL_NOT_VERIFIED));
}

const qualityScore = getQualityScore(description, Boolean(image), 0);

if (!projectInput.categories) {
Expand Down
Loading
Loading