fix(server): change password with token should be public (#7855)

toeverything · Aug 14, 2024 · 0ba5168 · 0ba5168
1 parent 7afba6b
commit 0ba5168
Show file tree

Hide file tree

Showing 12 changed files with 78 additions and 114 deletions.
diff --git 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,18 +114,18 @@ export class AuthResolver {
       TokenType.ChangePassword,
       token,
       {
-        credential: user.id,
+        credential: userId,
       }
     );
 
     if (!valid) {
       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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -129,26 +129,23 @@ export async function sendSetPasswordEmail(
 
 export async function changePassword(
   app: INestApplication,
-  userToken: string,
+  userId: string,
   token: string,
   password: string
 ): Promise<string> {
   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
@@ -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<void>;
   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),
-            })}
-            <a href={`mailto:${email}`}>{email}</a>
-          </>
-        )
+            })
       }
     >
       {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
@@ -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<void>;
   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),
-            })}
-            <a href={`mailto:${email}`}>{email}</a>
-          </>
-        )
+            })
       }
     >
       {hasSetUp ? (

diff --git 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 <OnboardingPage user={account} onOpenAffine={onOpenAffine} />;
+      return (
+        account && <OnboardingPage user={account} onOpenAffine={onOpenAffine} />
+      );
     case 'signUp': {
       return (
-        <SignUpPage
-          user={account}
-          passwordLimits={passwordLimits}
-          onSetPassword={onSetPassword}
-          onOpenAffine={onOpenAffine}
-        />
+        account && (
+          <SignUpPage
+            user={account}
+            passwordLimits={passwordLimits}
+            onSetPassword={onSetPassword}
+            onOpenAffine={onOpenAffine}
+          />
+        )
       );
     }
     case 'signIn': {
@@ -122,7 +126,6 @@ export const AuthPage = (): ReactElement | null => {
     case 'changePassword': {
       return (
         <ChangePasswordPage
-          user={account}
           passwordLimits={passwordLimits}
           onSetPassword={onSetPassword}
           onOpenAffine={onOpenAffine}
@@ -132,7 +135,6 @@ export const AuthPage = (): ReactElement | null => {
     case 'setPassword': {
       return (
         <SetPasswordPage
-          user={account}
           passwordLimits={passwordLimits}
           onSetPassword={onSetPassword}
           onOpenAffine={onOpenAffine}
@@ -198,25 +200,3 @@ export const loader: LoaderFunction = async args => {
   }
   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 <AuthPage />;
-  }
-
-  // TODO(@eyhn): loading UI
-  return null;
-};