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

Public share protect #111

Closed
wants to merge 3 commits into from
Closed

Public share protect #111

wants to merge 3 commits into from

Conversation

haribhandari07
Copy link
Contributor

Description

This PR adds acceptance test to check if public link share is protected from brute force attacks.

Related PR

#90

@codecov
Copy link

codecov bot commented Mar 17, 2020

Codecov Report

Merging #111 into master will increase coverage by 3.78%.
The diff coverage is 77.51%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #111      +/-   ##
============================================
+ Coverage     70.00%   73.78%   +3.78%     
- Complexity       38       54      +16     
============================================
  Files            11       13       +2     
  Lines           170      267      +97     
============================================
+ Hits            119      197      +78     
- Misses           51       70      +19     
Impacted Files Coverage Δ Complexity Δ
appinfo/Migrations/Version20191109111104.php 0.00% <0.00%> (ø) 2.00 <2.00> (?)
lib/BruteForceProtectionConfig.php 100.00% <ø> (ø) 8.00 <0.00> (ø)
templates/settings-admin.php 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
lib/Db/FailedLinkAccessMapper.php 100.00% <100.00%> (ø) 5.00 <5.00> (?)
lib/Db/FailedLoginAttemptMapper.php 100.00% <100.00%> (ø) 5.00 <2.00> (ø)
lib/Hooks.php 100.00% <100.00%> (+17.64%) 9.00 <7.00> (+4.00)
lib/Jobs/ExpireOldAttempts.php 100.00% <100.00%> (ø) 2.00 <0.00> (ø)
lib/Throttle.php 100.00% <100.00%> (+8.00%) 13.00 <11.00> (+5.00)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3dbc894...b4651f2. Read the comment docs.

@haribhandari07
Copy link
Contributor Author

@karakayasemi can you please explain what your PR fixes? I wrote an acceptance test to check if public link shares are brute force protected but it seems they are not.

@karakayasemi
Copy link
Contributor

@haribhandari07 public link share protection is using the same settings with login protection. The default configuration is ban the ip after 3 unsuccessful attempts. The ban period can be configured via settings.

I can see 3 attempts in your tests, if you make it 4 or if you reduce fail tolerance you can see the effect of the PR.

@phil-davis
Copy link
Contributor

Updated tests have been added to #90

@phil-davis phil-davis closed this May 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants