-
-
Notifications
You must be signed in to change notification settings - Fork 109
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
Dev authorization fakes #151
Conversation
…ion/authorization. Added unit tests for them.
…on test service, in the right state, to the TestServiceProvider. Added unit tests.
…ion fakes to validate the various states.
…Service to give user friendly reminder to call AddAuthorization to TestContext.Services.
Getting latest changes for docs from egil/bUnit dev branch.
…ion/authorization. Added unit tests for them.
…on test service, in the right state, to the TestServiceProvider. Added unit tests.
…ion fakes to validate the various states.
…Service to give user friendly reminder to call AddAuthorization to TestContext.Services.
…ro/bUnit into dev-authorization-fakes
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.
Very solid start. Here are a few general comments:
- I haven't read through the docs and changelog text yet, since that might change once the API settles. Ill do proper review of those later.
- Well structured tests, good work on those.
- I sometimes to suggest a code change in one place, but that change should also be made in multiple other places, where the same thing happens.
- I focused mostly on reviewing the tests, as they are using the API you are building. I will return to the implementation in bunit.web when the API is settled.
src/bunit.web.tests/TestDoubles/Authorization/FakeAuthenticationStateProviderTest.cs
Outdated
Show resolved
Hide resolved
src/bunit.web.tests/TestDoubles/Authorization/FakeAuthorizationExtensionsTest.cs
Outdated
Show resolved
Hide resolved
src/bunit.web.tests/TestDoubles/Authorization/FakeAuthorizationExtensionsTest.cs
Outdated
Show resolved
Hide resolved
src/bunit.web/TestDoubles/Authorization/FakeAuthenticationStateProvider.cs
Outdated
Show resolved
Hide resolved
src/bunit.web/TestDoubles/Authorization/FakeAuthenticationStateProvider.cs
Outdated
Show resolved
Hide resolved
src/bunit.web/TestDoubles/Authorization/MissingFakeAuthorizationException.cs
Outdated
Show resolved
Hide resolved
FYI: I have updated the dev branch with test-doubles as the name for the documentation section. |
Co-authored-by: Egil Hansen <[email protected]>
…eProvider.cs Change to method comment. Co-authored-by: Egil Hansen <[email protected]>
changing mocking to faking Co-authored-by: Egil Hansen <[email protected]>
…nPolicyProviderTest.cs Removing ConfigureAwait call. Co-authored-by: Egil Hansen <[email protected]>
removing NavigationManager Co-authored-by: Egil Hansen <[email protected]>
removed navigation manager from component. Co-authored-by: Egil Hansen <[email protected]>
removed navigation manager from component. Co-authored-by: Egil Hansen <[email protected]>
I pushed some changes to the PR including refactored docs to fit the general style. While documenting policies and roles i stumples onto something I think is a bug. I added a bunch of extra tests to the project, but in essens, the problem is content limited by roles and policies are always shown, even when inside a |
I don't see any new pushed commits from you to the PR. Did you forget to push them? ;) |
Weird, I did not. But maybe I don't have the right |
Ahh, I managed to push a new branch to this repo with the changes: https://www.github.com/egil/bUnit/tree/dev-authorization-fakes/ Not sure how I add these to your PR without pushing directly to your local repo. Suggestions? |
That's weird. You pushed a commit into this PR earlier: 3e4b04d. Did you do that in my forked repo? |
Hmm nop. But I did pull the PR down using the |
OK, I managed to push it to this PR instead. It seems that when you use |
Ok, I have your changes now. I will look at the failing tests. |
…s in AuthorizeAsync method.
Fixed the AuthorizationService.AuthorizeAsync to validate authorization state based on requirements and either policies, roles, or authorization state. This covers all of the states we have, I believe. I'll look at a SetClaims method as well though I'm not sure we'll be able to fake of the behavior that ClaimsPrincipal.Claims does. |
…View to behave as expected. Updated sample code 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.
Just a quick review here on gh, see review comments on code below.
Regarding SetClaims, let's leave it for now and see if somebody asks for the feature.
What about a way to test asynchronous authentication, e.g. the state (https://docs.microsoft.com/en-us/aspnet/core/blazor/security/?view=aspnetcore-3.1#content-displayed-during-asynchronous-authentication), is it a hard thing to get working?
src/bunit.web/TestDoubles/Authorization/FakeAuthorizationService.cs
Outdated
Show resolved
Hide resolved
src/bunit.web/TestDoubles/Authorization/FakeAuthorizationService.cs
Outdated
Show resolved
Hide resolved
src/bunit.web/TestDoubles/Authorization/TestAuthorizationContext.cs
Outdated
Show resolved
Hide resolved
…es, new SimpleAuthViewWithClaims component, and unit tests.
I got both SetClaims and the AuthorizeView.Authorizing state working. Authorizing was a little tricky because how to get into that state is not documented. But I figured it out looking through the AuthorizeViewCore code. |
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.
Awesome getting both claims and authorizing working. Ill update the docs tomorrow (I am on CET timezone) and play with the bits.
Found a few small things.
src/bunit.web.tests/TestDoubles/Authorization/AuthorizationTest.cs
Outdated
Show resolved
Hide resolved
src/bunit.web/TestDoubles/Authorization/FakeAuthenticationStateProvider.cs
Outdated
Show resolved
Hide resolved
src/bunit.web/TestDoubles/Authorization/FakeAuthorizationService.cs
Outdated
Show resolved
Hide resolved
sample white space formtting Co-authored-by: Egil Hansen <[email protected]>
…s set before call to RenderComponent. Additional other tweaks.
@DarthPedro I changed the code a bit to make it possible to call Please give my changes a review. If it looks alright, I'd say we are done! |
Ok, that looks good. Thanks for all of the help. |
No thank you. You almost all the work here! |
Pull request description
For components that use the authentication and authorization, there is a lot of boilerplate code required to write to create a test. This code can be complex and needs to be copied into user's test projects each time.
Provide a set of "fake" authorization services that allow the user to choose and change the authentication/authorization state and user identification for a test. (closes #146)
PR meta checklist
DEV
branch.Content checklist
Todo checklist