Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Flyte console refreshes often to re-authenticate #4817

Open
2 tasks done
ALMerrill opened this issue Feb 1, 2024 · 6 comments
Open
2 tasks done

[BUG] Flyte console refreshes often to re-authenticate #4817

ALMerrill opened this issue Feb 1, 2024 · 6 comments
Labels
backlogged For internal use. Reserved for contributor team workflow. bug Something isn't working ui Admin console user interface

Comments

@ALMerrill
Copy link
Contributor

Describe the bug

When using the flyte console throughout the day, the console will often refresh and force me to log back in, several times per day. It seems like if I lose focus on the console page for more than 10-20 minutes, when I regain focus on that page, it refreshes. This seems excessive, and it sounds like there should be a way to have it refresh the auth token in the background.

Talking to @davidmirror-ops, I think this is the relevant section of my values file.

    auth:
      appAuth:
        thirdPartyConfig:
          flyteClient:
            clientId: flytectl
            redirectUri: http://localhost:53593/callback
            scopes:
              - offline
              - all
        selfAuthServer:
          staticClients:
            flyte-cli:
              id: flyte-cli
              redirect_uris:
              - http://localhost:53593/callback
              - http://localhost:12345/callback
              grant_types:
                - refresh_token
                - authorization_code
              response_types:
                - code
                - token
              scopes:
                - all
                - offline
                - access_token
              public: true
            flytectl:
              id: flytectl
              redirect_uris:
                - http://localhost:53593/callback
                - http://localhost:12345/callback
              grant_types:
                - refresh_token
                - authorization_code
              response_types:
                - code
                - token
              scopes:
                - all
                - offline
                - access_token
              public: true
            flytepropeller:
              id: flytepropeller
              client_secret: "${secret}"
              redirect_uris:
                - http://localhost:3846/callback
              grant_types:
                - refresh_token
                - client_credentials
              response_types:
                - token
              scopes:
                - all
                - offline
                - access_token
              public: false

Expected behavior

Ideally the token would refresh in the background logins would not be necessary after the initial login

Additional context to reproduce

This is hosted in GCP, using Google Identity for authentication

Screenshots

No response

Are you sure this issue hasn't been raised already?

  • Yes

Have you read the Code of Conduct?

  • Yes
@ALMerrill ALMerrill added bug Something isn't working untriaged This issues has not yet been looked at by the Maintainers labels Feb 1, 2024
Copy link

welcome bot commented Feb 1, 2024

Thank you for opening your first issue here! 🛠

Copy link

dosubot bot commented Feb 1, 2024

🤖

It's great that you've identified a potential solution to the issue by suggesting that the token should refresh in the background. Your proposed solution could be a valuable contribution to the project. We encourage you to go ahead and create a pull request with your proposed changes. Thank you for your contribution!


This response is meant to be useful and save you time. It is not meant to be a precise solution, but rather a starting point for your own research.

Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

If you want to continue the conversation, start your reply with @dosu-bot.

@eapolinario eapolinario added backlogged For internal use. Reserved for contributor team workflow. and removed untriaged This issues has not yet been looked at by the Maintainers labels Feb 1, 2024
@davidmirror-ops
Copy link
Contributor

@ALMerrill what version of flyte are you running?

@ALMerrill
Copy link
Contributor Author

@ALMerrill what version of flyte are you running?

1.10.2

@fg91
Copy link
Member

fg91 commented Jun 12, 2024

Our users reported "constantly having to log in" in the flyteconsole (UI) as well. (Using Google IdP and Flyte 1.12)

Talking to @davidmirror-ops, I think this is the relevant section of my values file.

According to our findings, this is not related to the config in the helm values file above as this only configures Flyte's own auth server which is used for the client credentials flow by the clients in flytekit/flyteidl.

Increasing the lifetime of the access token in this section has no effect on how often users have to login in the UI:

appAuth:
    selfAuthServer:
        accessTokenLifespan: 30m0s  <- no effect on UI

Instead, we find that when logging in, flyteadmin responds with a set-cookie header with a value flyte_idt (defined here). The token in the cookie is hashed but by setting a breakpoint in flyteadmin here, we find that the original token expires after 1h. Leaving a tab open after having signed in requires a re-login after exactly 1h which seems to confirm this.

There appears to be no token refresh mechanism in flyteconsole. Other web apps usually refresh the token in the background. Once the token inside the flyte_idt cookie expires, requests to the backend result in a 401 and the frontend shows a Back to Sign in button. When this is clicked another auth flow is triggered. As the user is stilled logged in to Google, the redirect is almost instant, without seeing a login screen.

Given this, it appears the problem could be avoided by automatically/periodically refreshing the token in the background or by not requiring to press a button and just automatically redirecting to the sign in page - which the user would then never notice. If we are not mistaken, this pop up requiring a button press was only introduced with a recent upgrade of the UI.

@fg91 fg91 added the ui Admin console user interface label Jun 12, 2024
@fellhorn
Copy link
Contributor

fellhorn commented Oct 1, 2024

Potentially a useful workaround for other people having the same issue: For our specific case we could not trigger the auth flow automatically in the background to renew the token as it requires a cross site request to Google's auth servers.

We instead just added a simple hack for our specific case, which is probably not good to be upstreamed but might be useful for others. Instead of showing the "Authentication Required" popup we just forward the user directly to the login route to renew the token: https://github.com/fellhorn/flyteconsole/tree/dennis/fix/back-to-signin

diff --git a/packages/flyte-api/src/ApiProvider/index.tsx b/packages/flyte-api/src/ApiProvider/index.tsx
index d656e30..e955fe7 100644
--- a/packages/flyte-api/src/ApiProvider/index.tsx
+++ b/packages/flyte-api/src/ApiProvider/index.tsx
@@ -1,6 +1,6 @@
 import React, { createContext, useContext } from 'react';
 import AdminEndpoint from '../utils/AdminEndpoint';
-import { defaultLoginStatus, getLoginUrl, getLogoutUrl, LoginStatus } from './login';
+import { defaultLoginStatus, getLoginUrl, getLogoutUrl, relogin, LoginStatus } from './login';
 import getEndpointUrl from '../utils/getEndpointUrl';
 import RawEndpoint from '../utils/RawEndpoint';
 import getAdminApiUrl from '../utils/getAdminApiUrl';
@@ -11,6 +11,7 @@ export interface FlyteApiContextState {
   getLogoutUrl: (redirect?: string) => string;
   getProfileUrl: () => string;
   getAdminApiUrl: (endpoint: AdminEndpoint | string) => string;
+  relogin: () => void;
 }
 
 const FlyteApiContext = createContext<FlyteApiContextState>({
@@ -20,6 +21,7 @@ const FlyteApiContext = createContext<FlyteApiContextState>({
   getLogoutUrl: () => '#',
   getProfileUrl: () => '#',
   getAdminApiUrl: () => '#',
+  relogin: () => '#',
 });
 
 interface FlyteApiProviderProps {
@@ -53,6 +55,7 @@ export const FlyteApiProvider = (props: FlyteApiProviderProps) => {
         getLogoutUrl: (redirect) => getLogoutUrl(flyteApiDomain, redirect),
         getProfileUrl: () => getEndpointUrl(RawEndpoint.Profile, flyteApiDomain),
         getAdminApiUrl: (endpoint) => getAdminApiUrl(endpoint, flyteApiDomain),
+        relogin: () => relogin(flyteApiDomain),
       }}
     >
       {children}
diff --git a/packages/flyte-api/src/ApiProvider/login.ts b/packages/flyte-api/src/ApiProvider/login.ts
index 9dca526..e262cbe 100644
--- a/packages/flyte-api/src/ApiProvider/login.ts
+++ b/packages/flyte-api/src/ApiProvider/login.ts
@@ -34,3 +34,7 @@ export function getLogoutUrl(
   const baseUrl = getEndpointUrl(RawEndpoint.Logout, adminUrl);
   return `${baseUrl}?redirect_url=${redirectUrl}`;
 }
+
+export function relogin(adminUrl?: string) {
+  window.location.href = getLoginUrl(adminUrl);
+}
diff --git a/packages/oss-console/src/components/data/QueryAuthorizationObserver.tsx b/packages/oss-console/src/components/data/QueryAuthorizationObserver.tsx
index a42c9ce..3eaf349 100644
--- a/packages/oss-console/src/components/data/QueryAuthorizationObserver.tsx
+++ b/packages/oss-console/src/components/data/QueryAuthorizationObserver.tsx
@@ -13,8 +13,7 @@ export const QueryAuthorizationObserver: React.FC = () => {
   React.useEffect(() => {
     const unsubscribe = queryCache.subscribe(async (_query?: Query | undefined) => {
       if (!onlineManager.isOnline()) {
-        // trigger sign in modal
-        apiContext.loginStatus.setExpired(true);
+        apiContext.relogin();
       }
     });
     return unsubscribe;
diff --git a/packages/oss-console/src/components/hooks/useFetchableData.ts b/packages/oss-console/src/components/hooks/useFetchableData.ts
index 9d6c440..91b9b61 100644
--- a/packages/oss-console/src/components/hooks/useFetchableData.ts
+++ b/packages/oss-console/src/components/hooks/useFetchableData.ts
@@ -72,8 +72,7 @@ function createFetchFn<T extends object, DataType>({
       return mergedValue;
     } catch (error) {
       if (error instanceof NotAuthorizedError) {
-        // Trigger auth flow
-        apiContext.loginStatus.setExpired(true);
+        apiContext.relogin();
       }
       return Promise.reject(error instanceof Error ? error : new Error(JSON.stringify(error)));
     }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlogged For internal use. Reserved for contributor team workflow. bug Something isn't working ui Admin console user interface
Projects
None yet
Development

No branches or pull requests

5 participants