Skip to content

Conversation

@CameronNemo
Copy link
Contributor

@AlexisLessard Please try to apply these changes and let me know if you still see the issue. I do not have access to an LDAP server to test with at the moment.

Closes #1118

@codecov
Copy link

codecov bot commented Sep 14, 2019

Codecov Report

Merging #1119 into master will decrease coverage by 0.02%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1119      +/-   ##
==========================================
- Coverage   76.01%   75.99%   -0.03%     
==========================================
  Files          55       55              
  Lines        8598     8601       +3     
==========================================
  Hits         6536     6536              
- Misses       2062     2065       +3
Impacted Files Coverage Δ
flask_appbuilder/security/manager.py 51.34% <0%> (-0.22%) ⬇️

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 ab60302...83d380f. Read the comment docs.

@AlexisLessard
Copy link

AlexisLessard commented Sep 16, 2019

The following exception appears in my logs:

[2019-09-16 09:37:49,624] {manager.py:874} ERROR - {'desc': 'Bad search filter'}

The exception is written litteraly, and the app doesn't crash. User isn't impacted, and sysadmin can search for the error. Great job, @CameronNemo! Thx!

I've tested the change ad hoc, but if it follows in a following version, and when airflow updates the version is uses, I'll be able to use it in production.

@AlexisLessard
Copy link

If this PR is approved, you may close #1118

@dpgaspar
Copy link
Owner

Thank you (once more) @CameronNemo

@dpgaspar dpgaspar merged commit 3780a1f into dpgaspar:master Sep 18, 2019
@dpgaspar dpgaspar mentioned this pull request Sep 27, 2019
@dpgaspar dpgaspar mentioned this pull request Oct 16, 2019
@nytai nytai mentioned this pull request Nov 6, 2019
12 tasks
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.

Incorrect LDAP filter raises a critical exception that should be caught

3 participants