azure-activedirectory-library-for-cordova icon indicating copy to clipboard operation
azure-activedirectory-library-for-cordova copied to clipboard

Expand Resiliency Error Codes

Open kgidewall opened this issue 8 years ago • 7 comments

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.

kgidewall avatar Feb 13 '17 16:02 kgidewall

@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.

vladimir-kotikov avatar Feb 14 '17 09:02 vladimir-kotikov

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:[email protected]] Sent: Tuesday, February 14, 2017 1:03 AM To: AzureAD/azure-activedirectory-library-for-cordova [email protected] Cc: Kenton Gidewall [email protected]; Mention [email protected] 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 avatar Feb 14 '17 16:02 kgidewall

@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):

  • iOS SDK supports resiliency since 2.3.0 - https://github.com/AzureAD/azure-activedirectory-library-for-objc/commit/fa3b70d544c657c8e5574499b62693a777e0e47e
  • .Net - since 3.12 - https://github.com/AzureAD/azure-activedirectory-library-for-dotnet/commit/09b5b20dff17749e7cff624ee7921775522b51da)

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?

vladimir-kotikov avatar Feb 15 '17 11:02 vladimir-kotikov

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:[email protected]] Sent: Wednesday, February 15, 2017 3:04 AM To: AzureAD/azure-activedirectory-library-for-cordova [email protected] Cc: Kenton Gidewall [email protected]; Mention [email protected] 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.

kgidewall avatar Feb 15 '17 17:02 kgidewall

@kgidewall, thanks for clarification.

vladimir-kotikov avatar Feb 17 '17 09:02 vladimir-kotikov