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

casbin: add EnforceHandler to allow custom callback to handle enforcing. #66

Merged
merged 3 commits into from
Dec 16, 2021

Conversation

aldas
Copy link
Contributor

@aldas aldas commented Dec 7, 2021

casbin:

  • add EnforceHandler to allow custom callback to handle enforcing.
  • add ErrorHandler to allow custom handling of errors

CI:

  • run staticcheck only for latest Go version. Adding honnef.co/go/tools/cmd/staticcheck in CI flow causes dependency problems with older Go versions with we also support.

This should be enought to handle cases for: #58, #61 and #65

@codecov
Copy link

codecov bot commented Dec 7, 2021

Codecov Report

Merging #66 (d222dcd) into master (4d116ee) will increase coverage by 1.35%.
The diff coverage is 92.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #66      +/-   ##
==========================================
+ Coverage   55.41%   56.76%   +1.35%     
==========================================
  Files           8        8              
  Lines         462      643     +181     
==========================================
+ Hits          256      365     +109     
- Misses        186      256      +70     
- Partials       20       22       +2     
Impacted Files Coverage Δ
casbin/casbin.go 89.13% <92.85%> (+4.51%) ⬆️
zipkintracing/tracing.go 63.73% <0.00%> (-5.02%) ⬇️
session/session.go 88.46% <0.00%> (-1.02%) ⬇️
pprof/pprof.go 100.00% <0.00%> (ø)
jaegertracing/body_dump_response_writer.go 50.00% <0.00%> (ø)
jaegertracing/jaegertracing.go 48.79% <0.00%> (+2.02%) ⬆️
prometheus/prometheus.go 51.94% <0.00%> (+2.50%) ⬆️
zipkintracing/response_writer.go 21.27% <0.00%> (+5.27%) ⬆️

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 4d116ee...d222dcd. Read the comment docs.

@aldas aldas force-pushed the casbin_custom_enforcehandler branch from 819f0ac to d222dcd Compare December 7, 2021 18:57
@aldas
Copy link
Contributor Author

aldas commented Dec 7, 2021

@lammel could you review

return config.ErrorHandler(c, err, http.StatusInternalServerError)
}
if !pass {
return config.ErrorHandler(c, errors.New("enforce did not pass"), http.StatusForbidden)
Copy link

Choose a reason for hiding this comment

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

Can you make the status code configurable? For example, one might use 402 instead of 403.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this can be done inside/with errorhandler.

@aldas
Copy link
Contributor Author

aldas commented Dec 12, 2021

@lammel ping

Copy link
Contributor

@lammel lammel left a comment

Choose a reason for hiding this comment

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

Looks good to me.

The generic workflow-changes might be better handled in a separate commit. But thats just cosmetic.

@aldas aldas merged commit ed88076 into labstack:master Dec 16, 2021
@mcauto
Copy link

mcauto commented Jan 26, 2022

LGTM :)
Why not release it yet?

@aldas
Copy link
Contributor Author

aldas commented Jan 26, 2022

Should be ok now. Tagged a new release - 0.12.0.

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.

4 participants