-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Use Permissions.ViewContent for GetEndpoint #15786
Conversation
@@ -26,7 +26,7 @@ public static IEndpointRouteBuilder AddGetContentEndpoint(this IEndpointRouteBui | |||
IAuthorizationService authorizationService, | |||
HttpContext httpContext) | |||
{ | |||
if (!await authorizationService.AuthorizeAsync(httpContext.User, Permissions.AccessContentApi)) | |||
if (!await authorizationService.AuthorizeAsync(httpContext.User, Permissions.ViewContent)) |
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.
The Permissions.AccessContentApi
permission is the valid check here and should not be changed. Checking for Permissions.ViewContent is done on line 41
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.
yes, on line 41 there is. so i think we should not need:
if (!await authorizationService.AuthorizeAsync(httpContext.User, Permissions.AccessContentApi))
{
return httpContext.ChallengeOrForbid("Api");
}
otherwise, for Anonymous , the will never hit line 41
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.
yes and they should not hit line 41 by default. If you want Anonymous role to do that, then grant Anonymous role permission to AccessContentApi
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.
but AccessContentApi were assgined to create and delete too.
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.
yes that is the idea. Granting them Anonymous to AccessContentApi is not a good idea. but if you want you can grant AccessContentApi to viewContent but not create, delete or publish this way they can only view content via API too.
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.
can we give out good default values for the site. especially if we need automatically set up the site.
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.
you can control the default values in your recipes. There isn't anything in OC that should be change here
fix #15785