OneDrive / onedrive-sdk-dotnet-msa-auth-adapter

Other
25 stars 22 forks source link

Implement IAuthenticationProvider in a way that doesn't rely on WinForms or other Windows-specific UI components #21

Closed ericpapamarcos closed 7 years ago

ericpapamarcos commented 8 years ago

OnlineIdAuthenticationProvider is great for not prompting UI for credentials if an account is already connected to the device, but it doesn't work in a background task.

Sample code (UWP):

msaAuthenticationProvider = new OnlineIdAuthenticationProvider(scopes);
await msaAuthenticationProvider.AuthenticateUserAsync();

This returns the following error: Code: authenticationFailure Message: The request is not supported. (Exception from HRESULT: 0x80070032)

The culprit seems to be the CredentialPromptType within the GetAccountSessionAsync method. Even though the description for CredentialPromptType.PromptIfNeeded says "Show the UI only if an error occurred" and no error should be occurring, it returns the error message above.

Switching this type to CredentialPromptType.DoNotPrompt makes the scenario work in a background task, but of course there would need to be multiple code paths so that an app could prompt if this was the user's first time using the app, etc.

Is this the right approach to enable usage in a background task? Or is there an alternative recommended approach?

cdmayer commented 8 years ago

The auth adapter is definitely designed right now to work in the UI thread. That's the most common way that a developer will want to use the library, anyway. If running in the UI thread is a problem for your app, then yes you will probably need to write two code paths as you described.

I could see adding a flag for CredentialPromptType, though. You could submit a small PR for this if you end up implementing it. Here's what I recommend:

  1. Add a new enum to OnlineIdAuthenticationProvider. It should have analogous values to this enum.
  2. Add a private member to store the prompt type.
  3. Add an optional constructor argument that takes in a value from the enum you just made. Default should be PromptIfNeeded
  4. Use the stored value in GetAccountSessionAsync().

Then, simply add a try/catch in your code. If you get Graph.ServiceException.Error.Matches(OAuthConstants.ErrorCodes.AuthenticationFailure), you know authentication without prompting was unsuccessful and you need to use a prompt.

Let me know if you have other questions. If not, please close the issue when you're ready.

ericpapamarcos commented 8 years ago

Thanks for the guidance. I'll work on a small PR.

On Monday, October 17, 2016, Chris Mayer notifications@github.com wrote:

The auth adapter is definitely designed right now to work in the UI thread. That's the most common way that a developer will want to use the library, anyway. If running in the UI thread is a problem for your app, then yes you will probably need to write two code paths as you described.

I could see adding a flag for CredentialPromptType, though. You could submit a small PR for this if you end up implementing it. Here's what I recommend:

  1. Add a new enum to OnlineIdAuthenticationProvider. It should have analogous values to this enum https://msdn.microsoft.com/en-us/library/windows/apps/windows.security.authentication.onlineid.credentialprompttype.aspx .
  2. Add a private member to store the prompt type.
  3. Add an optional constructor argument that takes in a value from the enum you just made. Default should be PromptIfNeeded
  4. Use the stored value in GetAccountSessionAsync().

Then, simply add a try/catch in your code. If you get Graph.ServiceException.Error.Matches(OAuthConstants.ErrorCodes. AuthenticationFailure), you know authentication without prompting was unsuccessful and you need to use a prompt.

Let me know if you have other questions. If not, please close the issue when you're ready.

— 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/issues/21#issuecomment-254248630, or mute the thread https://github.com/notifications/unsubscribe-auth/AARQk6rcO2FubDxUAP6CMKyiOjq6rmnVks5q05nJgaJpZM4KX0Ki .

tipa commented 8 years ago

I am also in need of this functionality as I want to access OneDrive in a background task as well. Looking forward to the PR and the inclusion of it in the Nuget release :)

edit: There is also another use-case in which "DoNotPrompt" is useful: if I want to offer additional functionality in my app in case the user connected to OneDrive explicitly earlier but not want to disturb him with a popup if not, that enum is needed.

ericpapamarcos commented 8 years ago

Pull request submitted: #23

tipa commented 8 years ago

When can we expect a new Nuget version?

ericpapamarcos commented 7 years ago

@cdmayer, can we expect an updated NuGet package early this week?

cdmayer commented 7 years ago

One pending pull request needs to be resolved, then a new build will be uploaded.

ericpapamarcos commented 7 years ago

@cdmayer, has the pending pull request been resolved?

cdmayer commented 7 years ago

No, but you have waited long enough. Working on signing the new binaries now. Will post to Nuget ASAP.

cdmayer commented 7 years ago

https://www.nuget.org/packages/Microsoft.OneDriveSDK.Authentication/1.0.7 Very sorry you had to wait so long. Thanks for your patience!

Srusti-Thakkar commented 7 years ago

Can you please provide web authentication code and listing files and folders using Microsoft.OnedriveSdk nuget and OneDriveClient Class??

Thanks in advance

cdmayer commented 7 years ago

Everything you need to get started is here: https://github.com/OneDrive/onedrive-sdk-csharp#4-making-requests-to-the-service

Srusti-Thakkar commented 7 years ago

Yeah, but OneDriveClient Class use MsaAuthenticationProvider which is not available in Microsoft.OneDrive.sdk but it is available in Microsoft.OneDrive.Sdk.Authentication which is used for desktop app or Windows app, which are not used in web app.

cdmayer commented 7 years ago

"Desktop" can be used for a web app. That is one case where you can use the 'client secret' parameter because your web server code is protected from your users (whereas using a client secret in a desktop or Windows app is insecure). What is preventing you from using the MsaAuthenticationProvider for your web app?

Srusti-Thakkar commented 7 years ago

Can you please provide sample code?

chadwackerman commented 7 years ago

"What is preventing you from using the MsaAuthenticationProvider for your web app?"

@cdmayer The fact that the thing has dependencies on WinForms. You totally broke the web scenario with the authorization refactor and this is the third or fourth place I've seen you make dismissive comments to someone. It doesn't work.

Update this sample to the 2.0 SDK and you'll see. https://github.com/OneDrive/onedrive-webhooks-aspnet

There's no point in Microsoft pushing all this open source rah-rah crap and being Linux friendly in an attempt to woo startup people if you're going to respond to startup scenarios in this fashion. This is two lines of code with the DropBox API and with OneDrive it's just total insanity. Fix it and stop posting misinformation.

cdmayer commented 7 years ago

@chadwackerman that wasn't a dismissive comment or meant to be snarky. Sorry if it came off that way. It was a genuine question. I want to help figure out the issue, so I wanted to know what was blocking using the Desktop version of the package!

Your mention of WinForms is the first time in this thread that dependency issue was mentioned. Of course you're right that won't work on Linux, so thanks for bringing it up. This package is targeted at the Windows platform right now. The point of the refactor was to make it easier to write your own authentication provider for your own needs. If you want to write a web app that logs in many users, you can write an implementation of IAuthenticationProvider to get the job done.

cdmayer commented 7 years ago

I reopened the issue and re-titled it. We need a story for non-Windows platforms.

chadwackerman commented 7 years ago

It doesn't work on IIS/Windows either. Or any backend service. The reason there's a WinForms dependency is because it pops UI.

The readme here states "The MsaAuthenticationProvider constructor has an overload that takes in client secret for platforms that support web clients."

Nope. There's yet another wonky credential interface. But every auth call off that fails because everything is [STAThread]. That's a nonstarter.

Like I said, port the example to the 2.0 SDK and you'll see the problem in two seconds in Visual Studio on Windows.

chadwackerman commented 7 years ago

@cdmayer ... pretty please.

daboxu commented 7 years ago

Sorry guys for super late update. After reached for a while I found a work around which I believe is the best approach for ASP .NET web service using the OneDrive SDK and dealing with Oauth. For .NET MVC app, it should be the OWIN auth with OpenID and luckily MSA OAuth endpoint login.live.com supports that.

I set up a demo based on the .net connect example of Microsoft Graph asp .net example here with modified following files:

  1. in PM console, add SDK and Authentication package
    install-package Microsoft.OneDriveSDK
    install-package Microsoft.OneDriveSdk.Authentication
  2. Edit the Helper/SampleAuthProivider.cs to power OneDriveClient
    
    using System.Configuration;
    using System.Threading.Tasks;
    using Microsoft.Graph;
    using Microsoft.OneDrive.Sdk.Authentication;

namespace Microsoft_Graph_SDK_ASPNET_Connect.Helpers { using System.Net.Http; using System.Net.Http.Headers;

public sealed class SampleAuthProvider : IAuthenticationProvider
{

    // Properties used to get and manage an access token.
    private string redirectUri = ConfigurationManager.AppSettings["ida:RedirectUri"];
    private string appId = ConfigurationManager.AppSettings["ida:AppId"];
    private string appSecret = ConfigurationManager.AppSettings["ida:AppSecret"];
    private string scopes = ConfigurationManager.AppSettings["ida:GraphScopes"];

    private static readonly SampleAuthProvider instance = new SampleAuthProvider();
    private MsaAuthenticationProvider providerWithCache;

    private CredentialCache cache = new CredentialCache();

    private SampleAuthProvider()
    {
        providerWithCache = new MsaAuthenticationProvider(appId, appSecret, redirectUri, scopes.Split(new char[] { ' ' }), cache);
    } 

    public static SampleAuthProvider Instance
    {
        get
        {
            return instance;
        }
    }

    // Gets an access token. First tries to get the token from the token cache.
    private async Task<string> GetAccessToken() 
    {
        await this.providerWithCache.AuthenticateUserAsync();

        return this.providerWithCache.CurrentAccountSession.AccessToken;
    }

    public void SetAccessToken(AccountSession accountSession)
    {
        this.providerWithCache.CurrentAccountSession = accountSession;
    }

    public async Task AuthenticateRequestAsync(HttpRequestMessage request)
    {
        request.Headers.Authorization = new AuthenticationHeaderValue("bearer", await this.GetAccessToken());
    }
}

}


3. Edit Controller/HomeController.cs to add action for OneDriveClient:

```csharp
[Authorize]
public async Task<ActionResult> GetDriveId()
{
    OneDriveClient client = new OneDriveClient(SampleAuthProvider.Instance);
    ViewBag.DriveId = await graphService.GetMyDriveID(client);

    return View("Graph");
}
  1. Edit /App_Start/Startup.Auth.cs for using UseOpenIdConnectAuthentication
public void ConfigureAuth(IAppBuilder app) {
    ...
    // remove the original auth to Graph OAuth

    app.UseOpenIdConnectAuthentication(
        new OpenIdConnectAuthenticationOptions
        {
            ClientId = appId,
            Authority = "https://login.live.com",
            PostLogoutRedirectUri = redirectUri,
            RedirectUri = redirectUri,
            ResponseType = "code id_token",
            Scope = "openid " + graphScopes,
            TokenValidationParameters = new TokenValidationParameters
            {
                ValidateIssuer = false,
            },
            Notifications = new OpenIdConnectAuthenticationNotifications
            {
                AuthorizationCodeReceived = async (context) =>
                {
                    var code = context.Code;

                    OAuthHelper oauthHelper = new OAuthHelper();
                    var accountSession = await oauthHelper.RedeemAuthorizationCodeAsync(
                        code,
                        appId,
                        appSecret,
                        redirectUri,
                        graphScopes.Split(new char[] { ' ' }));

                    SampleAuthProvider.Instance.SetAccessToken(accountSession);
                },
                AuthenticationFailed = (context) =>
                {
                    context.HandleResponse();
                    context.Response.Redirect("/Error?message=" + context.Exception.Message);
                    return Task.FromResult(0);
                }
            }
        });
       ...
  }
  1. Edit /Models/GraphService.cs for add OneDrive Service

    public async Task<string> GetMyDriveID(IOneDriveClient client)
    {
    var drive = await client.Drive.Request().GetAsync();
    
    return drive.Id;
    }
  2. Edit Views/Home/Graph.cshtml

    
    <div class="col-sm-12">
    <p class="@(ViewBag.Message == null ? "hidden" : null)">@Html.Raw(ViewBag.Message)</p>
    </div>
    >>>add>>>>
    @using (Html.BeginForm("GetDriveId", "Home"))
    {
    <input name="get-id" type="submit"/>
    }

@(ViewBag.DriveId == null ? "empty" : @Html.Raw(ViewBag.DriveId))

<<<add<<<<


7. Edit /Helpers/SDKHelper.cs for adopt the SampleAuthProvider
```csharp
public static GraphServiceClient GetAuthenticatedClient()
{
    GraphServiceClient graphClient = new GraphServiceClient(
        SampleAuthProvider.Instance);
    return graphClient;
}
  1. Edit Web.config for app setting:
    <add key="ida:AppId" value="<appid in GUID>" />
    <add key="ida:AppSecret" value="<app secrete>" />
    <add key="ida:RedirectUri" value="<redirect uri>" />
    <add key="ida:GraphScopes" value="OneDrive.ReadWrite" />
daboxu commented 7 years ago

And FYI, if you want to do the Authentication instead of OWIN openconnectid on your own app like what Dropbox did, we do provide public interfaces in OAuthHelper in which you can generate the url for get a code, and redeem the code with OAuthHelper:

OAuthHelper helper = new OAuthHelper();
//get the code
helper.GetAuthorizationCodeRequestUrl();
//redeem token
helper.RedeemAuthorizationCodeAsync()
//refresh token
helper.RedeemRefreshTokenAsync()

You can build customized implements the IAuthenticationProivder interface from package Microsoft.Graph.Core and pass it to OneDrive client, like what I have done in the example SampleAuthProvider above.

NOTE: Right now the SampleAuthProvider in the example is a little bit hacky because it fed its internal MSAAuthProivder an AccountSession only for first time user login so it doesn't work with the MSAAuthProivder's cache. I want to get your feedbacks on the SampleAuthProvider class so I can polish it and integrate it to this repo as a part of the MSA Auth Adapter for web apps and also . Thanks in advance :)

daboxu commented 7 years ago

close it for now and feel free to reopen it if you run into any issue.