#214 authorization improvements #2659

mperk · 2020-01-16T20:35:21Z

Resolve first item in #214
MethodInvocationAuthorizationService should check Roles

maliming · 2020-01-17T01:46:46Z

...rk/src/Volo.Abp.Authorization/Volo/Abp/Authorization/MethodInvocationAuthorizationService.cs

                {
-                    throw new AbpAuthorizationException("Authorization failed! User has not logged in.");
+                    throw new AbpAuthorizationException("Authorization failed! Given roles has not granted: " + authorizationAttribute.Roles);
                }
            }


hi @mperk

Policy and Roles should be combined, which means we have to check them together, We can build a combination of Policy and Roles .

await _authorizationService.CheckAsync(authorizationPolicy);

https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.authorization.authorizationpolicybuilder?view=aspnetcore-3.1

Because AuthorizationPolicyBuilder can also add AuthenticationSchemes, we should also test it. As mentioned in #214

- [ ] MethodInvocationAuthorizationService should also evaluate the auth schema (not sure) .

ok thank you for quick reply. I will research and develop

maliming · 2020-01-20T00:48:33Z

framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/Authorization_Tests.cs

+        public async Task Should_Not_Allow_To_Call_Method_If_Has_No_Role_ProtectedByRole_Async()
+        {
+            await Assert.ThrowsAsync<AbpAuthorizationException>(async () =>
+            {
+                await _myAuthorizedService1.ProtectedByRole().ConfigureAwait(false);
+            }).ConfigureAwait(false);
+        }
+
+        [Fact]
+        public async Task Should_Allow_To_Call_Method_If_Has_No_Role_ProtectedByRole_Async()
+        {
+            int result = await _myAuthorizedService1.ProtectedByRole().ConfigureAwait(false);
+            result.ShouldBe(42);
+        }


await _myAuthorizedService1.ProtectedByRole().ConfigureAwait(false);

I guess one of the unit test methods will fail.

Yes, it is fail. I am still developing for that. Had very little.

Hi @maliming ,
I finished finally. You can check.

maliming · 2020-01-21T02:20:05Z

hi @mperk
I made some changes. see 583cdba

Regarding AuthenticationSchemes, if we use Authorize attribute in the web layer controller, the authentication function of Asp Net Core will automatically handle Policy, Roles, AuthenticationSchemes.

MethodInvocationAuthorizationService is more for application services.

Will AuthenticationScheme be used in application services? I'm not sure.

If we want to handle AuthenticationSchemes, we need to use HttpContext.

mperk · 2020-01-21T04:15:52Z

hi @mperk
I made some changes. see 583cdba

Regarding AuthenticationSchemes, if we use Authorize attribute in the web layer controller, the authentication function of Asp Net Core will automatically handle Policy, Roles, AuthenticationSchemes.

MethodInvocationAuthorizationService is more for application services.

Will AuthenticationScheme be used in application services? I'm not sure.

If we want to handle AuthenticationSchemes, we need to use HttpContext.

Very good. Thanks.

hikalkan · 2020-03-03T13:30:20Z

As I see, some unit tests are failing.

Message: 
    System.ArgumentNullException : Value cannot be null. (Parameter 'policy')
  Stack Trace: 
    AuthorizationServiceExtensions.AuthorizeAsync(IAuthorizationService service, ClaimsPrincipal user, Object resource, AuthorizationPolicy policy)
    AbpAuthorizationServiceExtensions.AuthorizeAsync(IAuthorizationService authorizationService, Object resource, AuthorizationPolicy policy) line 30
    AbpAuthorizationServiceExtensions.AuthorizeAsync(IAuthorizationService authorizationService, AuthorizationPolicy policy) line 39
    AbpAuthorizationServiceExtensions.IsGrantedAsync(IAuthorizationService authorizationService, AuthorizationPolicy policy) line 81
    AbpAuthorizationServiceExtensions.CheckAsync(IAuthorizationService authorizationService, AuthorizationPolicy policy) line 120
    MethodInvocationAuthorizationService.CheckAsync(MethodInvocationAuthorizationContext context) line 32
    AuthorizationInterceptor.AuthorizeAsync(IAbpMethodInvocation invocation) line 24
    AuthorizationInterceptor.InterceptAsync(IAbpMethodInvocation invocation) line 18
    CastleAsyncAbpInterceptorAdapter`1.InterceptAsync(IInvocation invocation, IInvocationProceedInfo proceedInfo, Func`3 proceed) line 20
    ControllerActionInvoker.<InvokeInnerFilterAsync>g__Awaited|13_0(ControllerActionInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
    ResourceInvoker.<InvokeNextExceptionFilterAsync>g__Awaited|25_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
    ResourceInvoker.Rethrow(ExceptionContextSealed context)
    ResourceInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
    ResourceInvoker.InvokeNextResourceFilter()
    --- End of stack trace from previous location where exception was thrown ---
    ResourceInvoker.Rethrow(ResourceExecutedContextSealed context)
    ResourceInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
    ResourceInvoker.InvokeFilterPipelineAsync()
    --- End of stack trace from previous location where exception was thrown ---
    ResourceInvoker.<InvokeAsync>g__Logged|17_1(ResourceInvoker invoker)
    EndpointMiddleware.<Invoke>g__AwaitRequestTask|6_0(Endpoint endpoint, Task requestTask, ILogger logger)
    AbpUnitOfWorkMiddleware.InvokeAsync(HttpContext context, RequestDelegate next) line 23
    <<UseMiddlewareInterface>b__1>d.MoveNext()
    --- End of stack trace from previous location where exception was thrown ---
    AbpExceptionHandlingMiddleware.InvokeAsync(HttpContext context, RequestDelegate next) line 32
    AbpExceptionHandlingMiddleware.InvokeAsync(HttpContext context, RequestDelegate next) line 52
    <<UseMiddlewareInterface>b__1>d.MoveNext()
    --- End of stack trace from previous location where exception was thrown ---
    AbpAuditingMiddleware.InvokeAsync(HttpContext context, RequestDelegate next) line 37
    AbpAuditingMiddleware.InvokeAsync(HttpContext context, RequestDelegate next) line 54
    <<UseMiddlewareInterface>b__1>d.MoveNext()
    --- End of stack trace from previous location where exception was thrown ---
    AuthorizationMiddleware.Invoke(HttpContext context)
    FakeAuthenticationMiddleware.InvokeAsync(HttpContext context, RequestDelegate next) line 29
    <<UseMiddlewareInterface>b__1>d.MoveNext()
    --- End of stack trace from previous location where exception was thrown ---
    RequestLocalizationMiddleware.Invoke(HttpContext context)
    AbpRequestLocalizationMiddleware.InvokeAsync(HttpContext context, RequestDelegate next) line 34
    <<UseMiddlewareInterface>b__1>d.MoveNext()
    --- End of stack trace from previous location where exception was thrown ---
    AbpCorrelationIdMiddleware.InvokeAsync(HttpContext context, RequestDelegate next) line 27
    <<UseMiddlewareInterface>b__1>d.MoveNext()
    --- End of stack trace from previous location where exception was thrown ---
    <<SendAsync>g__RunRequestAsync|0>d.MoveNext()
    --- End of stack trace from previous location where exception was thrown ---
    ClientHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
    HttpClient.FinishSendAsyncBuffered(Task`1 sendTask, HttpRequestMessage request, CancellationTokenSource cts, Boolean disposeCts)
    AbpAspNetCoreTestBase`1.GetResponseAsync(String url, HttpStatusCode expectedStatusCode) line 46
    AbpAspNetCoreTestBase`1.GetResponseAsStringAsync(String url, HttpStatusCode expectedStatusCode) line 35
    AuthTestController_Tests.Should_Authorize_For_Defined_And_Allowed_Permission() line 84
    --- End of stack trace from previous location where exception was thrown ---

maliming · 2020-03-04T01:20:14Z

@hikalkan I fixed the unit tests.

hikalkan · 2020-04-11T20:00:08Z

Thanks to both of you :)

mperk added 2 commits January 16, 2020 22:35

MethodInvocationAuthorizationService should check Roles

c0d6582

authorization tests with role

5016ba6

maliming reviewed Jan 17, 2020

View reviewed changes

hikalkan added this to the 2.1 milestone Jan 17, 2020

hikalkan self-requested a review January 17, 2020 16:17

authorize check with combine

85f956f

maliming reviewed Jan 20, 2020

View reviewed changes

mperk and others added 3 commits January 20, 2020 15:07

change authorization service to abpauthorization service

9617efc

authorization tests with role

cfba922

Check all authorization attributes at the same time.

583cdba

maliming and others added 2 commits February 19, 2020 16:37

Merge branch 'dev' into abpframework#214-authorization-improvements

0d57774

Fix build error.

9f2b49c

hikalkan modified the milestones: 2.1, 2.2 Feb 20, 2020

hikalkan modified the milestones: 2.2, 2.3 Mar 3, 2020

Check if authorizationPolicy is null.

88b8f6e

Merge branch 'dev' into abpframework#214-authorization-improvements

2a42ba8

hikalkan added abp-framework enhancement labels Mar 5, 2020

hikalkan modified the milestones: 2.3, 2.4, 2.5 Mar 18, 2020

hikalkan modified the milestones: 2.4, 2.5 Mar 18, 2020

ebicoglu modified the milestones: 2.5, 2.6 Apr 10, 2020

hikalkan merged commit 63cb2f0 into abpframework:dev Apr 11, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#214 authorization improvements #2659

#214 authorization improvements #2659

mperk commented Jan 16, 2020

maliming Jan 17, 2020

maliming Jan 17, 2020

mperk Jan 17, 2020 •

edited

maliming Jan 20, 2020

mperk Jan 20, 2020

mperk Jan 20, 2020

maliming commented Jan 21, 2020

mperk commented Jan 21, 2020

hikalkan commented Mar 3, 2020

maliming commented Mar 4, 2020

hikalkan commented Apr 11, 2020

#214 authorization improvements #2659

#214 authorization improvements #2659

Conversation

mperk commented Jan 16, 2020

maliming Jan 17, 2020

Choose a reason for hiding this comment

maliming Jan 17, 2020

Choose a reason for hiding this comment

mperk Jan 17, 2020 • edited

Choose a reason for hiding this comment

maliming Jan 20, 2020

Choose a reason for hiding this comment

mperk Jan 20, 2020

Choose a reason for hiding this comment

mperk Jan 20, 2020

Choose a reason for hiding this comment

maliming commented Jan 21, 2020

mperk commented Jan 21, 2020

hikalkan commented Mar 3, 2020

maliming commented Mar 4, 2020

hikalkan commented Apr 11, 2020

mperk Jan 17, 2020 •

edited