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

[Dashboard] AspNetCore 3.0+ authorization policy support via extensions #1663

Merged
merged 1 commit into from Jun 24, 2021

Conversation

dasiths
Copy link
Contributor

@dasiths dasiths commented May 6, 2020

The current DashboardOptions constructor sets the LocalRequestsOnlyAuthorizationFilter as a default filter. When you setup your Hangfire Dashboard using the new endpoints pattern and set an authorization policy it still goes ahead and uses the default anyway. To circumvent it we need to pass an empty list to Authorization as shown below.

            app.UseEndpoints(endpoints =>
            {
                endpoints.MapControllers()
                    .RequireAuthorization();
                endpoints.MapHangfireDashboard("/dashboard/hangfire", new DashboardOptions()
                    {
                        // We need to pass an empty list or Hangfire will use a local authorization filter
                        Authorization = new List<IDashboardAuthorizationFilter>()
                    })
                    .RequireAuthorization("hangfire-specific-auth-scheme");
            });

The new extension method introduced in this PR supports passing an authorization policy name and removes the LocalRequestsOnlyAuthorizationFilter from the default if no DashboardOptions instance has been passed.

@dasiths dasiths changed the title AspNetCore 3.0+ authorization policy support via extensions [Dashboard] AspNetCore 3.0+ authorization policy support via extensions May 6, 2020
@dasiths
Copy link
Contributor Author

dasiths commented Jun 1, 2020

How can I get this reviewed?

{
options = new DashboardOptions()
{
Authorization = new List<IDashboardAuthorizationFilter>() // We don't require the default LocalHost only filter since we provide our own policy
Copy link

@leonio leonio Nov 30, 2020

Choose a reason for hiding this comment

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

Perhaps this can just be Authorization = Enumerable.Empty<IDashboardAuthorizationFilter>()

Copy link
Contributor Author

@dasiths dasiths Jun 23, 2021

Choose a reason for hiding this comment

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

Thanks for the review @leonio. It doesn't make a whole lot of difference for readability or performance IMO. Can we let this go as is?

Copy link

@leonio leonio Jun 23, 2021

Choose a reason for hiding this comment

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

I don't have rights sorry, however this MR was very helpful for an issue I was resolving. FWIW hope it makes it through a full review.

@dasiths
Copy link
Contributor Author

dasiths commented Dec 1, 2020

@odinserj any feedback on this?

@odinserj
Copy link
Member

odinserj commented Jun 24, 2021

Thanks for your work and patience! Will merge this pull request and release changes with 1.7.24.

@odinserj odinserj merged commit 63e9372 into HangfireIO:master Jun 24, 2021
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

3 participants