-
Notifications
You must be signed in to change notification settings - Fork 525
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
Auth for ASP.NET Core #1163
Auth for ASP.NET Core #1163
Conversation
Tests still to come. |
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.
Drive-by review of just the project file :)
<Project Sdk="Microsoft.NET.Sdk"> | ||
|
||
<PropertyGroup> | ||
<TargetFramework>netcoreapp2.0</TargetFramework> |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
</PropertyGroup> | ||
|
||
<ItemGroup> | ||
<PackageReference Include="Microsoft.AspNetCore.Authentication.OpenIdConnect" Version="2.0.1" /> |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
var optionsMonitor = _services.GetRequiredService<IOptionsMonitor<OpenIdConnectOptions>>(); | ||
var clock = _services.GetRequiredService<ISystemClock>(); | ||
var authProperties = auth.Properties; | ||
var options = optionsMonitor.Get(OpenIdConnectDefaults.AuthenticationScheme); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
// TODO: Logging | ||
return null; | ||
} | ||
var optionsMonitor = _services.GetRequiredService<IOptionsMonitor<OpenIdConnectOptions>>(); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
@gijswijs I wrote a sample AspNetCore 2.1-RC1 app that uses a combination of Microsoft's AspNetCore Authentication middleware and Google's .NET Client. No changes or additions are needed to the .NET Client. In a nutshell, I use Microsoft's middleware to get a token, GoogleCredential.FromAccessToken to get the credential, and generated client APIs to use Google services. All of the interesting code is in Startup.cs and HomeController.cs. It might work with AspNetCore 2.0. |
Hi, just looking for an update on this? I've already got google auth in my asp.net core 2.1 web app ( If indeed work is on-going on this PR, is the best advice in the meantime to do something similar to the sample provided by @andyfmiller ? Thanks |
@mattwoberts If you have the auth up & running, than you have the hard part behind you. The rest is a matter of adding the correct API package using nuget (https://www.nuget.org/profiles/google-apis-packages) and then start using that. Presumably you'll get credentials back from the Auth, and with those credentials you can instantiate a Google API service. It could look like this:
|
Thanks @gijswijs . but right now, I'm totally confused.. I'm using public async Task<ActionResult> GoogleAuthComplete()
{
var loginInfo = await _signInManager.GetExternalLoginInfoAsync();
if (loginInfo == null)
return RedirectToAction("Login");
// At this point we know the email address.
var email = loginInfo.Principal.Claims.Where(c => c.Type == ClaimTypes.Email).Select(c => c.Value).SingleOrDefault();
// find the user that matches this email, then ..
await _signInManager.SignInAsync(user, isPersistent:true);
} But in the spirit of incremental auth, if the user then decides they want to integrate with their calendar, then I send another challenge to google with the new scope (very similar to what @andyfmiller has). Now that works, but what it gives me back isn't a nice I know that I can then make use of the Google API with that access token: HttpClientInitializer = GoogleCredential.FromAccessToken(accessToken), But, that token is going to expire in an hour, so I'd need to make use of the refresh token to get another access token... Feels like I'm going about this the wrong way.... |
@mattwoberts Ah, yes. I've been at the exact same point of total confusion. Remember, if you are not confused by oAuth at some point, you're doing it wrong.
Obviously, behind the scenes AddGoogle does that oAuth token dance as well, but sadly it's not possible to use those methods for anything else than interacting with the Google+ API. (Which is what AddGoogle interacts with if I remember correctly) This was discussed in PR #1109 after @buzallen created that PR to offer an alternative solution for the authentication flow in dotnet core. Based on that PR @chrisdunelm professed his preference for using the OpenIdConnect flow built into ASP.NET Core. This PR is his first stab at that approach. I'm not entirely sure why @chrisdunelm wants to use OpenIdConnect instead of directly use AspNetCore.Authentication.OAuth, which seems like a possibility as well. Anyhow, I ended up using the approach @buzallen took in PR #1109 , that, although less elegant, is the approach that I had success with. So my advice would be to try to fit his code into your application. Good luck! |
@mattwoberts - Have you tried re-issuing the Challenge when the access token has expired? This snippet checks to see if the access token has expired and re-issues the Challenge if it has. The
|
@andyfmiller interesting approach. How would you go about instantiating a AnalyticsReportingService for instance? |
Cheers @andyfmiller and @gijswijs for the help here ;) @andyfmiller - the issue that I have is that I need to do the oauth dance, get a refresh token, and then use that going forwards to do things from the back end - that aren't user-driven. So I can't issue another challenge to the user. I have however got something working - I've not yet looked at the code in PR #1109 but here's what I have: Firstly, I've got a second .AddGoogle("GoogleCalendar","GoogleCalendar", googleOptions =>
{
googleOptions.CallbackPath = new PathString("/signin-google-calendar");
googleOptions.ClientId = "***";
googleOptions.ClientSecret = "***";
googleOptions.AccessType = "offline";
googleOptions.SaveTokens = true;
googleOptions.Scope.Add(Google.Apis.Calendar.v3.CalendarService.Scope.Calendar);
}); Then, in my controller, I have an action method that is responsible for adding my google calendar integration (so once added, my backend services will use it long-term). So I check for tokens (hat-tip again to @andyfmiller): var redirectUrl = Url.Action("GoogleCalendar", "Integrations");
var loginHint = _currentUser.User.Email;
var accessToken = await HttpContext.GetTokenAsync("GoogleCalendar", "access_token");
var refreshToken = await HttpContext.GetTokenAsync("GoogleCalendar", "refresh_token");
var expires = await HttpContext.GetTokenAsync("GoogleCalendar", "expires_at");
DateTime.TryParse(expires, out var expiresAt);
if (accessToken == null || DateTime.Now > expiresAt)
{
return new ChallengeResult("GoogleCalendar", new AuthenticationProperties()
{
Parameters =
{
new KeyValuePair<string, object>("login_hint", loginHint ),
new KeyValuePair<string, object>("include_granted_scopes", true)
},
RedirectUri = redirectUrl
});
} Once I've done all that, then I've got a refresh token from google :) So then, I pass that over to the google APIs and let it do its thing: var flow = new GoogleAuthorizationCodeFlow(new GoogleAuthorizationCodeFlow.Initializer
{
ClientSecrets = new ClientSecrets
{
ClientId = "***",
ClientSecret = "***"
},
Scopes = new[] { CalendarService.Scope.CalendarReadonly },
DataStore = _dataStore // I have my own db-based IDataStore.
});
var tokenResponse = new TokenResponse
{AccessToken = accessToken, RefreshToken = refreshToken, TokenType = "offline"};
var c = new UserCredential(flow,"user",tokenResponse); I now have a Feel free to tell me I've done it all wrong :) |
Hi all. Just revisiting my code around this, and while it's working for me (Using the built in asp.net core |
I would like to stress my interest for this as well. I'd love to yank out my crappy code for something proper. |
I was not able to complete google authentication for Google drive API in ASP.net core 2.0 so I followed the following a link for console project and then applied the same process for ASP.net core 2.0. using Microsoft.AspNetCore.Http; namespace ThriivDental.Controllers
}Please provide me solution. |
efadbe0
to
d9b8bd9
Compare
51d32d5
to
8f90ac1
Compare
This is looking good, must be nice being able to call on @jskeet for code reviews ;) |
Something that might be relevant, since you're plugging into OIDC - there is a bug/issue in safari where the asp.net core SameSite cookie settings mean that the post back to your app will fail. It's pretty simple to reproduce - from looking at your integration test @chrisdunelm I think you should find it doesn't work in the latest safari (I'm on Version 12.0.1 (14606.2.104.1.1)) There are a number of workarounds posted, but so far the only success I've had personally is to set the cookie as follows: .AddCookie(opts => { opts.Cookie.SameSite = SameSiteMode.None; }) |
@mattwoberts thanks for pointing out the potential Safari problem. I can't check this immediately as Safari is no longer available for Windows, and I don't have an easy way to test this on a Mac right now. As this is a known OIDC issue, I'll probably go ahead with an initial beta, then see if we can work-around it for a beta02 release. |
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've looked through the "address PR comments" commit and it looks good. Happy to look at anything else you'd like me to take a closer look at, but if not, feel free to merge.
@jskeet Please could you check the csproj, especially the package title and description. |
FWIW, that name is completely clear.
…On Tue, Nov 20, 2018 at 12:19 PM Chris Bacon ***@***.***> wrote:
@jskeet <https://github.com/jskeet> Please could you check the csproj,
especially the package title and description.
And do you think Google.Apis.Auth.AspNetCore is a reasonable package name?
I think I'll hold off on merging this until next week, so we can touch
base beforehand, and so it won't be inadvertently auto-released ;)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1163 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACixguB_-OXWPDEwdhFibnj0d5rZt2BYks5uxGPkgaJpZM4SK4gb>
.
|
@andyfmiller Thanks. @mattwoberts, @andyfmiller As far as you can see, does this fulfill the requirements you have for an Google ASP.NET auth package? I.e. Auth with Google; support for incremental auth; access via DI to Google credentials and scopes; and the One extra feature that I would hope to implement soon is a programmatic scope check and (incremental) re-challenge if scope(s) isn't yet authorized. Thanks. |
That is a great list! I'll look at it more deeply this weekend. |
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.
A few thoughts about the project file. (I didn't look at the integration tests project file; can do that later if it's deemed useful.)
<Import Project="..\CommonProjectProperties.xml" /> | ||
|
||
<PropertyGroup> | ||
<!-- Only ASP.NET Core 2.0 on netstandard2.0 supported --> |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
<!-- Only ASP.NET Core 2.0 on netstandard2.0 supported --> | ||
<TargetFramework>netstandard2.0</TargetFramework> | ||
<!-- Beta release --> | ||
<Version>$(Version)-beta01</Version> |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
<Version>$(Version)-beta01</Version> | ||
</PropertyGroup> | ||
|
||
<!-- nupkg information --> |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
<PackageReference Include="Microsoft.AspNetCore.Authorization" Version="2.0.4" /> | ||
<PackageReference Include="Microsoft.AspNetCore.Mvc.Abstractions" Version="2.0.4" /> | ||
<PackageReference Include="Microsoft.AspNetCore.Mvc.Core" Version="2.0.4" /> | ||
<ProjectReference Include="..\Google.Apis.Auth\Google.Apis.Auth.csproj" /> |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Supports incremental auth, and an injectable IGoogleAuthProvider to supply Google credentials. | ||
|
||
Supported Platforms: | ||
- ASP.NET Core 2.x on netstandard2.x |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This makes the app code much simpler. I was able to remove quite a bit of code from my MVC sample which allows the user to sign in (no special scopes), then list their Google Classroom courses (which requires "https://www.googleapis.com/auth/classroom.courses.readonly"). For example, the following 4 lines of code replaced 2 AppFlowMetaData files and about 30 lines of code in the controller:
However I lost the ability to pass in a login hint with the request for additional scope. Passing in a login hint greatly simplifies the UX for additional scope. Is there a way to supply a login hint to the incremental authorization request? The full sample is here: https://github.com/andyfmiller/google-incremental-auth-sample/tree/master/src/GoogleIncrementalMvcSample After some experimentation, I found that I could add a LoginHint by handling OnRedirectToIdentityProvder in ConfigureServices:
And set LoginHint to the current user's email:
I can see the prompt in the authorization request to google, but google is still prompting me to select my account prior to showing me the consent screen for the additional scope. Is there any way to disable the step of selecting my account again? |
@andyfmiller The login_hint is now set for incremental auth. However, I agree that account selection is still occurring. This may be intentional, and is pre-selecting the correct account, but that still needs confirming by the user? |
I am seeing a problem with incremental auth.
I'm investigating to work out why this is happening. |
Thank you for including login_hint with incremental auth. I think it is a good idea even if it is not eliminating the account re-selection. It has occurred to me that account re-selection is occurring because my client has not been verified by Google (it is just for development/test like this) so accounts.google.com is being extra cautious. If I was using a verified client, the account re-selection might go away. |
I'm using a verified account for testing, and I'm still seeing the account selection. |
Possibly relevant SO question about the incremental auth: https://stackoverflow.com/questions/51495056/google-oauth-2-0-incremental-authorization-with-offline-access |
Bummer. I've also noticed that when I sign out ( |
The problem I was having with incremental auth was due to using the incorrect type of oauth credential. Using the correct "web" type means this now works as expected. |
Congratulations! |
@andyfmiller :) |
This is great @chrisdunelm! Sorry I’ve not had chance to try it out yet but I’m going to plumb it in on Monday and test it out👍 |
This : https://github.com/andyfmiller/google-incremental-auth-sample example work using this package? |
I followed the example in However, when I try to introduce Identity to the application via
Here is my
Does this possibly have something to do with the options in I'm going off of how I have my current Am I going about this the wrong way? Here are the logs for
|
No description provided.