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

Fix loginToken and legacy apiToken not working #8369

Merged
merged 1 commit into from
Nov 6, 2024
Merged

Conversation

charlesBochet
Copy link
Member

No description provided.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR modifies JWT token verification to maintain backward compatibility with legacy API keys while fixing login token functionality.

  • Modified jwt-wrapper.service.ts to handle legacy tokens without type/workspaceId fields using ACCESS_TOKEN_SECRET
  • Simplified generateAppSecretLegacy() to use raw ACCESS_TOKEN_SECRET instead of hashing
  • Added special case in verifyWorkspaceToken() to bypass sub check for FILE type tokens
  • Added specific error handling for TokenExpiredError and JsonWebTokenError with AuthException codes

1 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile

@@ -59,10 +59,11 @@ export class JwtWrapperService {
}

try {
if (!type && !payload.workspaceId) {
// TODO: Deprecate this once old API KEY tokens are no longer in use
if (!payload.type && !payload.workspaceId && type === 'ACCESS') {
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: This condition now only handles legacy ACCESS tokens, but doesn't handle legacy API_KEY tokens. May need to check if API_KEY type should also be included.

@charlesBochet charlesBochet merged commit 278ab4c into main Nov 6, 2024
19 checks passed
@charlesBochet charlesBochet deleted the last-fixes-0-32 branch November 6, 2024 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant