-
Notifications
You must be signed in to change notification settings - Fork 39
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
Implementing dashboard authorization filter. #63
Conversation
Hey @mo-esmp, Love the idea of going with the Hangfire authorization style! 🤗 I'm not sure when but I'll try to read and review the PR as soon as possible 😉 (fyi, I'm going onwards with the integration test pr - I just need to complete elastic ones and a small mongo-test refactor to merge that part. I'll write you about it when I complete it 😄) |
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.
LGTM!
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.
Hey @mo-esmp
I reviewed the PR - IMHO, it's a great feature, all green from me! In the feature we could think if it can be useful to add any auth related to policies/etc, right now I'd merge it ASAP 😄
A quick suggestion: I made a small cleanup branch on the tests naming etc here https://github.com/followynne/serilog-ui/tree/chore/ui-web-cleanup, give it a look if you think it's good to include on your branch 😃
A quick notice, on test "Local_Request_are_allowed_by_default": I saw it fails if the front end build wasn't run before launching it- in the future I'll try to see if there's a way to prevent this 😃
(@mo-esmp I already approved the PR as per prev comment) |
@followynne Please send your PR to this one, |
chore(auth-tests): create application factory util, rename files
Unifying authentication and authorization mechanism like the Hangfire dashboard!