Additions for OpenIdConnectMiddleware and OAuthBearer Beta1. #112
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| <?xml version="1.0" encoding="utf-8"?> | ||
| <Project ToolsVersion="14.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> | ||
| <PropertyGroup> | ||
| <VisualStudioVersion Condition="'$(VisualStudioVersion)' == ''">14.0</VisualStudioVersion> | ||
| <VSToolsPath Condition="'$(VSToolsPath)' == ''">$(MSBuildExtensionsPath32)\Microsoft\VisualStudio\v$(VisualStudioVersion)</VSToolsPath> | ||
| </PropertyGroup> | ||
| <Import Project="$(VSToolsPath)\AspNet\Microsoft.Web.AspNet.Props" Condition="'$(VSToolsPath)' != ''" /> | ||
| <PropertyGroup Label="Globals"> | ||
| <ProjectGuid>bef0f5c3-ef4e-4649-9c49-d5e279a3ca2b</ProjectGuid> | ||
| <RootNamespace>OpenIDConnectSample</RootNamespace> | ||
| <BaseIntermediateOutputPath Condition="'$(BaseIntermediateOutputPath)'=='' ">..\..\artifacts\obj\$(MSBuildProjectName)</BaseIntermediateOutputPath> | ||
| <OutputPath Condition="'$(OutputPath)'=='' ">..\..\artifacts\bin\$(MSBuildProjectName)\</OutputPath> | ||
| </PropertyGroup> | ||
| <PropertyGroup Label="Configuration" Condition="'$(Configuration)|$(Platform)'=='Debug|AnyCPU'"> | ||
| <AssemblyName>OpenIDConnectSample</AssemblyName> | ||
| </PropertyGroup> | ||
| <PropertyGroup> | ||
| <SchemaVersion>2.0</SchemaVersion> | ||
| <DevelopmentServerPort>42023</DevelopmentServerPort> | ||
| <CommandLineArguments /> | ||
| <DebugTarget> | ||
| </DebugTarget> | ||
| </PropertyGroup> | ||
| <Import Project="$(VSToolsPath)\AspNet\Microsoft.Web.AspNet.targets" Condition="'$(VSToolsPath)' != ''" /> | ||
| <ProjectExtensions> | ||
| <VisualStudio> | ||
| <UserProperties project_1json__JSONSchema="http://www.asp.net/media/4878834/project.json" /> | ||
| </VisualStudio> | ||
| </ProjectExtensions> | ||
| </Project> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| using Microsoft.AspNet.Builder; | ||
| using Microsoft.AspNet.Http; | ||
| using Microsoft.Framework.DependencyInjection; | ||
| using Microsoft.AspNet.Security.OpenIdConnect; | ||
| using Microsoft.AspNet.Http.Security; | ||
| using Microsoft.AspNet.Security; | ||
|
|
||
| namespace OpenIdConnectSample | ||
| { | ||
| public class Startup | ||
| { | ||
| public void Configure(IApplicationBuilder app) | ||
| { | ||
| app.UseServices(services => | ||
| { | ||
| services.AddDataProtection(); | ||
| services.Configure<ExternalAuthenticationOptions>(options => | ||
| { | ||
| options.SignInAsAuthenticationType = OpenIdConnectAuthenticationDefaults.AuthenticationType; | ||
|
There was a problem hiding this comment.
There was a problem hiding this comment. Interesting pinpoint about a simplier, the idea is that it just works if you pass in the authority pinpoint, we reach out for validation coordinates, keys, issues, etc and produce a ClaimsPrincipal. I am pushing for a consistent model for validation. I made a small change where the notification can set the AuthenticationTicket and return. Giving you complete freedom to take it all over. There was a problem hiding this comment. @brentschmaltz @Tratcher all these changes are great and we definitely need them (the OIDC configuration discovery is just amazing). But we also need a basic token validation middleware that behaves exactly like the existing OAuth2 bearer middleware and allows full pluggability without relying on complicated and OIDC-specific things. Here's my suggestion:
There was a problem hiding this comment. To add some clarify, the code written in the tests under: OAuthBearerMiddlewareTests don't use metadata. Inside any three of the notifications (message received, token received, token validated) you can take over. |
||
| }); | ||
|
|
||
| }); | ||
|
|
||
| app.UseCookieAuthentication(options => | ||
| { | ||
| options.AuthenticationType = OpenIdConnectAuthenticationDefaults.AuthenticationType; | ||
|
There was a problem hiding this comment. These auth types are all setup wrong. There was a problem hiding this comment. Why is that? What are you suggesting they be? There was a problem hiding this comment.
Or you can simply omit setting the property. There was a problem hiding this comment. Not blocking, let's think about that later. There was a problem hiding this comment. And the sample is working as expected? If it is, that's really sad: IMO, having multiple authentication handlers configured with the same IIRC, that was not the case with Katana 3, but an If @Tratcher agrees with the general concept, I suggest opening a new ticket to track that. There was a problem hiding this comment. We added some validation in the new stack for things like challenging for a missing auth type, but there's not currently any checks for duplicates. Duplicates are not supported and will cause strange side-effects with APIs like SignIn and Challenge. The correct config is: |
||
| }); | ||
|
|
||
| app.UseOpenIdConnectAuthentication(options => | ||
| { | ||
| options.ClientId = "fe78e0b4-6fe7-47e6-812c-fb75cee266a4"; | ||
| options.Authority = "https://login.windows.net/cyrano.onmicrosoft.com"; | ||
| options.RedirectUri = "http://localhost:42023"; | ||
| options.SignInAsAuthenticationType = OpenIdConnectAuthenticationDefaults.AuthenticationType; | ||
|
There was a problem hiding this comment.
Or just remove that, given that you already set the cookies middleware as the default authentication middleware via There was a problem hiding this comment. I am not entirely sure what to do here. One goal I have is to remove the need to explicitly set this Auth type by adding a property to the AuthenticationTicket. It always seemed add to me that the runtime requires the ClaimsIdentity to have a specific authtype for things to work. I am thinking of ways to re-think this. There was a problem hiding this comment. Please avoid redesigning auth types in this PR, it's way out of scope. If we're going to tackle that issue we need to do it as its own work item across the whole stack. There was a problem hiding this comment. I backed out some of the changes we will need moving forward. So we can get these in, we will deal with the other issues later. There was a problem hiding this comment. I think it's time to have a check in all the providers that use a @Tratcher your thoughts? There was a problem hiding this comment. I am actually trying to get rid of SignInAsAuthenticationType. Wouldn't the existence of an authentication ticket be sufficient? There was a problem hiding this comment. Huh? Why would you remove it? There was a problem hiding this comment. Conceptually this drives the ClaimsIdentity AuthenticationType, that shouldn't be linked. There was a problem hiding this comment. Not really: An example should be clearer: when the Google middleware creates its own identity using the profile endpoint, it calls the authentication handler corresponding to |
||
| options.AuthenticationType = OpenIdConnectAuthenticationDefaults.AuthenticationType; | ||
|
|
||
| }); | ||
|
|
||
| app.Run(async context => | ||
| { | ||
| if (context.User == null || !context.User.Identity.IsAuthenticated) | ||
| { | ||
| context.Response.Challenge(new AuthenticationProperties { RedirectUri = "/" }, OpenIdConnectAuthenticationDefaults.AuthenticationType); | ||
|
|
||
| context.Response.ContentType = "text/plain"; | ||
| await context.Response.WriteAsync("Hello First timer"); | ||
| return; | ||
| } | ||
|
|
||
| context.Response.ContentType = "text/plain"; | ||
| await context.Response.WriteAsync("Hello Authenticated User"); | ||
| }); | ||
|
|
||
|
|
||
|
|
||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| { | ||
| "dependencies": { | ||
| "Kestrel": "1.0.0-*", | ||
| "Microsoft.AspNet.Security.Cookies": "1.0.0-*", | ||
| "Microsoft.AspNet.Server.IIS": "1.0.0-*", | ||
| "Microsoft.AspNet.Security.OpenIdConnect": "1.0.0-*", | ||
| "Microsoft.AspNet.Server.WebListener": "1.0.0-*" | ||
| }, | ||
| "frameworks": { | ||
| "aspnet50": { }, | ||
| "aspnetcore50": { } | ||
| }, | ||
| "commands": { | ||
| "web": "Microsoft.AspNet.Hosting server=Microsoft.AspNet.Server.WebListener server.urls=http://localhost:12345", | ||
|
There was a problem hiding this comment. nit: All of our other samples use 5001 as port for weblistener. Yeah but does not matter though. |
||
| "kestrel": "Microsoft.AspNet.Hosting --server Kestrel --server.urls http://localhost:5004" | ||
| }, | ||
| "webroot": "wwwroot" | ||
| } | ||
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.
wondering if having
/api/v2is required or not necessary. Or may be for consistency with other feeds.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.
I thought about that, but am hoping I can get a core50, 50 and 4.5x version together.