-
-
Notifications
You must be signed in to change notification settings - Fork 525
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 authorization check bug #5504
Conversation
This commit adds some extra logic, and error messages around user authentication with a user supplied `authorize_callback` method. - Adds more messaging around the `RuntimeError` when an error occurs during authorization checks. - Adds messaging to a user attempting access a path they do not have access to. - Adds logic to ensure if an `authorize_callback` has only 1 parameter, it does not cause a `RuntimeError`. Resolves holoviz#5503
Codecov Report
@@ Coverage Diff @@
## main #5504 +/- ##
==========================================
- Coverage 83.55% 82.69% -0.86%
==========================================
Files 276 276
Lines 40147 40152 +5
==========================================
- Hits 33543 33205 -338
- Misses 6604 6947 +343
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 23 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW I reviewed the change and it looked good to me.
I had an optional suggestion for the error message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks!
This commit adds some extra logic, and error messages around user authentication with a user supplied
authorize_callback
method.RuntimeError
when an error occurs during authorization checks.authorize_callback
has only 1 parameter, it does not cause aRuntimeError
.Resolves #5503