we should differentiate `GoogleAuthErr` and `EnterpriseServerAuthErr`. When custom IdP is used and it is `EnterpriseServerAuthErr`, we should be showing `ConfiguredIdpOauth` popup instead of `GoogleOAuth` popup about flowcrypt-browser HOT 12 OPEN

Yeah, it could be.
Let me try that way.

from flowcrypt-browser.

@sosnovsky For this one, how can we differentiate between GoogleAuthErr and EnterpriseServerAuthErr?
Are you considering sending different HTTP status codes from FES side (e.g., 401 for GoogleAuthErr and another code for EnterpriseServerAuthErr)?
Or will we use the same 401 status code but with a distinct data object, perhaps with a type like UNAUTHORIZED_ENTERPRISE_SERVER or similar?

from flowcrypt-browser.

By the way, when a 401 error occurs, we currently trigger GoogleAuth.newAuthPopup (which also calls ConfiguredIdpOAuth.newAuthPopupForEnterpriseServerAuthenticationIfNeeded).
I think the current implementation doesn't disturb user too much, despite displaying two popups (Google and custom IDP) when a 401 error occurs?

What's your opinion?

from flowcrypt-browser.

@sosnovsky For this one, how can we differentiate between GoogleAuthErr and EnterpriseServerAuthErr? Are you considering sending different HTTP status codes from FES side (e.g., 401 for GoogleAuthErr and another code for EnterpriseServerAuthErr)? Or will we use the same 401 status code but with a distinct data object, perhaps with a type like UNAUTHORIZED_ENTERPRISE_SERVER or similar?

We should differentiate them by current stage of authorization:

• if extension receives authorization error while trying to login with Google account, it should be GoogleAuthErr and show GoogleOAuth popup
• if user successfully signed in with Google and receives error when login with custom IdP - error type will be EnterpriseServerAuthErr and should show ConfiguredIdpOauth popup

By the way, when a 401 error occurs, we currently trigger GoogleAuth.newAuthPopup (which also calls ConfiguredIdpOAuth.newAuthPopupForEnterpriseServerAuthenticationIfNeeded). I think the current implementation doesn't disturb user too much, despite displaying two popups (Google and custom IDP) when a 401 error occurs?

Probably it's possible to store current stage of user authorization, and if user successfully signed in with Google, then we should call only ConfiguredIdpOAuth.newAuthPopupForEnterpriseServerAuthenticationIfNeeded, without Google auth screen.

from flowcrypt-browser.

I think we might have a misunderstanding.
The custom IDP id_token is used in FES API calls.
From what I understand about this issue, when FES attempts to validate the id_token with the custom IDP and encounters an issue (such as unauthorized access or an invalid id_token), we should display the custom IDP authentication popup instead of the Google authentication popup.

Please let me know if I got this wrong.

from flowcrypt-browser.

Yeah, sorry, looks like I incorrectly thought it relates to initial user authorization, but mentioned differentiation should be used for FES and key manager calls.

I started reviewing #5802 and noticed that you use custom token only for FES calls, but it should also be used for key manager calls (like getPrivateKeys). And then we'll need method for retrieving token for custom IdP similar to googleApiAuthHeader, which will also refresh expired access tokens, so refresh token for custom IdP should also be stored locally.

from flowcrypt-browser.

Yeah, I Agree

from flowcrypt-browser.

Let me try to clairfy.
Do you mean we need to refresh access_token including id_token when we get unauthorized error from EKM/FES? (You know we use ID_TOKEN for FES and EKM calls. not access_token)

And then we'll need method for retrieving token for custom IdP similar to googleApiAuthHeader, which will also refresh expired access tokens

Also could you answer my first question?

Are you considering sending different HTTP status codes from FES side (e.g., 401 for GoogleAuthErr and another code for EnterpriseServerAuthErr)?
Or will we use the same 401 status code but with a distinct data object, perhaps with a type like UNAUTHORIZED_ENTERPRISE_SERVER or similar?

from flowcrypt-browser.

Do you mean we need to refresh access_token including id_token when we get unauthorized error from EKM/FES? (You know we use ID_TOKEN for FES and EKM calls. not access_token)

We currently have googleAuthRefreshToken method for refreshing user tokens, probably we can make it universal for both Google and custom IdP, so it'll generate refresh URL depending on IdP type. It saves locally access and id tokens, so we can store both of them for custom IdP too, while using only id token for Enterprise Server requests.

Are you considering sending different HTTP status codes from FES side (e.g., 401 for GoogleAuthErr and another code for EnterpriseServerAuthErr)?
Or will we use the same 401 status code but with a distinct data object, perhaps with a type like UNAUTHORIZED_ENTERPRISE_SERVER or similar?

I think it'll be the same 401 Unauthorized error on backend, and then client (browser extension) will decide which authorization popup it should show (Google or custom IdP). So when extension receives 401 Unauthorized from backend it should check if current user uses custom IdP - if yes then show custom IdP auth modal (as it's token is used for ES requests), if no custom IdP configured - show Google auth modal (like it works currently).

from flowcrypt-browser.

I see. Sounds good

from flowcrypt-browser.

@sosnovsky Is it really necessary to differentiate between the GoogleAuthPopup and the Custom IDP popup? When an EnterpriseServerAuthError occurs (such as when no id_token or refresh_token is saved or if the tokens are invalid), it seems acceptable to show both auth popups (Google OAuth and Custom IDP as it is now) to refresh both tokens.

I'm asking because differentiating between these two popups would require significant changes to the current notificationShowAuthPopupNeeded infrastructure.
Have to check if GoogleAuthError or EnterpriseServerAuthErr in every catch statements and add another method to show EnterpriseAuthPopup

from flowcrypt-browser.

I think it should be simpler than checking error type for each ApiErr.isAuthErr - custom IdP error can occur only when sending requests to Enterprise Server (like getting reply token for password-protected messages, fetching or updating private keys), probably all of them are in key-manager.ts and external-service.ts.
For example, requests for fetching inbox messages or searching recipients won't have custom IdP error, as they communicate directly with Google IdP.
What do you think, can we implement it this way?

from flowcrypt-browser.