Skip to content

LDAP backup server should not be queried when auth fails - #10031

Merged
MorrisJobke merged 1 commit into
masterfrom
fix/9987/ldap-backupserver-connect
Jun 28, 2018
Merged

LDAP backup server should not be queried when auth fails#10031
MorrisJobke merged 1 commit into
masterfrom
fix/9987/ldap-backupserver-connect

Conversation

@blizzz

Copy link
Copy Markdown
Member

fixes#9987

We should contact the backup server only when the first one is offline. This is already being dealt with the ServerNotAvailableException exception. So, if none occurs we can directly return the bind state. And therefore we don't need to take the error code into account. Added unit test to cover this scenario.

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@codecov

codecovBot commented Jun 27, 2018

Copy link
Copy Markdown

Codecov Report

Merging #10031 into master will increase coverage by 20.27%.
The diff coverage is 100%.

@@ Coverage Diff @@## master #10031 +/- ##
=============================================
+ Coverage 31.71% 51.99% +20.27% + Complexity 26010 26007 -3 
=============================================
Files 1661 1661 Lines 96137 96132 -5 Branches 1290 1290 =============================================
+ Hits 30494 49984 +19490 + Misses 65643 46148 -19495
Impacted FilesCoverage ΔComplexity Δ
apps/user_ldap/lib/Connection.php55.18% <100%> (+55.18%)121 <0> (-3)⬇️
lib/private/Files/Stream/Encryption.php75.22% <0%> (+0.45%)54% <0%> (ø)⬇️
apps/theming/lib/ThemingDefaults.php97.29% <0%> (+0.67%)52% <0%> (ø)⬇️
lib/private/Group/Group.php76.19% <0%> (+0.79%)62% <0%> (ø)⬇️
lib/private/Setup.php12.55% <0%> (+0.83%)52% <0%> (ø)⬇️
lib/public/AppFramework/Db/Mapper.php94.95% <0%> (+0.84%)30% <0%> (ø)⬇️
...vate/Authentication/Token/DefaultTokenProvider.php98.05% <0%> (+0.97%)32% <0%> (ø)⬇️
lib/private/Preview/Generator.php78.28% <0%> (+1.14%)53% <0%> (ø)⬇️
lib/private/App/DependencyAnalyzer.php95.62% <0%> (+1.25%)64% <0%> (ø)⬇️
lib/private/Settings/Manager.php65% <0%> (+1.42%)41% <0%> (ø)⬇️
... and 381 more

@skjnldsvskjnldsv left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

make sense

@MorrisJobkeMorrisJobke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🐘

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to reviewWaiting for reviewsbugfeature: ldap

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unexpected behavior when using LDAP backup server

3 participants

@blizzz@MorrisJobke@skjnldsv