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: prevent password reset email flooding #2076

Merged
merged 4 commits into from
Sep 28, 2022
Merged

Conversation

Tymek
Copy link
Member

@Tymek Tymek commented Sep 19, 2022

About the changes

Related to security, limit "forget password" requests. Linear 1-242

Discussion points

Do we want to inform users on the frontend about that? I think we kept the info if mail has been sent very vague before.

@vercel
Copy link

vercel bot commented Sep 19, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
unleash-docs ✅ Ready (Inspect) Visit Preview Sep 28, 2022 at 8:18AM (UTC)
2 Ignored Deployments
Name Status Preview Updated
unleash ⬜️ Ignored (Inspect) Sep 28, 2022 at 8:18AM (UTC)
unleash-monorepo-frontend ⬜️ Ignored (Inspect) Sep 28, 2022 at 8:18AM (UTC)

@github-actions
Copy link

github-actions bot commented Sep 19, 2022

Coverage report

❌ An unexpected error occurred. For more details, check console

Error: The process '/usr/local/bin/yarn' failed with exit code 1
St.
Category Percentage Covered / Total
🟢 Statements
88.58% (-2.8% 🔻)
7097/8012
🟡 Branches
78.91% (-0.64% 🔻)
1104/1399
🟢 Functions
82.34% (-3.89% 🔻)
2000/2429
🟢 Lines
88.9% (-2.41% 🔻)
6582/7404

⚠️ Details were not displayed: the report size has exceeded the limit.

Test suite run failed

Failed tests: 1/1172. Failed suites: 1/194.
  ● should create default config

    expect(received).toMatchSnapshot()

    Snapshot name: `should create default config 1`

    - Snapshot  - 0
    + Received  + 2

    @@ -66,19 +66,21 @@
            "ENABLE_DARK_MODE_SUPPORT": false,
            "anonymiseEventLog": false,
            "batchMetrics": false,
            "embedProxy": false,
            "embedProxyFrontend": false,
    +       "publicSignup": false,
          },
        },
        "flagResolver": FlagResolver {
          "experiments": {
            "ENABLE_DARK_MODE_SUPPORT": false,
            "anonymiseEventLog": false,
            "batchMetrics": false,
            "embedProxy": false,
            "embedProxyFrontend": false,
    +       "publicSignup": false,
          },
          "externalResolver": {
            "isEnabled": [Function],
          },
        },

      16 |     });
      17 |
    > 18 |     expect(config).toMatchSnapshot();
         |                    ^
      19 | });
      20 |
      21 | test('should add initApiToken for admin token from options', async () => {

      at Object.toMatchSnapshot (src/lib/create-config.test.ts:18:20)
          at runMicrotasks (<anonymous>)

Report generated by 🧪jest coverage report action from d31b8fd

@@ -400,11 +402,19 @@ class UserService {
if (!receiver) {
throw new NotFoundError(`Could not find ${receiverEmail}`);
}
if (this.passwordResetTimeouts[receiver.id]) {
return;
Copy link
Member Author

Choose a reason for hiding this comment

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

Works as a bugfix for now. More suggested improvements in: https://linear.app/unleash/issue/1-245/improve-forgotten-password-api

@Tymek Tymek requested a review from ivarconr September 26, 2022 10:37
Copy link
Contributor

@andreas-unleash andreas-unleash left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants