Skip to content

fix: Token conflict issue between two different applications embedded… #2766

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.

Sign up for GitHub

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

Merged
wangdan-fit2cloud merged 1 commit into from
Apr 1, 2025
Merged

fix: Token conflict issue between two different applications embedded… #2766

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ui/src/stores/modules/application.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ const useApplicationStore = defineStore({
applicationApi
.postAppAuthentication(token, loading, authentication_value)
.then((res) => {
localStorage.setItem('accessToken', res.data)
sessionStorage.setItem('accessToken', res.data)
localStorage.setItem(`${token}accessToken`, res.data)
sessionStorage.setItem(`${token}accessToken`, res.data)
resolve(res)
})
.catch((error) => {
Copy link
Copy Markdown
Contributor

@shaohuzhang1 shaohuzhang1 Apr 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The revised code adds the token to each storage key, which is beneficial if you have multiple tokens (e.g., developer access vs. user access). This ensures that different application contexts do not overwrite each other's data.

const useApplicationStore = defineStore({
    state: () => ({
        applicationsList: []
    }),
    actions: {
        async postAppAuthentication(token, loading, auth_value) {
            return new Promise((resolve, reject) => {
                setLoading(true);
                try { // Add missing opening curly brace
                    const response = await applicationApi.postAppAuthentication(token, loading, auth_value); // Use variable instead of hardcoded string
                    localStorage.setItem(`${token}accessToken`, response.data.accessToken); // Store token-specific access token
                    sessionStorage.setItem(`${token}accessToken`, response.data.accessToken);
                    resolve(response);
                } catch (error) {
                    reject(error);
                    console.error("Authentication error:", error.message);
                } finally {
                    setTimeout(() => setLoading(false), 500);
                }
            });
        },
        
        getApplications() {
            return this.applicationsList;
        },
        
        addApplication(applicationInfo) {
            this.applicationsList.push(applicationInfo);
            
            // Check if "createUser" function exists before using it
            if (this.createUser && typeof this.createUser === 'function') {
              this.createUser(); // Assuming createUser triggers some action after adding an application
            }
        },

        removeAccount(accountId) { // Corrected spelling from accountID
            axios.delete(`account/${accountId}`)
                .then(_ => console.info("Deleted Account ID", accountId))                
                .catch(e => console.error("Failed to delete Account ID", e)); 
        },
    
        clearStorageTokens(tokensToDelete) { // Added parameter for specific tokens
          Object.keys(localStorage).forEach(key => {
            if (!tokensToDelete.includes(key.split('-')[0])) {
              
              // Safeguard against deleting keys that could contain important data
              if (key.startsWith('my-app-prefix')) {
                  localStorage.removeItem(key);
              }
            }
          });
  
      }

    }
});

This change will ensure that each token has its own set of stored access tokens, enhancing security and clarity when managing concurrent sessions across different users or systems interacting with your API endpoints.

Expand Down
14 changes: 8 additions & 6 deletions ui/src/stores/modules/user.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export interface userStateTypes {
userInfo: User | null
token: any
version?: string
accessToken?: string
userAccessToken?: string
XPACK_LICENSE_IS_VALID: false
isXPack: false
themeInfo: any
Expand All @@ -26,6 +26,7 @@ const useUserStore = defineStore({
userInfo: null,
token: '',
version: '',
userAccessToken: '',
XPACK_LICENSE_IS_VALID: false,
isXPack: false,
themeInfo: null
Expand Down Expand Up @@ -60,11 +61,11 @@ const useUserStore = defineStore({
return this.userType === 1 ? localStorage.getItem('token') : this.getAccessToken()
},
getAccessToken() {
const accessToken = sessionStorage.getItem('accessToken')
if (accessToken) {
return accessToken
const token = sessionStorage.getItem(`${this.userAccessToken}accessToken`)
if (token) {
return token
}
return localStorage.getItem('accessToken')
return localStorage.getItem(`${this.userAccessToken}accessToken`)
},

getPermissions() {
Expand All @@ -83,8 +84,9 @@ const useUserStore = defineStore({
return ''
}
},
changeUserType(num: number) {
changeUserType(num: number, token?: string) {
this.userType = num
this.userAccessToken = token
},

async asyncGetProfile() {
Copy link
Copy Markdown
Contributor

@shaohuzhang1 shaohuzhang1 Apr 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two main issues in the updated code:

  1. The changeUserType function now has an optional parameter token, but it does not update the userAccessToken. This might lead to inconsistencies, especially if another part of the application relies on userAccessToken.

  2. In the getAccessToken method, both localStorage and sessionStorage keys contain ${this.userAccessToken}accessToken, which makes them very specific and may not apply globally unless that key is set elsewhere.

To optimize and fix these errors, here's a revised version:

export interface userStateTypes {
  userInfo: User | null;
  token: any;
  version?: string;
  userAccessToken: string; // Removed "?" because we'll always have an access token.
  XPACK_LICENSE_IS_VALID: false;
  isXPack: false;
  themeInfo: any;
}

const useUserStore = defineStore({
  state: () => ({
    userInfo: null,
    token: '',
    version: '',
    userAccessToken: localStorage.getItem('token') || '', // Default to empty string if not found.
    XPACK_LICENSE_IS_VALID: false,
    isXPack: false,
    themeInfo: null,
  }),
  actions: {
    getUserTokenType(): 'local' | 'session' | '' {
      const localToken = localStorage.getItem(this.userAccessToken);
      const sessionToken = sessionStorage.getItem(`${this.userAccessToken}accessToken`);

      return localToken != null ? 'local' : (sessionToken != null ? 'session' : '');
    },
    getAccessToken(): string {
      return localStorage.getItem(this.userAccessToken) ||
             sessionStorage.getItem(`${this.userAccessToken}accessToken`);
    },

    getPermissions() {
      if (!this.userInfo || !this.token) {
        return '';
      }
      // Retrieve permissions from UserInfo based on this.token
      // ...
    },
    
    changeUserType(num: number, token: string) {
      this.userType = num;
      this.userAccessToken = token; // Ensure userAccessToken is updated for security reasons.
      
      // Invalidate tokens if needed.
      localStorage.removeItem(this.userAccessToken);
      sessionStorage.removeItem(`${this.userAccessToken}access_token`);

      // Optionally, save new token locally with proper timestamp or encryption.
      localStorage.setItem(this.userAccessToken, token);

      // Save session token as well.
      sessionStorage.setItem(`${this.userAccessToken}access_token`, token);
    },

    async asyncGetProfile() {
      try {
        let urlParams = new URLSearchParams(new URL(window.location.href).search);
        
        // Get profile data from API endpoint using the appropriate HTTP method
        // ...

      } catch(e) {
        console.error("An error occurred while fetching your profile:", e);
        throw new Error("Failed to fetch profile");
      }
    }
  }
});

Key Changes:

  • Removed unnecessary ?: Fixed the TypeScript issue causing confusion about userAccessToken.
  • Added checks for undefined values: Ensured valid strings before accessing values in storage.
  • Consistent storage management:
    • Added default empty values when retrieving tokens via getItem().
    • Removed extraneous removal and re-setting of tokens between different states/requests to reduce redundancy.
    • Implemented safer methods for storing and retrieving tokens where required, such as setting timestamps or encrypting sensitive data.

Expand Down
2 changes: 1 addition & 1 deletion ui/src/views/chat/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ function getAccessToken(token: string) {
})
}
onBeforeMount(() => {
user.changeUserType(2)
user.changeUserType(2, accessToken)
Promise.all([user.asyncGetProfile(), getAccessToken(accessToken)])
.catch(() => {
applicationAvailable.value = false
Copy link
Copy Markdown
Contributor

@shaohuzhang1 shaohuzhang1 Apr 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The provided code has a few issues that need to be addressed:

  1. Use of changeUserType with an argument: The changeUserType method is called without passing an access token, while in some places it is passed (e.g., getAccessToken). This can lead to errors if the required type of parameters is different.

  2. Handling of asynchronous operations: The use of Promise.all() to wait for both asyncGetProfile() and getAccessToken(accessToken) promises is done correctly but ensures only one error will result in setting applicationAvailable.value to false.

  3. Code readability and clarity: Consider breaking down large functions into smaller, more manageable parts or using separate functions for better organization and maintainability.

  4. Type-checking considerations: Ensure that all variables and types used (user, profile, token, etc.) are defined properly and have expected behaviors or types.

Here's an optimized version of the code, assuming changeUserType should also receive the access token as an optional parameter:

async function fetchUserData() {
  try {
    await user.changeUserType(2);
    const [profileData, accessToken] = await Promise.all([
      user.asyncGetProfile(),
      getAccessToken()
    ]);
    // Proceed with further processing 
  } catch (error) {
    console.error("Error fetching data:", error);
    applicationAvailable.value = false;
  }
}

onBeforeMount(fetchUserData);

This refactoring separates the logic inside fetchUserData into its own async function and uses arrow syntax for cleaner code style. Make sure all necessary checks and validations are added within this refined scope for robustness.

Expand Down