From 0ba516866fa7f33c1d58cab8e3e01a36b71f60b0 Mon Sep 17 00:00:00 2001 From: forehalo Date: Wed, 14 Aug 2024 03:34:35 +0000 Subject: [PATCH] fix(server): change password with token should be public (#7855) --- .../backend/server/src/core/auth/resolver.ts | 39 +++++++-------- .../server/src/fundamentals/error/def.ts | 4 ++ .../src/fundamentals/error/errors.gen.ts | 7 +++ packages/backend/server/src/schema.gql | 3 +- packages/backend/server/tests/auth.e2e.ts | 9 ++-- packages/backend/server/tests/utils/user.ts | 13 ++--- .../auth-components/change-password-page.tsx | 22 ++------ .../auth-components/set-password-page.tsx | 22 ++------ packages/frontend/core/src/pages/auth.tsx | 50 ++++++------------- .../graphql/src/graphql/change-password.gql | 10 ++-- .../frontend/graphql/src/graphql/index.ts | 6 +-- packages/frontend/graphql/src/schema.ts | 7 ++- 12 files changed, 78 insertions(+), 114 deletions(-) diff --git a/packages/backend/server/src/core/auth/resolver.ts b/packages/backend/server/src/core/auth/resolver.ts index 11672cf262..ea68df129c 100644 --- a/packages/backend/server/src/core/auth/resolver.ts +++ b/packages/backend/server/src/core/auth/resolver.ts @@ -16,6 +16,7 @@ import { EmailTokenNotFound, EmailVerificationRequired, InvalidEmailToken, + LinkExpired, SameEmailProvided, SkipThrottle, Throttle, @@ -89,12 +90,17 @@ export class AuthResolver { }; } - @Mutation(() => UserType) + @Public() + @Mutation(() => Boolean) async changePassword( - @CurrentUser() user: CurrentUser, @Args('token') token: string, - @Args('newPassword') newPassword: string + @Args('newPassword') newPassword: string, + @Args('userId', { type: () => String, nullable: true }) userId?: string ) { + if (!userId) { + throw new LinkExpired(); + } + const config = await this.config.runtime.fetchAll({ 'auth/password.max': true, 'auth/password.min': true, @@ -108,7 +114,7 @@ export class AuthResolver { TokenType.ChangePassword, token, { - credential: user.id, + credential: userId, } ); @@ -116,10 +122,10 @@ export class AuthResolver { throw new InvalidEmailToken(); } - await this.auth.changePassword(user.id, newPassword); - await this.auth.revokeUserSessions(user.id); + await this.auth.changePassword(userId, newPassword); + await this.auth.revokeUserSessions(userId); - return user; + return true; } @Mutation(() => UserType) @@ -163,7 +169,7 @@ export class AuthResolver { user.id ); - const url = this.url.link(callbackUrl, { token }); + const url = this.url.link(callbackUrl, { userId: user.id, token }); const res = await this.auth.sendChangePasswordEmail(user.email, url); @@ -176,19 +182,7 @@ export class AuthResolver { @Args('callbackUrl') callbackUrl: string, @Args('email', { nullable: true }) _email?: string ) { - if (!user.emailVerified) { - throw new EmailVerificationRequired(); - } - - const token = await this.token.createToken( - TokenType.ChangePassword, - user.id - ); - - const url = this.url.link(callbackUrl, { token }); - - const res = await this.auth.sendSetPasswordEmail(user.email, url); - return !res.rejected.length; + return this.sendChangePasswordEmail(user, callbackUrl); } // The change email step is: @@ -305,6 +299,7 @@ export class AuthResolver { TokenType.ChangePassword, userId ); - return this.url.link(callbackUrl, { token }); + + return this.url.link(callbackUrl, { userId, token }); } } diff --git a/packages/backend/server/src/fundamentals/error/def.ts b/packages/backend/server/src/fundamentals/error/def.ts index 003ba03c7b..2a65225489 100644 --- a/packages/backend/server/src/fundamentals/error/def.ts +++ b/packages/backend/server/src/fundamentals/error/def.ts @@ -279,6 +279,10 @@ export const USER_FRIENDLY_ERRORS = { type: 'invalid_input', message: 'An invalid email token provided.', }, + link_expired: { + type: 'bad_request', + message: 'The link has expired.', + }, // Authentication & Permission Errors authentication_required: { diff --git a/packages/backend/server/src/fundamentals/error/errors.gen.ts b/packages/backend/server/src/fundamentals/error/errors.gen.ts index 883b9c770a..9c9ed2f66e 100644 --- a/packages/backend/server/src/fundamentals/error/errors.gen.ts +++ b/packages/backend/server/src/fundamentals/error/errors.gen.ts @@ -137,6 +137,12 @@ export class InvalidEmailToken extends UserFriendlyError { } } +export class LinkExpired extends UserFriendlyError { + constructor(message?: string) { + super('bad_request', 'link_expired', message); + } +} + export class AuthenticationRequired extends UserFriendlyError { constructor(message?: string) { super('authentication_required', 'authentication_required', message); @@ -520,6 +526,7 @@ export enum ErrorNames { SIGN_UP_FORBIDDEN, EMAIL_TOKEN_NOT_FOUND, INVALID_EMAIL_TOKEN, + LINK_EXPIRED, AUTHENTICATION_REQUIRED, ACTION_FORBIDDEN, ACCESS_DENIED, diff --git a/packages/backend/server/src/schema.gql b/packages/backend/server/src/schema.gql index 1f9d490430..cf53902eef 100644 --- a/packages/backend/server/src/schema.gql +++ b/packages/backend/server/src/schema.gql @@ -235,6 +235,7 @@ enum ErrorNames { INVALID_OAUTH_CALLBACK_STATE INVALID_PASSWORD_LENGTH INVALID_RUNTIME_CONFIG_TYPE + LINK_EXPIRED MAILER_SERVICE_IS_NOT_CONFIGURED MEMBER_QUOTA_EXCEEDED MISSING_OAUTH_QUERY_PARAMETER @@ -409,7 +410,7 @@ type Mutation { addWorkspaceFeature(feature: FeatureType!, workspaceId: String!): Int! cancelSubscription(idempotencyKey: String!, plan: SubscriptionPlan = Pro): UserSubscription! changeEmail(email: String!, token: String!): UserType! - changePassword(newPassword: String!, token: String!): UserType! + changePassword(newPassword: String!, token: String!, userId: String): Boolean! """Cleanup sessions""" cleanupCopilotSession(options: DeleteSessionInput!): [String!]! diff --git a/packages/backend/server/tests/auth.e2e.ts b/packages/backend/server/tests/auth.e2e.ts index 571829684b..965f5c98a8 100644 --- a/packages/backend/server/tests/auth.e2e.ts +++ b/packages/backend/server/tests/auth.e2e.ts @@ -132,13 +132,14 @@ test('set and change password', async t => { ); const newPassword = randomBytes(16).toString('hex'); - const userId = await changePassword( + const success = await changePassword( app, - u1.token.token, + u1.id, setPasswordToken as string, newPassword ); - t.is(u1.id, userId, 'failed to set password'); + + t.true(success, 'failed to change password'); const ret = auth.signIn(u1Email, newPassword); t.notThrowsAsync(ret, 'failed to check password'); @@ -201,7 +202,7 @@ test('should revoke token after change user identify', async t => { await sendSetPasswordEmail(app, u3.token.token, u3Email, 'affine.pro'); const token = await getTokenFromLatestMailMessage(); const newPassword = randomBytes(16).toString('hex'); - await changePassword(app, u3.token.token, token as string, newPassword); + await changePassword(app, u3.id, token as string, newPassword); const user = await currentUser(app, u3.token.token); t.is(user, null, 'token should be revoked'); diff --git a/packages/backend/server/tests/utils/user.ts b/packages/backend/server/tests/utils/user.ts index 5e5265d1f4..7f7a170e74 100644 --- a/packages/backend/server/tests/utils/user.ts +++ b/packages/backend/server/tests/utils/user.ts @@ -129,26 +129,23 @@ export async function sendSetPasswordEmail( export async function changePassword( app: INestApplication, - userToken: string, + userId: string, token: string, password: string ): Promise { const res = await request(app.getHttpServer()) .post(gql) - .auth(userToken, { type: 'bearer' }) .set({ 'x-request-id': 'test', 'x-operation-name': 'test' }) .send({ query: ` - mutation changePassword($token: String!, $password: String!) { - changePassword(token: $token, newPassword: $password) { - id - } + mutation changePassword($token: String!, $userId: String!, $password: String!) { + changePassword(token: $token, userId: $userId, newPassword: $password) } `, - variables: { token, password }, + variables: { token, password, userId }, }) .expect(200); - return res.body.data.changePassword.id; + return res.body.data.changePassword; } export async function sendVerifyChangeEmail( diff --git a/packages/frontend/component/src/components/auth-components/change-password-page.tsx b/packages/frontend/component/src/components/auth-components/change-password-page.tsx index 94f7d402ea..9b3747afd3 100644 --- a/packages/frontend/component/src/components/auth-components/change-password-page.tsx +++ b/packages/frontend/component/src/components/auth-components/change-password-page.tsx @@ -7,19 +7,12 @@ import { Button } from '../../ui/button'; import { notify } from '../../ui/notification'; import { AuthPageContainer } from './auth-page-container'; import { SetPassword } from './set-password'; -import type { User } from './type'; export const ChangePasswordPage: FC<{ - user: User; passwordLimits: PasswordLimitsFragment; onSetPassword: (password: string) => Promise; onOpenAffine: () => void; -}> = ({ - user: { email }, - passwordLimits, - onSetPassword: propsOnSetPassword, - onOpenAffine, -}) => { +}> = ({ passwordLimits, onSetPassword: propsOnSetPassword, onOpenAffine }) => { const t = useI18n(); const [hasSetUp, setHasSetUp] = useState(false); @@ -45,17 +38,12 @@ export const ChangePasswordPage: FC<{ : t['com.affine.auth.reset.password.page.title']() } subtitle={ - hasSetUp ? ( - t['com.affine.auth.sent.reset.password.success.message']() - ) : ( - <> - {t['com.affine.auth.page.sent.email.subtitle']({ + hasSetUp + ? t['com.affine.auth.sent.reset.password.success.message']() + : t['com.affine.auth.page.sent.email.subtitle']({ min: String(passwordLimits.minLength), max: String(passwordLimits.maxLength), - })} - {email} - - ) + }) } > {hasSetUp ? ( diff --git a/packages/frontend/component/src/components/auth-components/set-password-page.tsx b/packages/frontend/component/src/components/auth-components/set-password-page.tsx index 61536f4b27..1335981917 100644 --- a/packages/frontend/component/src/components/auth-components/set-password-page.tsx +++ b/packages/frontend/component/src/components/auth-components/set-password-page.tsx @@ -7,19 +7,12 @@ import { Button } from '../../ui/button'; import { notify } from '../../ui/notification'; import { AuthPageContainer } from './auth-page-container'; import { SetPassword } from './set-password'; -import type { User } from './type'; export const SetPasswordPage: FC<{ - user: User; passwordLimits: PasswordLimitsFragment; onSetPassword: (password: string) => Promise; onOpenAffine: () => void; -}> = ({ - user: { email }, - passwordLimits, - onSetPassword: propsOnSetPassword, - onOpenAffine, -}) => { +}> = ({ passwordLimits, onSetPassword: propsOnSetPassword, onOpenAffine }) => { const t = useI18n(); const [hasSetUp, setHasSetUp] = useState(false); @@ -45,17 +38,12 @@ export const SetPasswordPage: FC<{ : t['com.affine.auth.set.password.page.title']() } subtitle={ - hasSetUp ? ( - t['com.affine.auth.sent.set.password.success.message']() - ) : ( - <> - {t['com.affine.auth.page.sent.email.subtitle']({ + hasSetUp + ? t['com.affine.auth.sent.set.password.success.message']() + : t['com.affine.auth.page.sent.email.subtitle']({ min: String(passwordLimits.minLength), max: String(passwordLimits.maxLength), - })} - {email} - - ) + }) } > {hasSetUp ? ( diff --git a/packages/frontend/core/src/pages/auth.tsx b/packages/frontend/core/src/pages/auth.tsx index 7fb288134c..7d3dac4f2d 100644 --- a/packages/frontend/core/src/pages/auth.tsx +++ b/packages/frontend/core/src/pages/auth.tsx @@ -17,8 +17,7 @@ import { } from '@affine/graphql'; import { useI18n } from '@affine/i18n'; import { useLiveData, useService } from '@toeverything/infra'; -import type { ReactElement } from 'react'; -import { useCallback, useEffect } from 'react'; +import { useCallback } from 'react'; import type { LoaderFunction } from 'react-router-dom'; import { redirect, useParams, useSearchParams } from 'react-router-dom'; import { z } from 'zod'; @@ -39,7 +38,7 @@ const authTypeSchema = z.enum([ 'verify-email', ]); -export const AuthPage = (): ReactElement | null => { +export const Component = () => { const authService = useService(AuthService); const account = useLiveData(authService.session.account$); const t = useI18n(); @@ -89,6 +88,7 @@ export const AuthPage = (): ReactElement | null => { async (password: string) => { await changePassword({ token: searchParams.get('token') || '', + userId: searchParams.get('userId') || '', newPassword: password, }); }, @@ -98,22 +98,26 @@ export const AuthPage = (): ReactElement | null => { jumpToIndex(RouteLogic.REPLACE); }, [jumpToIndex]); - if (!passwordLimits || !account) { + if (!passwordLimits) { // TODO(@eyhn): loading UI return null; } switch (authType) { case 'onboarding': - return ; + return ( + account && + ); case 'signUp': { return ( - + account && ( + + ) ); } case 'signIn': { @@ -122,7 +126,6 @@ export const AuthPage = (): ReactElement | null => { case 'changePassword': { return ( { case 'setPassword': { return ( { } return null; }; - -export const Component = () => { - const authService = useService(AuthService); - const isRevalidating = useLiveData(authService.session.isRevalidating$); - const loginStatus = useLiveData(authService.session.status$); - const { jumpToExpired } = useNavigateHelper(); - - useEffect(() => { - authService.session.revalidate(); - }, [authService]); - - if (loginStatus === 'unauthenticated' && !isRevalidating) { - jumpToExpired(RouteLogic.REPLACE); - } - - if (loginStatus === 'authenticated') { - return ; - } - - // TODO(@eyhn): loading UI - return null; -}; diff --git a/packages/frontend/graphql/src/graphql/change-password.gql b/packages/frontend/graphql/src/graphql/change-password.gql index fb60b1a952..2e74373502 100644 --- a/packages/frontend/graphql/src/graphql/change-password.gql +++ b/packages/frontend/graphql/src/graphql/change-password.gql @@ -1,5 +1,7 @@ -mutation changePassword($token: String!, $newPassword: String!) { - changePassword(token: $token, newPassword: $newPassword) { - id - } +mutation changePassword( + $token: String! + $userId: String! + $newPassword: String! +) { + changePassword(token: $token, userId: $userId, newPassword: $newPassword) } diff --git a/packages/frontend/graphql/src/graphql/index.ts b/packages/frontend/graphql/src/graphql/index.ts index 295bf20483..b78f0d4eb4 100644 --- a/packages/frontend/graphql/src/graphql/index.ts +++ b/packages/frontend/graphql/src/graphql/index.ts @@ -121,10 +121,8 @@ export const changePasswordMutation = { definitionName: 'changePassword', containsFile: false, query: ` -mutation changePassword($token: String!, $newPassword: String!) { - changePassword(token: $token, newPassword: $newPassword) { - id - } +mutation changePassword($token: String!, $userId: String!, $newPassword: String!) { + changePassword(token: $token, userId: $userId, newPassword: $newPassword) }`, }; diff --git a/packages/frontend/graphql/src/schema.ts b/packages/frontend/graphql/src/schema.ts index 57a2b29fb8..509d1b9745 100644 --- a/packages/frontend/graphql/src/schema.ts +++ b/packages/frontend/graphql/src/schema.ts @@ -306,6 +306,7 @@ export enum ErrorNames { INVALID_OAUTH_CALLBACK_STATE = 'INVALID_OAUTH_CALLBACK_STATE', INVALID_PASSWORD_LENGTH = 'INVALID_PASSWORD_LENGTH', INVALID_RUNTIME_CONFIG_TYPE = 'INVALID_RUNTIME_CONFIG_TYPE', + LINK_EXPIRED = 'LINK_EXPIRED', MAILER_SERVICE_IS_NOT_CONFIGURED = 'MAILER_SERVICE_IS_NOT_CONFIGURED', MEMBER_QUOTA_EXCEEDED = 'MEMBER_QUOTA_EXCEEDED', MISSING_OAUTH_QUERY_PARAMETER = 'MISSING_OAUTH_QUERY_PARAMETER', @@ -467,7 +468,7 @@ export interface Mutation { addWorkspaceFeature: Scalars['Int']['output']; cancelSubscription: UserSubscription; changeEmail: UserType; - changePassword: UserType; + changePassword: Scalars['Boolean']['output']; /** Cleanup sessions */ cleanupCopilotSession: Array; /** Create change password url */ @@ -557,6 +558,7 @@ export interface MutationChangeEmailArgs { export interface MutationChangePasswordArgs { newPassword: Scalars['String']['input']; token: Scalars['String']['input']; + userId: InputMaybe; } export interface MutationCleanupCopilotSessionArgs { @@ -1321,12 +1323,13 @@ export type CreateChangePasswordUrlMutation = { export type ChangePasswordMutationVariables = Exact<{ token: Scalars['String']['input']; + userId: Scalars['String']['input']; newPassword: Scalars['String']['input']; }>; export type ChangePasswordMutation = { __typename?: 'Mutation'; - changePassword: { __typename?: 'UserType'; id: string }; + changePassword: boolean; }; export type CopilotQuotaQueryVariables = Exact<{ [key: string]: never }>;