Skip to content

Handle legacy jwt TokenLookup migration#240

Merged
ReneWerner87 merged 4 commits into
masterfrom
codex/2025-11-30-18-08-51
Nov 30, 2025
Merged

Handle legacy jwt TokenLookup migration#240
ReneWerner87 merged 4 commits into
masterfrom
codex/2025-11-30-18-08-51

Conversation

@ReneWerner87
Copy link
Copy Markdown
Member

@ReneWerner87 ReneWerner87 commented Nov 30, 2025

Summary

  • expand the JWT extractor migration to also process legacy github.com/gofiber/jwt import paths
  • add a regression test ensuring TokenLookup configurations from legacy imports are converted to extractors

Testing

  • make lint
  • make test (fails: vendor tree is out of sync with go.mod)

Codex Task

Summary by CodeRabbit

  • Bug Fixes
    • JWT import handling now supports both current and legacy package paths for improved compatibility
    • Enhanced detection of imports with inline comments and formatting variations
    • Improved robustness of JWT-related code processing

✏️ Tip: You can customize this high-level summary in your review settings.

@ReneWerner87 ReneWerner87 requested a review from a team as a code owner November 30, 2025 18:08
@ReneWerner87 ReneWerner87 requested review from efectn, gaby and sixcolors and removed request for a team November 30, 2025 18:08
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Nov 30, 2025

Warning

Rate limit exceeded

@ReneWerner87 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 51 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between ddeb0f5 and 88b6dd5.

⛔ Files ignored due to path filters (2)
  • go.mod is excluded by !**/*.mod
  • go.sum is excluded by !**/*.sum, !**/*.sum
📒 Files selected for processing (1)
  • cmd/internal/migrations/v3/jwt_extractor_test.go (2 hunks)

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Modified the JWT package import detection regex in the migration tool to accept both legacy and new import paths (github.com/gofiber/jwt and github.com/gofiber/contrib/jwt), and support inline comments. Added test coverage for the new import path formats.

Changes

Cohort / File(s) Change Summary
JWT Extractor Migration
cmd/internal/migrations/v3/jwt_extractor.go
Updated import detection regex to accept optional contrib/ path segment and trailing inline comments after import statement
JWT Extractor Migration Tests
cmd/internal/migrations/v3/jwt_extractor_test.go
Changed Extractor assertion from exact string match to Regexp-based matching for flexible whitespace handling; added two new test cases covering import paths with inline comments and legacy import paths

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Regex pattern review: Verify the updated regex correctly matches both new and legacy import path formats (contrib/ optional) and inline comment handling
  • Test assertion flexibility: Confirm Regexp-based assertions properly accommodate formatting variations without reducing test specificity
  • New test case logic: Validate the two new test cases exercise the intended code paths and maintain consistency with existing test structure

Possibly related PRs

Suggested labels

☢️ Bug

Suggested reviewers

  • gaby
  • sixcolors
  • efectn

Poem

🐰 A regex now bends with grace,
To catch both paths in their place,
Legacy and new, comments too,
The migration hops on through!
Tests hop along, so sturdy and strong. 🎉

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: expanding JWT extractor migration to handle legacy import paths, which aligns with the code modifications shown in the summary.
Description check ✅ Passed The description provides a clear summary of changes and testing steps. However, it does not fully explain the problem being solved or provide detailed reasoning for the changes as suggested by the template.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello @ReneWerner87, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the JWT extractor migration tool by extending its compatibility to include older github.com/gofiber/jwt import paths. This ensures that users with legacy configurations can seamlessly migrate their TokenLookup settings to the new Extractor format, preventing potential breakage during upgrades. The change involves a minor adjustment to a regular expression and the addition of a dedicated test case to validate the expanded coverage.

Highlights

  • Expanded JWT Extractor Migration: The migration logic for JWT extractors has been updated to recognize and process legacy import paths for github.com/gofiber/jwt, in addition to the existing github.com/gofiber/contrib/jwt.
  • New Regression Test: A new regression test has been added to specifically ensure that TokenLookup configurations from these newly supported legacy import paths are correctly converted into Extractor configurations during the migration process.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request correctly expands the JWT extractor migration to handle legacy github.com/gofiber/jwt import paths. The change to the import path regex is accurate, and the addition of a regression test for this legacy path is excellent for ensuring the migration works as expected. I have one suggestion to simplify the new test case to improve its clarity and consistency with other tests in the file.

Comment thread cmd/internal/migrations/v3/jwt_extractor_test.go
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
cmd/internal/migrations/v3/jwt_extractor_test.go (1)

127-155: LegacyImportPath test validates migration for github.com/gofiber/jwt

This regression test confirms that configs using the legacy github.com/gofiber/jwt/v2 import are migrated to a cookie Extractor, with the fiber/v3/extractors import and migration notice present, which is exactly what the new regex is supposed to enable. If you want to keep the fixture minimal (as suggested in the earlier review), you could drop the extra ErrorHandler/SigningKey fields since they don’t affect the text-based migration being asserted.

🧹 Nitpick comments (1)
cmd/internal/migrations/v3/jwt_extractor.go (1)

16-16: Regex extension correctly supports both contrib and legacy jwt imports

The updated reImport pattern cleanly handles optional aliases, both github.com/gofiber/contrib/jwt and github.com/gofiber/jwt (with optional /vN), and inline // comments without changing the alias capture group index. You might optionally add a short comment above it documenting the accepted import forms, since the regex is getting dense.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 922957d and ddeb0f5.

📒 Files selected for processing (2)
  • cmd/internal/migrations/v3/jwt_extractor.go (1 hunks)
  • cmd/internal/migrations/v3/jwt_extractor_test.go (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
cmd/internal/migrations/v3/jwt_extractor_test.go (1)
cmd/internal/migrations/v3/jwt_extractor.go (1)
  • MigrateJWTExtractor (15-95)
🪛 GitHub Actions: golangci-lint
cmd/internal/migrations/v3/jwt_extractor_test.go

[error] 39-39: golangci-lint: File is not properly formatted (gci).

🪛 GitHub Check: lint
cmd/internal/migrations/v3/jwt_extractor_test.go

[failure] 39-39:
File is not properly formatted (gci)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Build (1.25.x, macos-13)
  • GitHub Check: Build (1.25.x, ubuntu-latest)
  • GitHub Check: Build (1.25.x, macos-latest)
  • GitHub Check: Build (1.25.x, windows-latest)
🔇 Additional comments (2)
cmd/internal/migrations/v3/jwt_extractor_test.go (2)

91-95: Custom-alias migration assertions look correct

These assertions confirm that TokenLookup is removed, the cookie extractor is emitted, the extractors import is present, and the migration notice is printed for a custom alias, which matches the intended behavior.


97-125: ImportWithComment test covers inline-comment import regression

This test exercises a jwt import line with an inline // comment and verifies that TokenLookup is migrated to Extractor, the fiber/v3/extractors import is added, and the migration notice is printed. That gives good, focused coverage for the new regex support of commented imports.

Comment thread cmd/internal/migrations/v3/jwt_extractor_test.go Outdated
@ReneWerner87 ReneWerner87 added the 🤖 Dependencies Pull requests that update a dependency file label Nov 30, 2025
@ReneWerner87 ReneWerner87 merged commit cc00b7f into master Nov 30, 2025
14 checks passed
@ReneWerner87 ReneWerner87 deleted the codex/2025-11-30-18-08-51 branch November 30, 2025 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

codex 🤖 Dependencies Pull requests that update a dependency file 🧹 Updates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant