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.

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

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

Merged
merged 1 commit into from
Aug 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
39 changes: 17 additions & 22 deletions packages/backend/server/src/core/auth/resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
EmailTokenNotFound,
EmailVerificationRequired,
InvalidEmailToken,
LinkExpired,
SameEmailProvided,
SkipThrottle,
Throttle,
Expand Down Expand Up @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -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);

Expand All @@ -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:
Expand Down Expand Up @@ -305,6 +299,7 @@ export class AuthResolver {
TokenType.ChangePassword,
userId
);
return this.url.link(callbackUrl, { token });

return this.url.link(callbackUrl, { userId, token });
}
}
4 changes: 4 additions & 0 deletions packages/backend/server/src/fundamentals/error/def.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
7 changes: 7 additions & 0 deletions packages/backend/server/src/fundamentals/error/errors.gen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion packages/backend/server/src/schema.gql
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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!]!
Expand Down
9 changes: 5 additions & 4 deletions packages/backend/server/tests/auth.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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');
Expand Down
13 changes: 5 additions & 8 deletions packages/backend/server/tests/utils/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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 ? (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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 ? (
Expand Down
50 changes: 15 additions & 35 deletions packages/frontend/core/src/pages/auth.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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();
Expand Down Expand Up @@ -89,6 +88,7 @@ export const AuthPage = (): ReactElement | null => {
async (password: string) => {
await changePassword({
token: searchParams.get('token') || '',
userId: searchParams.get('userId') || '',
newPassword: password,
});
},
Expand All @@ -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': {
Expand All @@ -122,7 +126,6 @@ export const AuthPage = (): ReactElement | null => {
case 'changePassword': {
return (
<ChangePasswordPage
user={account}
passwordLimits={passwordLimits}
onSetPassword={onSetPassword}
onOpenAffine={onOpenAffine}
Expand All @@ -132,7 +135,6 @@ export const AuthPage = (): ReactElement | null => {
case 'setPassword': {
return (
<SetPasswordPage
user={account}
passwordLimits={passwordLimits}
onSetPassword={onSetPassword}
onOpenAffine={onOpenAffine}
Expand Down Expand Up @@ -198,25 +200,3 @@ export const loader: LoaderFunction = async args => {
}
return null;
forehalo marked this conversation as resolved.
Show resolved Hide resolved
};

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;
};
Loading
Loading