search

AzureAD / azure-activedirectory-library-for-cordova

ADAL for Cordova
59 stars 111 forks source link

Expand Resiliency Error Codes #133

Open kgidewall opened 7 years ago

kgidewall commented 7 years ago

Issue Description

We currently invoke resiliency mode when the following conditions occur: network timeout or http error codes 500, 503, 504. After discussion, we have decided to expand the error code list to include ALL http error codes 500 - 599. Please change your logic to include all of those codes for Resiliency.

vladimir-kotikov commented 7 years ago

@kgidewall, could you please clarify about resiliency mode and the changes you suggest? Also i think that we're not handling http errors at all - this responsibility is delegated to native SDKs.

kgidewall commented 7 years ago

Kenton Gidewall has shared OneDrive for Business files with you. To view them, click the links below.

[icon]

ClientResiliency.NoMarkup.docxhttps://microsoft-my.sharepoint.com/personal/kentong_microsoft_com/Documents/0.DevEx-ADAL.C++/C++%20Resiliency/ClientResiliency.NoMarkup.docx?web=1

[icon]

Supporting extended lifetime tokens in ADAL.noMarkup.docxhttps://microsoft-my.sharepoint.com/personal/kentong_microsoft_com/Documents/0.DevEx-ADAL.C++/C++%20Resiliency/Supporting%20extended%20lifetime%20tokens%20in%20ADAL.noMarkup.docx?web=1

I’m not familiar with Cordova at all. When you said, “this responsibility is delegated to native SDKs.”, do you mean that Cordova sits on top of another ADAL SDK? Did you implement Resiliency with the rest of the SDKs? Here are the docs about Resiliency attached.

Thanks, Kenton

Kenton Gidewall Azure ADAL Team Office: 425-705-5312 Cell: 425-299-7761

From: Vladimir Kotikov [mailto:notifications@github.com] Sent: Tuesday, February 14, 2017 1:03 AM To: AzureAD/azure-activedirectory-library-for-cordova azure-activedirectory-library-for-cordova@noreply.github.com Cc: Kenton Gidewall kentong@microsoft.com; Mention mention@noreply.github.com Subject: Re: [AzureAD/azure-activedirectory-library-for-cordova] Expand Resiliency Error Codes (#133)

@kgidewallhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fkgidewall&data=02%7C01%7Ckentong%40microsoft.com%7Cf20bc0fd36294324c1a308d454b85099%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636226597985881956&sdata=BCM0csC%2FHR586nzGquImzDshKyzUr9mSGX7wlDujJJ0%3D&reserved=0, could you please clarify about resiliency mode and the changes you suggest? Also i think that we're not handling http errors at all - this responsibility is delegated to native SDKs.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FAzureAD%2Fazure-activedirectory-library-for-cordova%2Fissues%2F133%23issuecomment-279647508&data=02%7C01%7Ckentong%40microsoft.com%7Cf20bc0fd36294324c1a308d454b85099%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636226597985881956&sdata=CrtHewLmGBML7mZ%2FYI%2BEOO2KFh%2BrYJR1%2BZdctwbXb5w%3D&reserved=0, or mute the threadhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAHm1JUWGuPI6mxcH88Xekxj8uwoC2G5Lks5rcW3RgaJpZM4L_cJ7&data=02%7C01%7Ckentong%40microsoft.com%7Cf20bc0fd36294324c1a308d454b85099%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636226597985881956&sdata=E30VeBama3mZztNFybfY6ZyTXS%2FcPaJwtly9xYcEeKg%3D&reserved=0.

kgidewall commented 7 years ago

To view ClientResiliency.NoMarkup.docx, sign inhttps://microsoft-my.sharepoint.com/personal/kentong_microsoft_com/_layouts/15/acceptinvite.aspx?invitation=%7BEC5CC7DB%2D87CF%2D400C%2D8260%2D3DBB13C7A2AB%7D&listId=ce55ea2b%2D0f7a%2D4519%2D8668%2De9ee7f4981e5&itemId=daab0c2e%2Dc366%2D4232%2Da82f%2D22e659d0c8a5 or create an account.

kgidewall commented 7 years ago

To view Supporting extended lifetime tokens in ADAL.noMarkup.docx, sign inhttps://microsoft-my.sharepoint.com/personal/kentong_microsoft_com/_layouts/15/acceptinvite.aspx?invitation=%7BC1388693%2D1FAB%2D44AA%2D8264%2D7F4CE0C37745%7D&listId=ce55ea2b%2D0f7a%2D4519%2D8668%2De9ee7f4981e5&itemId=8af924f8%2D89a9%2D4d0b%2D975e%2Dfe29e20ad1dc or create an account.

vladimir-kotikov commented 7 years ago

@kgidewall, answering you questions:

When you said, “this responsibility is delegated to native SDKs.”, do you mean that Cordova sits on top of another ADAL SDK?

Exactly. ADAL for Cordova as a cross-platform solution uses a respective native SDK for each of 3 platforms it supports.

The plugin itself is a thin wrapper written in Javascript + native code to align APIs of all 3 native SDKs ad expose those to JavaScript.

Did you implement Resiliency with the rest of the SDKs?

I think we didn't. I think we'd nee to upgarde our dependencies first to support it. We currently use ADAL for iOS v 2.2.6, ADAL for Android 1.10.0 and ADAL for .Net v 2.28.1, hence we'd need to upgrade .Net and iOS SDKs (ADAL for Android 1.10.0 already supports it - https://github.com/AzureAD/azure-activedirectory-library-for-android/commit/212756a4e198693fba15064daa49f761c16715da):

I also quickly looked at the docs you shared and it sounds for me that to support resiliency we'd need the following 2 things (as mentioned in "ClientResiliency":

  1. Expose ExtendedLifeTimeEnabled option for AuthenticationContext
  2. Make sure that we propagate ExtendedExpiresOn and ExtendedLifeTimeToken properties of AuthenticationResult from native SDK to JavaScript (BTW are these properties also available for tokens in cache?)

Is my understanding correct?

kgidewall commented 7 years ago

Thanks for the info, Vladimir. Your understanding is correct. You would first need to make sure you are using platform-specific SDKs that support Resiliency. They will take care of the actual resiliency functionality. All you would need to do in your code is to support the flags going into and coming out of the SDK. My additional comments below:

  1. Expose ExtendedLifeTimeEnabled option for AuthenticationContext, and pass this flag in to the SDK.
  2. Make sure that we propagate ExtendedExpiresOn and ExtendedLifeTimeToken properties of AuthenticationResult from native SDK to JavaScript (BTW are these properties also available for tokens in cache?).
    • Per the documentation, when the token is returned, the user only sees the ExpiresOn value. There is a flag returned with the request which the developer can check to see if the token was returned because of Resiliency mode. If it was, then the developer knows that the ExpiresOn value returned is actually the ExtendedExpiresOn. You’ll need to support passing that flag to the user.
    • The ExtendedExpiresOn value is attached to the token and stored in the cache. It will depend on the SDK platform as to whether the tokens can be accessed and that value can be read directly. I am talking in generalities here, since even though all platforms followed the same spec, there are some differences in implementation because of their platform. For more details during implementation, you should speak directly to each of the platform SDK owners. Thanks, Kenton

Kenton Gidewall Azure ADAL Team Office: 425-705-5312 Cell: 425-299-7761

From: Vladimir Kotikov [mailto:notifications@github.com] Sent: Wednesday, February 15, 2017 3:04 AM To: AzureAD/azure-activedirectory-library-for-cordova azure-activedirectory-library-for-cordova@noreply.github.com Cc: Kenton Gidewall kentong@microsoft.com; Mention mention@noreply.github.com Subject: Re: [AzureAD/azure-activedirectory-library-for-cordova] Expand Resiliency Error Codes (#133)

@kgidewallhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fkgidewall&data=02%7C01%7Ckentong%40microsoft.com%7Cfae566748c2847a001fc08d455925f4c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636227534535439292&sdata=FKsODLM8FZPVN7Zkt1UYLyoZcy6SN0ddLhBi1xS%2FzYU%3D&reserved=0, answering you questions:

When you said, “this responsibility is delegated to native SDKs.”, do you mean that Cordova sits on top of another ADAL SDK?

Exactly. ADAL for Cordova as a cross-platform solution uses a respective native SDK for each of 3 platforms it supports.

The plugin itself is a thin wrapper written in Javascript + native code to align APIs of all 3 native SDKs ad expose those to JavaScript.

Did you implement Resiliency with the rest of the SDKs?

I think we didn't. I think we'd nee to upgarde our dependencies first to support it. We currently use ADAL for iOS v 2.2.6, ADAL for Android 1.10.0 and ADAL for .Net v 2.28.1, hence we'd need to upgrade .Net and iOS SDKs (ADAL for Android 1.10.0 already supports it - AzureAD/azure-activedirectory-library-for-android@212756ahttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FAzureAD%2Fazure-activedirectory-library-for-android%2Fcommit%2F212756a4e198693fba15064daa49f761c16715da&data=02%7C01%7Ckentong%40microsoft.com%7Cfae566748c2847a001fc08d455925f4c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636227534535449294&sdata=eRyy%2Bt3YM4ger0rg9oy6iw6hm2APnFE%2FMtOf3rqk35k%3D&reserved=0):

I also quickly looked at the docs you shared and it sounds for me that to support resiliency we'd need the following 2 things (as mentioned in "ClientResiliency":

  1. Expose ExtendedLifeTimeEnabled option for AuthenticationContext
  2. Make sure that we propagate ExtendedExpiresOn and ExtendedLifeTimeToken properties of AuthenticationResult from native SDK to JavaScript (BTW are these properties also available for tokens in cache?)

Is my understanding correct?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FAzureAD%2Fazure-activedirectory-library-for-cordova%2Fissues%2F133%23issuecomment-279981863&data=02%7C01%7Ckentong%40microsoft.com%7Cfae566748c2847a001fc08d455925f4c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636227534535449294&sdata=bUbL6B2kf96tueDvEa%2FUo7s2olfkLlMFPVmriGSJhyI%3D&reserved=0, or mute the threadhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAHm1JdJe0Sv1B09IWm71vJCGSFT9PQ6iks5rctuqgaJpZM4L_cJ7&data=02%7C01%7Ckentong%40microsoft.com%7Cfae566748c2847a001fc08d455925f4c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636227534535449294&sdata=%2B9QwZQnLFHGbrnOmW0%2BXq%2B5x%2FCbOrUegP7E8utrO3cI%3D&reserved=0.

vladimir-kotikov commented 7 years ago

@kgidewall, thanks for clarification.