Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions apps/client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
"@pivanov/vite-plugin-svg-sprite": "^3.1.3",
"@tailwindcss/vite": "^4.1.12",
"@tanstack/react-query-devtools": "^5.87.4",
"@types/chrome": "^0.0.273",
"@vitejs/plugin-react-swc": "^4.0.0",
"eslint": "^9.33.0",
"eslint-plugin-react-hooks": "^5.2.0",
Expand Down
7 changes: 7 additions & 0 deletions apps/client/src/pages/onBoarding/GoogleCallback.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,14 @@ const GoogleCallback = () => {
if (isUser) {
if (accessToken) {
localStorage.setItem('token', accessToken);

Copy link
Collaborator

Choose a reason for hiding this comment

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

크론도 따로 추가해줘야하는지 몰랐어요..반성하겠습니다

if (typeof chrome !== 'undefined' && chrome.storage?.local) {
chrome.storage.local.set({ token: accessToken }, () => {
console.log('Token saved to chrome storage');
});
}
Comment on lines +30 to +34
Copy link
Member

Choose a reason for hiding this comment

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

크게 console이 필요없다면 지워도 될 것 같아요!
그리고 추가로 typeof chrome !== 'undefined' && chrome.storage?.local이 조건은 어떤 조건인가요??
이름을 붙이는 것도 좋을 것 같아요!

}

navigate('/');
} else {
navigate('/onboarding?step=ALARM');
Expand Down
5 changes: 5 additions & 0 deletions apps/client/src/shared/apis/queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,11 @@ export const usePostSignUp = () => {
};
if (newToken) {
localStorage.setItem('token', newToken);
if (typeof chrome !== 'undefined' && chrome.storage?.local) {
chrome.storage.local.set({ token: newToken }, () => {
console.log('Token saved to chrome storage');
});
}
Comment on lines 81 to +87
Copy link
Member

Choose a reason for hiding this comment

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

localStorage set 하는 곳에서 chrome storage도 다 같이 해줘야 하니, 애초에 이 두개를 같이 하는 util을 추상화 시켜서 사용하면 좋을 것 같아요.

스프린트 작업 대충 끝나면 Storage를 class형태나 util로 빼서 재사용 하도록 만드는 것이 좋을 것 같은데 어떻게 생각하시나요?

sendTokenToExtension(newToken);
}

Expand Down
71 changes: 7 additions & 64 deletions apps/client/src/shared/apis/setting/axiosInstance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,55 +8,11 @@ const apiRequest = axios.create({
},
});

const refreshToken = async (email: string) => {
try {
const response = await axios.get(
`${import.meta.env.VITE_BASE_URL}/api/v1/auth/token`,
{
params: { email },
}
);

const newToken = response.data.data?.token || response.data.token;

if (newToken) {
localStorage.setItem('token', newToken);
return newToken;
}

throw new Error('토큰 재발급 실패');
} catch (error) {
console.error('토큰 재발급 실패:', error);
throw error;
}
};

// 요청 인터셉터
apiRequest.interceptors.request.use(async (config) => {
const noAuthNeeded = [
'/api/v1/auth/token',
'/api/v2/auth/signup',
'/api/v2/auth/google',
];
const isNoAuth = noAuthNeeded.some((url) => config.url?.includes(url));

if (!isNoAuth) {
let token = localStorage.getItem('token');
const email = localStorage.getItem('email');
if (email) {
try {
token = await refreshToken(email);
} catch (err) {
console.error('요청 인터셉터에서 토큰 재발급 실패:', err);
localStorage.removeItem('token');
window.location.href = '/onboarding';
throw err;
}
} else {
console.error('토큰이 없습니다. 온보딩을 먼저 완료해주세요.');
throw new Error('토큰이 없습니다. 온보딩을 먼저 완료해주세요.');
}
const token = localStorage.getItem('token');

if (token) {
config.headers.Authorization = `Bearer ${token}`;
}

Expand All @@ -68,11 +24,13 @@ apiRequest.interceptors.response.use(
(response) => response,
async (error) => {
const originalRequest = error.config;

const noAuthNeeded = [
'/api/v1/auth/token',
'/api/v2/auth/signup',
'/api/v2/auth/google',
];

const isNoAuth = noAuthNeeded.some((url) =>
originalRequest.url?.includes(url)
);
Expand All @@ -85,25 +43,10 @@ apiRequest.interceptors.response.use(
) {
originalRequest._retry = true;

try {
const email = localStorage.getItem('email');
localStorage.removeItem('token');
window.location.href = '/onboarding?step=SOCIAL_LOGIN';
Comment on lines +46 to +47
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Chrome storage도 함께 정리해야 합니다.

이 PR은 온보딩 시 토큰을 localStorage와 chrome.storage 양쪽에 저장하도록 변경합니다. 하지만 인증 오류(401/403) 발생 시 localStorage만 삭제하고 chrome.storage는 정리하지 않아 두 저장소 간 불일치가 발생합니다.

🔎 chrome.storage도 정리하는 수정안
      originalRequest._retry = true;

      localStorage.removeItem('token');
+     if (typeof chrome !== 'undefined' && chrome.storage?.local) {
+       chrome.storage.local.remove('token', () => {
+         console.log('Token removed from chrome storage');
+       });
+     }
      window.location.href = '/onboarding?step=SOCIAL_LOGIN';

      return Promise.reject(error);

또는 이전 리뷰 코멘트에서 제안된 중앙화된 유틸리티 함수를 사용하는 것을 권장합니다:

// shared/utils/storage.ts에 추가
export const removeToken = async (): Promise<void> => {
  localStorage.removeItem('token');
  
  if (typeof chrome !== 'undefined' && chrome.storage?.local) {
    try {
      await chrome.storage.local.remove('token');
      console.log('Token removed from chrome storage');
    } catch (error) {
      console.error('Chrome storage 삭제 실패:', error);
    }
  }
};

그 다음 이 파일에서:

      originalRequest._retry = true;

-     localStorage.removeItem('token');
+     await removeToken();
      window.location.href = '/onboarding?step=SOCIAL_LOGIN';

      return Promise.reject(error);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
localStorage.removeItem('token');
window.location.href = '/onboarding?step=SOCIAL_LOGIN';
localStorage.removeItem('token');
if (typeof chrome !== 'undefined' && chrome.storage?.local) {
chrome.storage.local.remove('token', () => {
console.log('Token removed from chrome storage');
});
}
window.location.href = '/onboarding?step=SOCIAL_LOGIN';
🤖 Prompt for AI Agents
In apps/client/src/shared/apis/setting/axiosInstance.ts around lines 46-47, the
code only clears localStorage on auth errors but leaves chrome.storage.token
intact; update the error handling to also remove the token from chrome.storage
(or call the centralized utility shared/utils/storage.removeToken if available)
and await its completion before redirecting to '/onboarding?step=SOCIAL_LOGIN';
ensure you guard for typeof chrome !== 'undefined' and chrome.storage?.local and
catch/log any chrome.storage errors so redirect still occurs even if chrome
removal fails.


if (email) {
const newToken = await refreshToken(email);
originalRequest.headers.Authorization = `Bearer ${newToken}`;
return apiRequest(originalRequest);
} else {
console.error(
'사용자 이메일이 없습니다. 온보딩을 다시 완료해주세요.'
);
localStorage.removeItem('token');
window.location.href = '/onboarding';
}
} catch (refreshError) {
console.error('토큰 재발급 실패:', refreshError);
localStorage.removeItem('token');
window.location.href = '/onboarding';
}
return Promise.reject(error);
}

return Promise.reject(error);
Expand Down
77 changes: 33 additions & 44 deletions apps/extension/src/apis/axiosInstance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,75 +7,64 @@ const apiRequest = axios.create({
},
});

const fetchToken = async (email?: string) => {
const response = await axios.get(
`${import.meta.env.VITE_BASE_URL}/api/v1/auth/token`,
{
params: { email },
}
);
const newToken = response.data.data.token;
chrome.storage.local.set({ token: newToken }, () => {
console.log('Token re-saved to chrome storage');
});
return newToken;
};

apiRequest.interceptors.request.use(async (config) => {
const noAuthNeeded = ['/api/v1/auth/token', '/api/v1/auth/signup'];
const isNoAuth = noAuthNeeded.some((url) => config.url?.includes(url));

if (isNoAuth) return config;

const email = await new Promise<string | undefined>((resolve) => {
chrome.storage.local.get('email', (result) => resolve(result.email));
});

let token = await new Promise<string | undefined>((resolve) => {
const getTokenFromStorage = () => {
return new Promise<string | undefined>((resolve) => {
chrome.storage.local.get('token', (result) => {
resolve(result.token);
});
});
};
Comment on lines +10 to +16
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

런타임 안전성 검사 및 에러 핸들링이 누락되었습니다.

PR objectives에서 명시한 대로 chrome API 호출 전에 TypeError 방지를 위한 조건문이 필요합니다. 또한 chrome.storage.local.get 실패 시 에러 처리가 없어 Promise가 영원히 pending 상태가 될 수 있습니다.

🔎 제안하는 수정안
 const getTokenFromStorage = () => {
   return new Promise<string | undefined>((resolve, reject) => {
+    if (typeof chrome === 'undefined' || !chrome.storage?.local) {
+      resolve(undefined);
+      return;
+    }
+
-    chrome.storage.local.get('token', (result) => {
+    chrome.storage.local.get('token', (result) => {
+      if (chrome.runtime.lastError) {
+        reject(chrome.runtime.lastError);
+        return;
+      }
       resolve(result.token);
     });
   });
 };
🤖 Prompt for AI Agents
In apps/extension/src/apis/axiosInstance.ts around lines 10-16,
getTokenFromStorage lacks runtime checks for the chrome API and has no error
handling which can cause TypeError or a permanently pending Promise; update the
function to first check that window.chrome and chrome.storage and
chrome.storage.local exist (return undefined if absent), wrap
chrome.storage.local.get in a try/catch or provide the optional error callback
to handle failures and ensure the Promise always resolves or rejects (resolve
undefined on error), and consider adding a short timeout fallback so the Promise
cannot remain pending indefinitely.


if (!isNoAuth) {
if (email) {
try {
token = await fetchToken(email);
} catch (err) {
console.error('요청 인터셉터에서 토큰 재발급 실패:', err);
localStorage.removeItem('token');
window.location.href = '/onboarding';
throw err;
}
} else {
throw new Error('토큰이 없습니다. 온보딩을 먼저 완료해주세요.');
}
apiRequest.interceptors.request.use(async (config) => {
const token = await getTokenFromStorage();

if (token) {
config.headers.Authorization = `Bearer ${token}`;
}

return config;
});

// TODO: 환경변수로 분리
// eslint-disable-next-line turbo/no-undeclared-env-vars
const onboardingUrl = import.meta.env.DEV
? 'http://localhost:5173/onboarding?step=SOCIAL_LOGIN'
: 'https://pinback.today/onboarding?step=SOCIAL_LOGIN';

let isRedirecting = false;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

리다이렉트 플래그에 잠재적 경쟁 조건이 있습니다.

모듈 레벨의 isRedirecting 플래그는 여러 요청이 동시에 401/403을 받을 경우 제대로 동작하지 않을 수 있습니다. 또한 고정된 2초 타임아웃에 의존하여 플래그를 리셋하는 것은 탭 생성이 실패하거나 지연될 경우 문제가 될 수 있습니다.

💡 개선 제안
-let isRedirecting = false;
+let redirectPromise: Promise<void> | null = null;

그리고 응답 인터셉터에서:

-      if (!isRedirecting) {
-        isRedirecting = true;
+      if (!redirectPromise) {
+        redirectPromise = new Promise((resolve) => {
+          chrome.storage.local.remove(['token', 'email'], () => {});
 
-        chrome.storage.local.remove(['token', 'email'], () => {});
-
-        chrome.tabs.create({ url: onboardingUrl }, () => {
-          setTimeout(() => {
-            isRedirecting = false;
-          }, 2000);
-        });
+          chrome.tabs.create({ url: onboardingUrl }, () => {
+            redirectPromise = null;
+            resolve();
+          });
+        });
       }

Committable suggestion skipped: line range outside the PR's diff.


apiRequest.interceptors.response.use(
(response) => response,
async (error) => {
const originalRequest = error.config;
const noAuthNeeded = ['/api/v1/auth/token', '/api/v1/auth/signup'];

const noAuthNeeded = [
'/api/v1/auth/token',
'/api/v1/auth/signup',
'/api/v2/auth/google',
];
const isNoAuth = noAuthNeeded.some((url) =>
originalRequest.url?.includes(url)
);

if (
error.response &&
(error.response.status === 401 || error.response.status === 403) &&
!originalRequest._retry &&
!isNoAuth
) {
originalRequest._retry = true;
const newToken = await fetchToken('[email protected]');
originalRequest.headers.Authorization = `Bearer ${newToken}`;
return apiRequest(originalRequest);
if (!isRedirecting) {
isRedirecting = true;

chrome.storage.local.remove(['token', 'email'], () => {});

chrome.tabs.create({ url: onboardingUrl }, () => {
setTimeout(() => {
isRedirecting = false;
}, 2000);
});
}
Comment on lines +55 to +65
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Chrome API 호출에 에러 핸들링과 런타임 검사가 누락되었습니다.

chrome.storage.local.removechrome.tabs.create 호출에서 실패 케이스를 처리하지 않고 있으며, PR objectives에서 명시한 런타임 안전성 검사도 없습니다.

🔎 제안하는 수정안
       if (!isRedirecting) {
         isRedirecting = true;
 
-        chrome.storage.local.remove(['token', 'email'], () => {});
+        if (typeof chrome !== 'undefined' && chrome.storage?.local) {
+          chrome.storage.local.remove(['token', 'email'], () => {
+            if (chrome.runtime.lastError) {
+              console.error('토큰 제거 실패:', chrome.runtime.lastError);
+            }
+          });
+        }
 
-        chrome.tabs.create({ url: onboardingUrl }, () => {
-          setTimeout(() => {
-            isRedirecting = false;
-          }, 2000);
-        });
+        if (typeof chrome !== 'undefined' && chrome.tabs?.create) {
+          chrome.tabs.create({ url: onboardingUrl }, () => {
+            if (chrome.runtime.lastError) {
+              console.error('탭 생성 실패:', chrome.runtime.lastError);
+            }
+            setTimeout(() => {
+              isRedirecting = false;
+            }, 2000);
+          });
+        } else {
+          isRedirecting = false;
+        }
       }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!isRedirecting) {
isRedirecting = true;
chrome.storage.local.remove(['token', 'email'], () => {});
chrome.tabs.create({ url: onboardingUrl }, () => {
setTimeout(() => {
isRedirecting = false;
}, 2000);
});
}
if (!isRedirecting) {
isRedirecting = true;
if (typeof chrome !== 'undefined' && chrome.storage?.local) {
chrome.storage.local.remove(['token', 'email'], () => {
if (chrome.runtime.lastError) {
console.error('토큰 제거 실패:', chrome.runtime.lastError);
}
});
}
if (typeof chrome !== 'undefined' && chrome.tabs?.create) {
chrome.tabs.create({ url: onboardingUrl }, () => {
if (chrome.runtime.lastError) {
console.error('탭 생성 실패:', chrome.runtime.lastError);
}
setTimeout(() => {
isRedirecting = false;
}, 2000);
});
} else {
isRedirecting = false;
}
}
🤖 Prompt for AI Agents
In apps/extension/src/apis/axiosInstance.ts around lines 55-65, the chrome API
calls lack runtime existence checks and error handling; guard that chrome,
chrome.storage?.local and chrome.tabs exist before calling, check
chrome.runtime?.lastError inside each callback, and handle failures (log/report
and ensure isRedirecting is reset on error); also propagate or handle callback
errors instead of ignoring empty callbacks and ensure setTimeout still clears
isRedirecting if chrome.tabs.create fails.

}

return Promise.reject(error);
}
);
Expand Down
3 changes: 3 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading