lockelost
commented
7 years ago Hi,
Sorry I was busy with my Job. Just quick question.
I tried to apply your suggestions, but the main problem is that if I use ' AuthenticationContextWrapper' and call 'AcquireTokenSilentAsync()' to work as a Daemon, (with no user interaction) it throws an exception it failed, to use ''AcquireTokenAsync()' instead.
Is it because of the OneDrive SDK I installed from NugetPackage?
Best regards., Sunghyun wang
On Tue, Oct 25, 2016 at 8:19 PM, Chris Mayer notifications@github.com wrote:
@cdmayer requested changes on this pull request.
Please address all requested changes.
In src/OneDrive.Sdk.Authentication.Desktop/Business/ AdalDaemonAuthenticationProvider.cs https://github.com/OneDrive/onedrive-sdk-dotnet-msa-auth-adapter/pull/25#pullrequestreview-5736605 :
- public AuthenticationContext authContext;
- ClientCredential clientCredential; +
- // 'applicationId' : Your Application ID
- // 'applicationKey' : Your Application Key
- // 'tenant' : is usually a domain name for your Office365 service. Like 'yourcompany.onmicrosoft.com'
- public AdalDaemonAuthenticationProvider(
- string applicationId,
- string applicationKey,
- string tenant)
- {
- clientId = applicationId;
- clientKey = applicationKey; +
- string authority = String.Format(CultureInfo.InvariantCulture, "https://login.microsoftonline.com/{0}", tenant);
- authContext = new AuthenticationContext(authority);
authContext = new AuthenticationContext(authority);
Use AuthenticationContextWrapper: https://github.com/OneDrive/ onedrive-sdk-dotnet-msa-auth-adapter/blob/master/src/ OneDrive.Sdk.Authentication.Desktop/Business/AuthenticationContextWrapper.
cs
In src/OneDrive.Sdk.Authentication.Desktop/Business/ AdalDaemonAuthenticationProvider.cs https://github.com/OneDrive/onedrive-sdk-dotnet-msa-auth-adapter/pull/25#pullrequestreview-5736605 :
+
- do
- {
- retry = false;
- try
- {
- // ADAL includes an in memory cache, so this call will only send a message to the server if the cached token is expired.
- result = await authContext.AcquireTokenAsync(serviceResourceId, clientCredential);
- }
- catch (AdalException ex)
- {
- if (ex.ErrorCode == "temporarily_unavailable")
- {
- retry = true;
- retryCount++;
- Thread.Sleep(3000);
await Task.Delay(3000);
In src/OneDrive.Sdk.Authentication.Desktop/Business/ AdalDaemonAuthenticationProvider.cs https://github.com/OneDrive/onedrive-sdk-dotnet-msa-auth-adapter/pull/25#pullrequestreview-5736605 :
- public class AdalDaemonAuthenticationProvider : IAuthenticationProvider
- {
- public AccountSession CurrentAccountSession { get; internal set; }
- string clientId;
- string clientKey; +
- public AuthenticationContext authContext;
- ClientCredential clientCredential; +
- // 'applicationId' : Your Application ID
- // 'applicationKey' : Your Application Key
- // 'tenant' : is usually a domain name for your Office365 service. Like 'yourcompany.onmicrosoft.com'
- public AdalDaemonAuthenticationProvider(
- string applicationId,
- string applicationKey,
- string tenant)
string applicationId, string applicationKey,
Please use clientId and clientSecret like the rest of the library. Change
throughout this file.
In src/OneDrive.Sdk.Authentication.Desktop/Business/ AdalDaemonAuthenticationProvider.cs https://github.com/OneDrive/onedrive-sdk-dotnet-msa-auth-adapter/pull/25#pullrequestreview-5736605 :
+using System.Globalization; +using System.Threading; + +namespace Microsoft.OneDrive.Sdk.Authentication.Business +{
- public class AdalDaemonAuthenticationProvider : IAuthenticationProvider
- {
- public AccountSession CurrentAccountSession { get; internal set; }
- string clientId;
- string clientKey; +
- public AuthenticationContext authContext;
- ClientCredential clientCredential; +
- // 'applicationId' : Your Application ID
- // 'applicationKey' : Your Application Key
Please use typical class comments like this example in AuthenticationContextWrapper
/// <summary> /// Authenticates the user silently using <see cref="AuthenticationContext.AcquireTokenSilentAsync(string, string, UserIdentifier)"/>. /// </summary> /// <param name="resource">The resource to authenticate against.</param> /// <param name="clientId">The client ID of the application.</param> /// <param name="userIdentifier">The <see cref="UserIdentifier"/> of the user.</param>
In src/OneDrive.Sdk.Authentication.Desktop/Business/ AdalDaemonAuthenticationProvider.cs https://github.com/OneDrive/onedrive-sdk-dotnet-msa-auth-adapter/pull/25#pullrequestreview-5736605 :
+ +
- public async Task AuthenticateUserAsync(string serviceResourceId)
- {
- AuthenticationResult result = null;
- result = null;
- int retryCount = 0;
- bool retry = false; +
- do
- {
- retry = false;
- try
- {
- // ADAL includes an in memory cache, so this call will only send a message to the server if the cached token is expired.
- result = await authContext.AcquireTokenAsync(serviceResourceId, clientCredential);
Again, AuthenticationContextWrapper
In src/OneDrive.Sdk.Authentication.Desktop/Business/ AdalDaemonAuthenticationProvider.cs https://github.com/OneDrive/onedrive-sdk-dotnet-msa-auth-adapter/pull/25#pullrequestreview-5736605 :
- {
- if (ex.ErrorCode == "temporarily_unavailable")
- {
- retry = true;
- retryCount++;
- Thread.Sleep(3000);
- } +
- Console.WriteLine(
- String.Format("An error occurred while acquiring a token\nTime: {0}\nError: {1}\nRetry: {2}\n",
- DateTime.Now.ToString(),
- ex.ToString(),
- retry.ToString()));
- } +
- } while ((retry == true) && (retryCount < 3));
retryCount < 3
Make this a constant at the top of the file instead of an embedded literal.
In src/OneDrive.Sdk.Authentication.Desktop/Business/ AdalDaemonAuthenticationProvider.cs https://github.com/OneDrive/onedrive-sdk-dotnet-msa-auth-adapter/pull/25#pullrequestreview-5736605 :
+
- public async Task AuthenticateRequestAsync(HttpRequestMessage request)
- {
- if (this.CurrentAccountSession == null)
- {
- throw new ServiceException(
- new Error
- {
- Code = OAuthConstants.ErrorCodes.AuthenticationFailure,
- Message = "Please call one of the AuthenticateUserAsync...() methods to authenticate the user before trying to authenticate a request.",
- });
- } +
- if (this.CurrentAccountSession.IsExpiring)
- {
- throw new ServiceException(
Refresh the token, similar to this https://github.com/OneDrive/onedrive-sdk-dotnet-msa-auth-adapter/blob/master/src/OneDrive.Sdk.Authentication.Common/Business/AdalAuthenticationProviderBase.cs#L130
.
In src/OneDrive.Sdk.Authentication.Desktop/Business/ AdalDaemonAuthenticationProvider.cs https://github.com/OneDrive/onedrive-sdk-dotnet-msa-auth-adapter/pull/25#pullrequestreview-5736605 :
- Message = ""
- });
- } +
- var accessTokenType = string.IsNullOrEmpty(this.CurrentAccountSession.AccessTokenType)
- ? OAuthConstants.Headers.Bearer
- : this.CurrentAccountSession.AccessTokenType; +
- var uri = new UriBuilder(request.RequestUri);
- if (string.IsNullOrEmpty(uri.Query))
- uri.Query = string.Format("client_secret={0}", clientKey);
- else
- uri.Query = uri.Query.TrimStart('?') + string.Format("&client_secret={0}", clientKey);
- request.RequestUri = uri.Uri; + +
Extra whitespace
In src/OneDrive.Sdk.Authentication.Desktop/Business/ AdalDaemonAuthenticationProvider.cs https://github.com/OneDrive/onedrive-sdk-dotnet-msa-auth-adapter/pull/25#pullrequestreview-5736605 :
- // 'tenant' : is usually a domain name for your Office365 service. Like 'yourcompany.onmicrosoft.com'
- public AdalDaemonAuthenticationProvider(
- string applicationId,
- string applicationKey,
- string tenant)
- {
- clientId = applicationId;
- clientKey = applicationKey; +
- string authority = String.Format(CultureInfo.InvariantCulture, "https://login.microsoftonline.com/{0}", tenant);
- authContext = new AuthenticationContext(authority);
- clientCredential = new ClientCredential(clientId, clientKey);
- } + + +
Only 1 extra line
In src/OneDrive.Sdk.Authentication.Desktop/Business/ AdalDaemonAuthenticationProvider.cs https://github.com/OneDrive/onedrive-sdk-dotnet-msa-auth-adapter/pull/25#pullrequestreview-5736605 :
- {
- retry = true;
- retryCount++;
- Thread.Sleep(3000);
- } +
- Console.WriteLine(
- String.Format("An error occurred while acquiring a token\nTime: {0}\nError: {1}\nRetry: {2}\n",
- DateTime.Now.ToString(),
- ex.ToString(),
- retry.ToString()));
- } +
- } while ((retry == true) && (retryCount < 3)); + +
Extra whitespace
In src/OneDrive.Sdk.Authentication.Desktop/Business/ AdalDaemonAuthenticationProvider.cs https://github.com/OneDrive/onedrive-sdk-dotnet-msa-auth-adapter/pull/25#pullrequestreview-5736605 :
- {
- throw new ServiceException(
- new Error
- {
- Code = OAuthConstants.ErrorCodes.AuthenticationFailure,
- Message = ""
- });
- } +
- var accessTokenType = string.IsNullOrEmpty(this.CurrentAccountSession.AccessTokenType)
- ? OAuthConstants.Headers.Bearer
- : this.CurrentAccountSession.AccessTokenType; +
- var uri = new UriBuilder(request.RequestUri);
- if (string.IsNullOrEmpty(uri.Query))
- uri.Query = string.Format("client_secret={0}", clientKey);
Add braces around if and else statement blocks
In src/OneDrive.Sdk.Authentication.Desktop/Business/ AdalDaemonAuthenticationProvider.cs https://github.com/OneDrive/onedrive-sdk-dotnet-msa-auth-adapter/pull/25#pullrequestreview-5736605 :
- : this.CurrentAccountSession.AccessTokenType; +
- var uri = new UriBuilder(request.RequestUri);
- if (string.IsNullOrEmpty(uri.Query))
- uri.Query = string.Format("client_secret={0}", clientKey);
- else
- uri.Query = uri.Query.TrimStart('?') + string.Format("&client_secret={0}", clientKey);
- request.RequestUri = uri.Uri; + +
- request.Headers.Authorization = new AuthenticationHeaderValue(
- accessTokenType,
- this.CurrentAccountSession.AccessToken);
- } +
- protected AccountSession ConvertAuthenticationResultToAccountSession(AuthenticationResult authenticationResult)
Use the existing implementation in AdalAuthenticationProviderBase
In src/OneDrive.Sdk.Authentication.Desktop/Business/ AdalDaemonAuthenticationProvider.cs https://github.com/OneDrive/onedrive-sdk-dotnet-msa-auth-adapter/pull/25#pullrequestreview-5736605 :
@@ -0,0 +1,134 @@ +using System; +using System.Threading.Tasks; +using Microsoft.Graph; +using Microsoft.IdentityModel.Clients.ActiveDirectory; +using System.Net.Http; +using System.Net.Http.Headers; +using System.Globalization; +using System.Threading; + +namespace Microsoft.OneDrive.Sdk.Authentication.Business +{
- public class AdalDaemonAuthenticationProvider : IAuthenticationProvider
Is there a reason this doesn't extend AdalAuthenticationProviderBase?
In src/OneDrive.Sdk.Authentication.Desktop/Business/ AdalDaemonAuthenticationProvider.cs https://github.com/OneDrive/onedrive-sdk-dotnet-msa-auth-adapter/pull/25#pullrequestreview-5736605 :
- retry = false;
- try
- {
- // ADAL includes an in memory cache, so this call will only send a message to the server if the cached token is expired.
- result = await authContext.AcquireTokenAsync(serviceResourceId, clientCredential);
- }
- catch (AdalException ex)
- {
- if (ex.ErrorCode == "temporarily_unavailable")
- {
- retry = true;
- retryCount++;
- Thread.Sleep(3000);
- } +
- Console.WriteLine(
Shouldn't be writing to the console in the library. Remove this. If you
want to log this stuff, you can log it at the client layer.
In src/OneDrive.Sdk.Authentication.Desktop/Business/ AdalDaemonAuthenticationProvider.cs https://github.com/OneDrive/onedrive-sdk-dotnet-msa-auth-adapter/pull/25#pullrequestreview-5736605 :
@@ -0,0 +1,134 @@ +using System;
Add the license
// ------------------------------------------------------------------------------ // Copyright (c) Microsoft Corporation. All Rights Reserved. Licensed under the MIT License. See License in the project root for license information. // ------------------------------------------------------------------------------
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/OneDrive/onedrive-sdk-dotnet-msa-auth-adapter/pull/25#pullrequestreview-5736605, or mute the thread https://github.com/notifications/unsubscribe-auth/ABQ0OnqnlZSfkdM3MlPWYkggcDtM9d58ks5q3pybgaJpZM4Kcl2c .
Humble S/W developer, Always. Bruce Wang (왕성현)
cdmayer
new provoder for Daemon type apps.