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

toeverything · Aug 13, 2024 · 3ca562b · 3ca562b
1 parent 42b63f2
commit 3ca562b
Show file tree

Hide file tree

Showing 10 changed files with 70 additions and 104 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
@@ -63,7 +63,7 @@ export class UserFriendlyError extends Error {
     // disallow message override for `internal_server_error`
     // to avoid leak internal information to user
     let msg =
-      name === 'internal_server_error' ? defaultMsg : (message ?? defaultMsg);
+      name === 'internal_server_error' ? defaultMsg : message ?? defaultMsg;
 
     if (typeof msg === 'function') {
       msg = msg(args);
@@ -95,7 +95,7 @@ export class UserFriendlyError extends Error {
 
     new Logger(context).error(
       'Internal server error',
-      this.cause ? ((this.cause as any).stack ?? this.cause) : this.stack
+      this.cause ? (this.cause as any).stack ?? this.cause : this.stack
     );
   }
 }
@@ -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/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;
-};
diff --git 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
@@ -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)
 }`,
 };