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

Jump to bottom

fix: Forgot password API - reject requests with invalid email format #9179

Merged
merged 9 commits into from
Oct 3, 2024
Merged

fix: Forgot password API - reject requests with invalid email format #9179

merged 9 commits into from
Oct 3, 2024

Conversation

abichan99911111
Copy link
Contributor

@abichan99911111 abichan99911111 commented Sep 27, 2024 •
edited by reiji-h
Loading

タスク

forgot-password api へのメールヘッダインジェクション対応
pr and merge: https://redmine.weseek.co.jp/issues/154297

修正後の挙動

正しい挙動

この例のエラーは、メールが正しいと判断された後のエラー
今回のタスク要件は満たしています。

image

エラー時の挙動

不適切なメールアドレスに対しエラーを出します
image

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Sep 27, 2024 •
edited
Loading

⚠️ No Changeset found

Latest commit: 6a3ce0d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@abichan99911111 abichan99911111 changed the title fix/154291-reject requests with invalid format fix: reject requests with invalid format Sep 27, 2024
@abichan99911111 abichan99911111 changed the title fix: reject requests with invalid format fix: forgot-password api - reject requests with invalid email format Sep 27, 2024
@abichan99911111 abichan99911111 force-pushed the fix/154291-fix-password-api branch from efeb4ea to 55dff1c Compare September 27, 2024 08:42
@miya miya requested review from miya and yuki-takei October 1, 2024 06:33
miya
miya requested changes Oct 3, 2024
View reviewed changes
apps/app/src/server/routes/apiv3/forgot-password.js Outdated
Comment on lines 77 to 80
const error = validationResult(req);
if (!error.isEmpty()) {
throw Error('invalid email format');
}
Copy link
Member

Choose a reason for hiding this comment

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

これは必要？ express middleware で当てている validator に引っ掛かればそちらでエラーを出してくれるはず。なので validatrion エラーがあればそもそもここに到達しないのでは？

Copy link
Member

@miya miya Oct 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

apiV3FormValidator が用意されているのでそちらを使う

Copy link
Contributor

Choose a reason for hiding this comment

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

修正しました。

@yuki-takei yuki-takei changed the title fix: forgot-password api - reject requests with invalid email format fix: Forgot password API - reject requests with invalid email format Oct 3, 2024
@yuki-takei
Copy link
Member

@mergify queue

@mergify Mergify
Copy link
Contributor

mergify bot commented Oct 3, 2024 •
edited
Loading

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 8bbf17d

mergify bot added a commit that referenced this pull request Oct 3, 2024
@mergify
Merge of #9179
fe83bb7
@mergify mergify bot mentioned this pull request Oct 3, 2024
Closed
15 tasks
@mergify mergify bot merged commit 8bbf17d into master Oct 3, 2024
18 checks passed
@mergify mergify bot deleted the fix/154291-fix-password-api branch October 3, 2024 12:18
This was referenced Oct 3, 2024
Merged
Closed
@github-actions github-actions bot mentioned this pull request Oct 16, 2024
Closed
@github-actions github-actions bot mentioned this pull request Oct 18, 2024
Merged
@yuki-takei yuki-takei mentioned this pull request Oct 31, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@reiji-h reiji-h reiji-h left review comments

@miya miya miya approved these changes

@yuki-takei yuki-takei Awaiting requested review from yuki-takei

Assignees
No one assigned
Labels
type/bug
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@abichan99911111 @yuki-takei @miya @reiji-h