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

not populating User/ClaimsPrincipal in native app oauth authorization header token request case #7

Closed
myusrn opened this issue Dec 8, 2018 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@myusrn
Copy link

myusrn commented Dec 8, 2018

Works great for populating User/ClaimsPrincipal in easyauth browser agent openid connect session cookie secured request cases but not for native [ desktop/mobile/postman] app oauth authorization header token secured request case. The latter is how all web api secured calls will typically be coming in.

I think I've figured out what is going on here and have a branch of updates that addresses the issue. One matter that jumped out is this code currently assumes an azurewebsites/.auth/me request will flow through it that can be used to create the GenericIdentity, GenericPrincipal and attach that to the this.Context.User object. My guess what was happening is that only happens once in the case of openid connect browser client interaction after which a session cookie is created that takes care of all subsequent requests being authenticated. For oauth native app interaction that isn't going to be the case as every request is secured using bearer token that EasyAuth flips into X-MS-* header values. So the fix I have working for both cases looks for X-MS-TOKEN-AAD-ID-TOKEN and if found creates this.Context.User object from there.

The upside here is there is no overhead of a GetAuthMe() network request happening ever except in the locahost case where you want it to happen to pull in a sample azurewebsites.net/.auth/me json output captured and saved off somewhere. For now i'm having to pull it from a blob storage url because of issue i'm having with iisexpress serving .json file as noted in issue #8 .

Also found issue with extracting claims from json payload where authentication method [amr] and roles claim need to be handled as array and broken out into individual claim entries to match what happens in the wild. That said getting unexpected User.IsInRole results even though I was able to confirm the following. Perhaps you folks have run into this previously with your work on this web app easyauth middleware and know what might be causing the unexpected role check results. Need it working especially for [Authorize(Roles = "RoleUserIsKnownToHaveClaimFor")] attribute setting to block entries into controllers or controller methods.

User.IsInRole("RoleUserIsKnownToHaveClaimFor") -> false
(User.FindFirst(ClaimTypes.Role))?.Value -> "RoleUserIsKnownToHaveClaimFor"
(User.Identity as ClaimsIdentity).RoleClaimType -> "http://schemas.microsoft.com/ws/2008/06/identity/claims/role"
(User.Identity as ClaimsIdentity).RoleClaimType == ClaimTypes.Role -> true

I'll create a fork with my proposed above changes addressing this issue if you all are interested in a review and update of them in order to eventually pull back into this repo and get nuget package updated published.

@myusrn myusrn changed the title not finding it populates User/ClaimsPrincipal in desktop/mobile[/postman] app oauth Authorization header token request case not finding it populates User/ClaimsPrincipal in native app oauth authorization header token request case Dec 8, 2018
@myusrn myusrn changed the title not finding it populates User/ClaimsPrincipal in native app oauth authorization header token request case not populating User/ClaimsPrincipal in native app oauth authorization header token request case Dec 8, 2018
@paule96
Copy link
Collaborator

paule96 commented Dec 9, 2018

@myusrn please fork the repo and add your changes there. So we can take a look to this :)

@paule96 paule96 added the bug Something isn't working label Dec 9, 2018
@kirkone kirkone added the help wanted Extra attention is needed label Dec 9, 2018
@myusrn
Copy link
Author

myusrn commented Dec 10, 2018

@paule96 and @kirkone, I just created fork here with my proposed fixes and have tested on my system and gives me results for native app oauth authorization header bearer token case now in addition to browser client openid connect [oidc] case which unlike native app after first request is handled using easyauth oidc issued session cookie. With update only localhost debugging wwwroot/.auth/me.json scenario involves network request versus other cases which now leverage easyauth injected X-MS-* request headers.

I have tested localhost f5 debug with just browser caller where wwwroot/.auth/me.json gets used and postman request where I include X-MS-CLIENT-PRINCIPAL-NAME, X-MS-CLIENT-PRINCIPAL-IDP, X-MS-CLIENT-PRINCIPAL and X-MS-TOKEN-AAD-ID-TOKEN headers from azurewebsites.net/api/values call where easyauth generates and includes those headers and more. I have tested published case using browser caller and postman request [ and native app using msft authentication library (msal) ] included Authorization header contained bearer token. My postman collection that you can use to repro localhost tests, as header data expiration is not an issue, can be found here and consumed using postman | import | import from link.

Details on using postman to acquire oauth token and use in authorization header bearer token secured web api request which is covered in this post.

Everything in my fork update is producing the result i'd expect to see locally and in cloud deployed case for browser openid connect as well as native app oauth authorization header secured web api call.

Also note that attempts to open the sample project KK.AspNetCore.EasyAuthAuthentication.Sample displayed in the solution view generate a dialog with the following details Microsoft Visual Studio | Project file is incomplete. Expected imports are missing. | OK . Not sure why that is happening on my win10 1809/17763.168 + vs17 15.9.3 setup.

@myusrn
Copy link
Author

myusrn commented Dec 10, 2018

update on User.IsInRole() behavior: I've figured out why User.IsInRole() wasn't working and with last pair of checkins now have the resulting User, User.Identity and UserIdentities results showing up with exact same types as the non-easyauth authentication middleware case.

kirkone added a commit that referenced this issue Dec 10, 2018
collaboration on updates to address issue #7
@kirkone kirkone removed the help wanted Extra attention is needed label Dec 10, 2018
@kirkone
Copy link
Owner

kirkone commented Dec 10, 2018

@myusrn Thanks for your help to get this sorted out. The new version is on Nuget.org

@myusrn
Copy link
Author

myusrn commented Dec 10, 2018

@kirkone and @paule96 nice work on the quick turn around with refactoring and updates to get this issue addressed. I just used my test solution environment to yank the project reference to your work and swap in the new nuget package drop and both localhost as well as both published test passes [ browser openid connect (oidc) session cookie and native app/postman oauth bearer token ] all working.

@myusrn myusrn closed this as completed Dec 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants