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

feat(language-server): hotfix for OAuth2 races #5717

Merged

Conversation

bastiandoetsch
Copy link
Contributor

@bastiandoetsch bastiandoetsch commented Feb 7, 2025

Pull Request Submission Checklist

  • Follows CONTRIBUTING guidelines
  • Includes detailed description of changes
  • Contains risk assessment (Low | Medium | High)
  • Highlights breaking API changes (if applicable)
  • Links to automated tests covering new functionality
  • Includes manual testing instructions (if necessary)
  • Updates relevant GitBook documentation (PR link: ___)

What does this PR do?

A potential race condition related to authentication has been identified in IntelliJ.

  1. IDE starts up CLI with initializationOptions incl. OAuth2 token
  2. LS starts WHOAMI workflow with (expired) credentials
  3. WHOAMI workflow refreshes OAuth2 token
  4. LS saves token and triggers $/hasAuthenticated with new token
  5. At the same time, IDE sends the current configuration with DidChangeConfiguration, with the old credentials
  6. LS updates the credentials to the old credentials
  7. Next call to API returns not authenticated and creds are deleted in LS
  8. LS forwards the logout to the IDE.

Solution: when updating the token, do not accept super-seded tokens (check Expiry)

Risk: Low and only affects language server

Where should the reviewer start?

reference: https://github.com/snyk/snyk-ls/releases/tag/hotfix-1.1295.3

@bastiandoetsch bastiandoetsch requested a review from a team as a code owner February 7, 2025 11:43
chore: adapt expectation after changing endpoint
Copy link
Contributor

github-actions bot commented Feb 7, 2025

Warnings
⚠️ There are multiple commits on your branch, please squash them locally before merging!

Generated by 🚫 dangerJS against 0e520a9

@PeterSchafer PeterSchafer merged commit 4a80e78 into release-candidate Feb 7, 2025
4 checks passed
@PeterSchafer PeterSchafer deleted the ls-hotfix-to-release-candidate-1.1295.3 branch February 7, 2025 17:41
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.

2 participants