Conversation
urkishan
force-pushed
the
feature/redis-cluster
branch
from
da0510c to
cf0056f
Compare
Feature: Added methods to update refresh token expiry
WalkthroughRenames uid → uuid across the JWT sessions adapters and tokens, adds JTI↔UUID helpers, adds Redis SCAN support and redis_cluster/password options, and introduces methods for updating refresh expirations and explicit refresh UUID management. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes Areas to focus on during review:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (3)
lib/jwt_sessions/refresh_token.rb (3)
52-53: Update necessary fordestroymethod calls to match the new signatureThe
destroymethod inlib/jwt_sessions/refresh_token.rbhas been updated with the new parameters. However, there are calls todestroyin other files that still use the old parameter names. Please update these calls to match the new signature:
lib/jwt_sessions/session.rbtest/units/jwt_sessions/test_access_token.rbtest/units/jwt_sessions/test_refresh_token.rbAnalysis chain
LGTM! But verify the function usage in the codebase.
The changes in the
destroymethod are consistent and correctly applied.However, ensure that all calls to
destroymatch the new signature.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all calls to `destroy` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type ruby -A 5 $'def destroy(uuid, store, namespace)'Length of output: 59
Script:
#!/bin/bash # Description: Verify all calls to `destroy` match the new signature. # Search for the definition of the destroy method with the new signature. rg --type ruby 'def destroy(uuid, store, namespace)' # Search for calls to the destroy method to ensure they match the new signature. rg --type ruby 'destroy('Length of output: 149
Script:
#!/bin/bash # Description: Verify all calls to `destroy` match the new signature. # Search for the definition of the destroy method with the new signature. rg --type ruby 'def destroy\(uuid, store, namespace\)' # Search for calls to the destroy method to ensure they match the new signature. rg --type ruby 'destroy\('Length of output: 961
72-81: Ensure all calls toupdatematch the new signature.The test file
test/units/jwt_sessions/test_refresh_token.rbusesnew_access_uidinstead ofnew_access_uuid. Update this to ensure consistency:
test/units/jwt_sessions/test_refresh_token.rb:
- Change
new_access_uidtonew_access_uuidin theupdatemethod call.Other occurrences in
lib/jwt_sessions/session.rbare correct.Analysis chain
LGTM! But verify the function usage in the codebase.
The changes in the
updatemethod are consistent and correctly applied.However, ensure that all calls to
updatematch the new signature.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all calls to `update` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type ruby -A 5 $'def update(access_uuid, access_expiration, csrf)'Length of output: 72
Script:
#!/bin/bash # Description: Verify all calls to `update` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type ruby '\.update\('Length of output: 327
Script:
#!/bin/bash # Description: Verify the parameter names in the calls to the `update` method. # Check the specific lines in the files where `update` is called. rg --type ruby -A 2 -B 2 '\.update\(' test/units/jwt_sessions/test_refresh_token.rb lib/jwt_sessions/session.rbLength of output: 1275
46-49: Update Required: Ensure all calls tofindmatch the new signatureThe changes in the
findmethod are consistent and correctly applied inlib/jwt_sessions/session.rb. However, there are multiple instances in the test files where the old signature withuidis still used. Please update these calls to match the new signature withuuid.
- Files with old signature usage:
test/units/jwt_sessions/test_session.rbtest/units/jwt_sessions/test_refresh_token.rbAnalysis chain
LGTM! But verify the function usage in the codebase.
The changes in the
findmethod are consistent and correctly applied.However, ensure that all calls to
findmatch the new signature.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all calls to `find` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type ruby -A 5 $'def find(uuid, store, namespace = nil, first_match: false)'Length of output: 82
Script:
#!/bin/bash # Description: Verify all calls to `find` match the new signature. # Search for the method definition to confirm its presence. rg --type ruby 'def find(' # Search for all calls to the `find` method. rg --type ruby '\.find('Length of output: 189
Script:
#!/bin/bash # Description: Verify all calls to `find` match the new signature. # Search for the method definition to confirm its presence. rg --type ruby 'def find\(' # Search for all calls to the `find` method. rg --type ruby '\.find\('Length of output: 2853
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (13)
- .gitignore (1 hunks)
- lib/jwt_sessions/access_token.rb (2 hunks)
- lib/jwt_sessions/authorization.rb (2 hunks)
- lib/jwt_sessions/rails_authorization.rb (1 hunks)
- lib/jwt_sessions/refresh_token.rb (2 hunks)
- lib/jwt_sessions/session.rb (9 hunks)
- lib/jwt_sessions/store_adapters/abstract_store_adapter.rb (1 hunks)
- lib/jwt_sessions/store_adapters/memory_store_adapter.rb (3 hunks)
- lib/jwt_sessions/store_adapters/redis_store_adapter.rb (4 hunks)
- test/units/jwt_sessions/store_adapters/test_memory_store_adapter.rb (1 hunks)
- test/units/jwt_sessions/test_access_token.rb (1 hunks)
- test/units/jwt_sessions/test_refresh_token.rb (2 hunks)
- test/units/jwt_sessions/test_session.rb (7 hunks)
Files skipped from review due to trivial changes (4)
- .gitignore
- lib/jwt_sessions/rails_authorization.rb
- lib/jwt_sessions/store_adapters/abstract_store_adapter.rb
- test/units/jwt_sessions/test_access_token.rb
Additional comments not posted (72)
lib/jwt_sessions/access_token.rb (9)
5-5: Update theattr_readerto useuuid.The
attr_readerhas been updated to useuuidinstead ofuid.
7-11: Update theinitializemethod to useuuid.The
initializemethod now usesuuidinstead ofuid. Ensure that all instances ofuidin the payload are updated touuid.
16-16: Update thedestroymethod to useuuid.The
destroymethod now usesuuidinstead ofuid. Ensure thatstore.destroy_accesscorrectly handlesuuid.
19-20: Renamerefresh_uid=torefresh_uuid=.The method
refresh_uid=has been renamed torefresh_uuid=and updates the payload correctly.
23-24: Renamerefresh_uidtorefresh_uuid.The method
refresh_uidhas been renamed torefresh_uuidand accesses the payload correctly.
34-34: Update thecreatemethod to useuuid.The
createmethod now usesuuidinstead ofuid. Ensure thatstore.persist_accesscorrectly handlesuuid.
38-39: Update the class methoddestroyto useuuid.The class method
destroynow usesuuidinstead ofuid. Ensure thatstore.destroy_accesscorrectly handlesuuid.
45-48: Update thefindmethod to useuuid.The
findmethod now usesuuidinstead ofuid. Ensure thatstore.fetch_accesscorrectly handlesuuid.
53-54: Update thebuild_with_token_attrsmethod to useuuid.The
build_with_token_attrsmethod now usesuuidinstead ofuid. Ensure that the method correctly handlesuuid.test/units/jwt_sessions/test_refresh_token.rb (2)
7-7: Update theattr_readerto useaccess_uuid.The
attr_readerhas been updated to useaccess_uuidinstead ofaccess_uid.
32-37: Add a new test methodtest_update_expiry.A new test method
test_update_expiryhas been added to test the functionality of updating the expiration time of a token. Ensure that the test covers all relevant scenarios.test/units/jwt_sessions/store_adapters/test_memory_store_adapter.rb (3)
20-21: Update thetest_persist_and_fetch_accessmethod to useuuid.The
test_persist_and_fetch_accessmethod now usesuuidinstead ofuid. Ensure that the method correctly handlesuuid.
30-37: Update thetest_persist_and_fetch_refreshmethod to useuuid.The
test_persist_and_fetch_refreshmethod now usesuuidinstead ofuid. Ensure that the method correctly handlesuuid.
56-56: Update thetest_update_refreshmethod to useuuid.The
test_update_refreshmethod now usesuuidinstead ofuid. Ensure that the method correctly handlesuuid.lib/jwt_sessions/refresh_token.rb (4)
39-40: LGTM! But verify the function usage in the codebase.The changes in the
allmethod are consistent and correctly applied.However, ensure that all calls to
allmatch the new signature.Verification successful
We need to re-verify the usage of the
allmethod to ensure it matches the new signature. Let's search for all occurrences of theallmethod in the codebase.
We need to correct the regex pattern and re-run the search to verify the usage of the
allmethod.
Verified: The function usage matches the new signature.
The
allmethod is consistently called with the new signature (namespace, store) across the codebase.
lib/jwt_sessions/session.rbtest/units/jwt_sessions/test_refresh_token.rbScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all calls to `all` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type ruby -A 5 $'def all(namespace, store)'Length of output: 49
Script:
#!/bin/bash # Description: Verify all calls to `all` match the new signature. # Test: Search for the function usage in a more general way. Expect: Occurrences of the `all` method usage. rg --type ruby 'all('Length of output: 82
Script:
#!/bin/bash # Description: Verify all calls to `all` match the new signature. # Test: Search for the function usage with corrected regex pattern. Expect: Occurrences of the `all` method usage. rg --type ruby 'all\('Length of output: 578
85-90: LGTM! But verify the function usage in the codebase.The new
update_expirymethod is correctly implemented and consistent with the existing code.However, ensure that all calls to
update_expiryare correctly implemented.Verification successful
LGTM! The
update_expirymethod is correctly implemented and its usage is verified in the codebase.The method is utilized in the following files:
test/units/jwt_sessions/test_refresh_token.rbtest/units/jwt_sessions/test_session.rblib/jwt_sessions/session.rbThe method is also being tested, ensuring its correct functionality.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all calls to `update_expiry` are correctly implemented. # Test: Search for the function usage. Expect: Only occurrences of the new method. rg --type ruby -A 5 $'def update_expiry(uuid, expiration)'Length of output: 59
Script:
#!/bin/bash # Description: Verify all calls to `update_expiry` are correctly implemented. # Test: Search for the function calls. Expect: All occurrences of the new method. rg --type ruby -A 5 'update_expiry('Length of output: 117
Script:
#!/bin/bash # Description: Verify all calls to `update_expiry` are correctly implemented. # Test: Search for the function calls. Expect: All occurrences of the new method. rg --type ruby -A 5 'update_expiry\('Length of output: 1602
5-19: LGTM! But verify the function usage in the codebase.The changes in the
initializemethod are consistent and correctly applied.However, ensure that all calls to
initializematch the new signature.Verification successful
Let's correct the regex syntax error and re-run the verification.
Let's correct the regex by escaping the parentheses and re-run the verification.
Let's simplify the search pattern to just look for the
initializemethod and manually inspect the results.
LGTM! But verify the function usage in the codebase.
The changes in the
initializemethod are consistent and correctly applied.
- The
initializemethod inrefresh_token.rbmatches the new signature.- Other
initializemethods in different files have different signatures, which is expected.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all calls to `initialize` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type ruby -A 5 $'def initialize(csrf, access_uuid, access_expiration, store, options = {})'Length of output: 324
Script:
#!/bin/bash # Description: Verify all calls to `initialize` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type ruby -A 5 'def initialize(csrf, access_uuid, access_expiration, store, options = {})'Length of output: 324
Script:
#!/bin/bash # Description: Verify all calls to `initialize` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type ruby -A 5 'def initialize\(csrf, access_uuid, access_expiration, store, options = {}\)'Length of output: 329
Script:
#!/bin/bash # Description: Verify all calls to `initialize` match the new signature. # Test: Search for the function usage. Expect: Occurrences of the `initialize` method. rg --type ruby -A 5 'def initialize'Length of output: 2969
Line range hint
23-31:
LGTM! But verify the function usage in the codebase.The changes in the
createmethod are consistent and correctly applied.However, ensure that all calls to
creatematch the new signature.Verification successful
Verification Successful: All calls to the
createmethod match the new signature.
lib/jwt_sessions/refresh_token.rb:
- The
createmethod is defined with the new parameter names.- Calls to
createuse the new parameter names correctly.lib/jwt_sessions/session.rb:
- Calls to
RefreshToken.createuse the new parameter names correctly.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all calls to `create` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type ruby -A 5 $'def create(csrf, access_uuid, access_expiration, store, payload, namespace, expiration = JWTSessions.refresh_expiration)'Length of output: 144
Script:
#!/bin/bash # Description: Verify all calls to `create` method match the new signature. # Test: Search for all calls to the `create` method in the codebase. rg --type ruby 'create('Length of output: 91
Script:
#!/bin/bash # Description: Verify all calls to `create` method match the new signature. # Test: Search for all calls to the `create` method in the codebase with corrected regex pattern. rg --type ruby 'create\('Length of output: 1474
Script:
#!/bin/bash # Description: Verify the calls to `create` method in relevant files match the new signature. # Test: Extract lines around the `create` calls in `lib/jwt_sessions/refresh_token.rb` and `lib/jwt_sessions/session.rb`. rg --type ruby -A 3 'create\(' lib/jwt_sessions/refresh_token.rb lib/jwt_sessions/session.rbLength of output: 854
lib/jwt_sessions/store_adapters/redis_store_adapter.rb (17)
23-25: LGTM!The changes improve semantic clarity by using
uuidinstead ofuid.
28-31: LGTM!The changes improve semantic clarity by using
uuidinstead ofuid.
34-39: LGTM!The changes improve semantic clarity by using
uuidinstead ofuid.
42-52: LGTM!The changes improve semantic clarity by using
uuidinstead ofuid.
55-61: LGTM!The changes improve semantic clarity by using
uuidinstead ofuid.
64-68: LGTM!The new method enhances the functionality for managing token expiration.
69-74: LGTM!The changes improve semantic clarity by using
uuidinstead ofuid.
78-80: LGTM!The changes improve semantic clarity by using
uuidinstead ofuid.
83-85: LGTM!The changes improve semantic clarity by using
uuidinstead ofuid.
87-89: LGTM!The new method enhances the functionality for managing token UUIDs.
91-93: LGTM!The new method enhances the functionality for managing token UUIDs with expiration.
Line range hint
97-115: LGTM!The additional parameters enhance the configuration options for connecting to a Redis instance.
129-131: LGTM!The changes improve semantic clarity by using
uuidinstead ofuid.
133-136: LGTM!The changes improve semantic clarity by using
uuidinstead ofuid.
138-140: LGTM!The changes improve semantic clarity by using
uuidinstead ofuid.
143-145: LGTM!The changes improve semantic clarity by using
uuidinstead ofuid.
147-149: LGTM!The changes improve semantic clarity by using
uuidinstead ofuid.lib/jwt_sessions/store_adapters/memory_store_adapter.rb (10)
15-17: LGTM!The parameter renaming from
uidtouuidimproves clarity and consistency.
20-22: LGTM!The parameter renaming from
uidtouuidimproves clarity and consistency.
25-33: LGTM!The parameter renaming from
uidtouuidimproves clarity and consistency.
37-43: LGTM!The parameter renaming from
uidtouuidimproves clarity and consistency.
48-54: LGTM!The parameter renaming from
uidtouuidimproves clarity and consistency.
58-60: LGTM!The method correctly updates the expiration time of the refresh token.
70-71: LGTM!The parameter renaming from
uidtouuidimproves clarity and consistency.
74-75: LGTM!The parameter renaming from
uidtouuidimproves clarity and consistency.
78-79: LGTM!The method correctly retrieves the UUID based on the provided key and namespace.
82-83: LGTM!The method correctly stores the refresh token associated with a key and expiration time in the storage.
lib/jwt_sessions/authorization.rb (5)
70-70: LGTM!The change from double quotes to single quotes improves consistency in string formatting.
74-74: LGTM!The change from double quotes to single quotes improves consistency in string formatting.
78-78: LGTM!The change from double quotes to single quotes improves consistency in string formatting.
101-101: LGTM!The change from double quotes to single quotes improves consistency in string formatting.
106-109: LGTM!The enhancement to include a fallback mechanism that checks the
envof the request headers improves the robustness of the token retrieval process.lib/jwt_sessions/session.rb (17)
35-40: LGTM!The method correctly facilitates the renewal of tokens based on a given
jtiand expiration time.
59-59: LGTM!The change from
refresh_by_uidtorefresh_by_uuidimproves consistency with the new identifier schema.
64-69: LGTM!The change from
uidtouuidimproves consistency with the new identifier schema.
74-75: LGTM!The change from
uidtouuidimproves consistency with the new identifier schema.
80-81: LGTM!The change from
uidtouuidimproves consistency with the new identifier schema.
85-88: LGTM!The method correctly flushes the session by refresh token UUID.
93-96: LGTM!The method correctly flushes the session by refresh token JTI.
103-103: LGTM!The change from
access_uidtoaccess_uuidimproves consistency with the new identifier schema.
113-113: LGTM!The change from
access_uidtoaccess_uuidimproves consistency with the new identifier schema.
121-121: LGTM!The change from
access_uidtoaccess_uuidimproves consistency with the new identifier schema.
127-131: LGTM!The change from
uidtouuidimproves consistency with the new identifier schema.
136-139: LGTM!The method correctly updates the expiration time of the refresh token.
141-142: LGTM!The method correctly retrieves the UUID based on the provided key and namespace.
145-147: LGTM!The method correctly stores the refresh token associated with a key and expiration time in the storage.
159-161: LGTM!The change from
uidtouuidimproves consistency with the new identifier schema.
176-177: LGTM!The change from
uidtouuidimproves consistency with the new identifier schema.
183-184: **LGTM!test/units/jwt_sessions/test_session.rb (5)
94-94: LGTM! Variableuidrenamed touuid.The variable renaming is consistent and correctly used.
107-107: LGTM! Variableuidrenamed touuid.The variable renaming is consistent and correctly used.
121-121: LGTM! Variableuidrenamed touuid.The variable renaming is consistent and correctly used.
134-139: LGTM! Function and variable names updated.The function name and variable renaming are consistent and correctly used.
338-345: New functionupdate_refresh_expiryadded.The function correctly updates the expiry of a refresh token and asserts that an unauthorized error is raised when attempting to find a refresh token with an outdated UID.
| expiration: Time.now.to_i + 3600 | ||
| ) | ||
| end | ||
|
|
There was a problem hiding this comment.
Update other methods to use access_uuid.
The other methods still use access_uid. Ensure that all instances of access_uid are updated to access_uuid for consistency.
- assert_equal new_access_uid, token.access_uid
+ assert_equal new_access_uid, token.access_uuid
- assert_equal found_token.access_uid, token.access_uid
+ assert_equal found_token.access_uuid, token.access_uuid
- assert_equal [token.token, token_2.token].sort, JWTSessions::RefreshToken.all(nil, JWTSessions.token_store).map(&:token).sort
+ assert_equal [token.token, token_2.token].sort, JWTSessions::RefreshToken.all(nil, JWTSessions.token_store).map(&:token).sortCommittable suggestion was skipped due to low confidence.
| store.persist_access("uuid", "csrf", Time.now.to_i + 3600) | ||
| assert_equal({ csrf: "csrf" }, store.fetch_access("uuid")) |
There was a problem hiding this comment.
Update other methods to use uuid.
The other methods still use uid. Ensure that all instances of uid are updated to uuid for consistency.
- store.persist_access("uid", "csrf", Time.now.to_i + 3600)
+ store.persist_access("uuid", "csrf", Time.now.to_i + 3600)
- store.persist_access("uid", "csrf", Time.now.to_i - 3600)
+ store.persist_access("uuid", "csrf", Time.now.to_i - 3600)
- store.persist_refresh(
- uid: "uid",
+ store.persist_refresh(
+ uuid: "uuid",
- refresh = store.fetch_refresh("uid", "")
+ refresh = store.fetch_refresh("uuid", "")Also applies to: 23-24, 42-44, 49-49
Fix: Changed redis method from keys to scan.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
lib/jwt_sessions/store_adapters/redis_store_adapter.rb (4)
34-40:first_matchparameter onfetch_refreshis unused
fetch_refresh(uuid, namespace, first_match = false)ignoresfirst_match, while the memory adapter implements cross-namespace behavior whenfirst_matchis true.If the Redis adapter is intentionally not supporting that mode, consider renaming the parameter to
_first_match(or similar) to make it explicit and avoid confusion. If the behavior is expected to match the memory adapter, this method would need a corresponding implementation.
42-68: Refresh persistence/update flow is coherent; consider taming parameter list
persist_refresh,update_refresh, andupdate_refresh_expirywork together coherently: they usefull_refresh_key, write the same fields as defined inREFRESH_KEYS, and consistently rely onexpireatwith an absoluteexpirationtimestamp, which matches the rest of the codebase.The 6-argument keyword signature on
persist_refreshis what RuboCop flagged. If that becomes painful to maintain, consider wrapping the refresh attributes into a small value object or a single options hash, but this is not blocking.
97-118: Redis cluster/password configuration: confirm expectations and consider non-cluster password useThe new
configure_redis_clientlogic looks sane overall:
- It guards against mixing
redis_urlwith host/port/db components.- It reuses
build_redis_urlwhenredis_urlis not provided.- It switches between
{ cluster: redis_url, password: redis_password, driver: :hiredis }and{ url: redis_url, driver: :hiredis }based onredis_cluster.Two points to double‑check:
Password usage in non-cluster mode
redis_passwordis only applied whenredis_clusteris true; non-cluster setups must embed the password inredis_urlorREDIS_URL. If consumers expectredis_passwordto work in both modes, you might want to also include it in the non-cluster config.Cluster URL shape
Ensure upstream code passesredis_urlin the form expected byRedis.new(cluster: ...)for cluster mode (e.g., array of node URLs vs single URL), to avoid surprises at runtime.Both are behavioral questions rather than correctness issues in this diff, but worth verifying against how the gem is documented and used.
131-138: Key patterns, SCAN, and helper methods are generally solid; minor nits only
full_refresh_key,refresh_key, anduuid_from_keyare internally consistent and compatible with the patterns used byscan_keysandall_refresh_tokens.scan_keysimplements a proper SCAN loop and supports the “find-first” case viacount: 1, which keepsfirst_refresh_keyefficient.Minor, non-blocking observations:
- The rescue block in
scan_keyslogs “falling back to direct key lookup,” but the current implementation simply returns[]. If you no longer fall back toKEYS, consider updating the message to match behavior.- If namespaces or prefixes can contain underscores,
uuid_from_key’skey.split("_").lastremains safe as long as UUIDs never include underscores (which is a reasonable assumption).These are all minor polish points; the core logic looks good.
Also applies to: 140-159, 161-171
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/jwt_sessions/store_adapters/redis_store_adapter.rb(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
lib/jwt_sessions/store_adapters/redis_store_adapter.rb (4)
lib/jwt_sessions/store_adapters/memory_store_adapter.rb (10)
fetch_access(15-18)persist_access(20-23)fetch_refresh(25-35)persist_refresh(37-46)update_refresh(48-56)update_refresh_expiry(58-60)all_refresh_tokens(62-68)destroy_refresh(70-72)destroy_access(74-76)refresh_uuid(78-80)lib/jwt_sessions/store_adapters/abstract_store_adapter.rb (9)
fetch_access(5-38)fetch_access(6-8)persist_access(10-12)fetch_refresh(15-17)persist_refresh(19-21)update_refresh(23-25)all_refresh_tokens(27-29)destroy_refresh(31-33)destroy_access(35-37)lib/jwt_sessions.rb (1)
access_expiration(131-133)lib/jwt_sessions/access_token.rb (2)
refresh_uuid(19-21)refresh_uuid(23-25)
🪛 RuboCop (1.81.7)
lib/jwt_sessions/store_adapters/redis_store_adapter.rb
[convention] 42-42: Avoid parameter lists longer than 5 parameters. [6/5]
(Metrics/ParameterLists)
[convention] 97-97: Avoid parameter lists longer than 5 parameters. [6/5]
(Metrics/ParameterLists)
🔇 Additional comments (1)
lib/jwt_sessions/store_adapters/redis_store_adapter.rb (1)
8-32: UUID migration and access token storage look consistentThe switch to
access_uuidinREFRESH_KEYSand thefetch_access/persist_access/access_keyhelpers is internally consistent and matches the UUID-based interface used by the abstract and memory adapters. Expiration withexpireatis aligned with the rest of the library’s timestamp-based expirations.
| def all_refresh_tokens(namespace) | ||
| keys_in_namespace = storage.keys(refresh_key("*", namespace)) | ||
| keys_in_namespace = scan_keys(refresh_key("*", namespace)) | ||
| (keys_in_namespace || []).each_with_object({}) do |key, acc| | ||
| uid = uid_from_key(key) | ||
| acc[uid] = fetch_refresh(uid, namespace) | ||
| uuid = uuid_from_key(key) | ||
| acc[uuid] = fetch_refresh(uuid, namespace) | ||
| end |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
jwt_sessions ruby redis store adapter all_refresh_tokens namespace
💡 Result:
Short answer — yes. Configure the gem to use Redis as the token store, pass a prefix/redis URL in JWTSessions.token_store, and use Session.new(namespace: '...') to group (namespace) sessions so refresh tokens for that namespace are stored/queried together.
Example (config/initializers/jwt_sessions.rb):
JWTSessions.token_store = :redis, {
redis_url: ENV.fetch("REDIS_URL", "redis://127.0.0.1:6379/0"),
token_prefix: "jwt_" # optional key prefix
}
create/session ops with namespace
session = JWTSessions::Session.new(payload: {user_id: user.id}, namespace: "user:#{user.id}")
tokens = session.login
Useful APIs:
- Session.new(namespace: "...") groups sessions; RefreshToken.all(namespace, store) lists refresh tokens for that namespace. Use session.flush_namespaced / session.flush_namespaced_access_tokens to remove tokens by namespace. [1][2][3]
Sources:
[1] jwt_sessions README (token_store, token_prefix, namespace, Session API).
[2] jwt_sessions (rubydoc) — JWTSessions.token_store and StoreAdapters.
[3] JWTSessions::Session docs — refresh/flush and RefreshToken.all usage.
🏁 Script executed:
find . -type f -name "*redis*adapter*" | head -20Repository: urkishan/jwt_sessions
Length of output: 190
🏁 Script executed:
find . -type f -name "*store*adapter*" | head -20Repository: urkishan/jwt_sessions
Length of output: 415
🏁 Script executed:
git ls-files | grep -i adapter | head -20Repository: urkishan/jwt_sessions
Length of output: 403
🏁 Script executed:
cat -n lib/jwt_sessions/store_adapters/redis_store_adapter.rb 2>/dev/null | head -100Repository: urkishan/jwt_sessions
Length of output: 4112
🏁 Script executed:
ls -la lib/jwt_sessions/store_adapters/ 2>/dev/null || echo "Path not found, searching..."
find . -path "*store_adapters*" -type f 2>/dev/nullRepository: urkishan/jwt_sessions
Length of output: 747
🏁 Script executed:
git ls-files | grep store_adapterRepository: urkishan/jwt_sessions
Length of output: 403
🏁 Script executed:
git ls-files | grep -E "redis|adapter|store"Repository: urkishan/jwt_sessions
Length of output: 434
🏁 Script executed:
pwd && git ls-files | head -30Repository: urkishan/jwt_sessions
Length of output: 1033
🏁 Script executed:
find . -type f -name "*.rb" | grep -i redis | head -10Repository: urkishan/jwt_sessions
Length of output: 190
🏁 Script executed:
find . -type f -name "*.rb" | head -50Repository: urkishan/jwt_sessions
Length of output: 2690
🏁 Script executed:
find . -type d -name "jwt_sessions" 2>/dev/nullRepository: urkishan/jwt_sessions
Length of output: 109
🏁 Script executed:
ls -la && echo "---" && find . -maxdepth 3 -type d | head -30Repository: urkishan/jwt_sessions
Length of output: 1349
🏁 Script executed:
find . -name "*redis_store*" -o -name "*memory_store*" 2>/dev/null | head -20Repository: urkishan/jwt_sessions
Length of output: 318
🏁 Script executed:
ls -laR | grep -E "(redis|adapter)" | head -30Repository: urkishan/jwt_sessions
Length of output: 712
🏁 Script executed:
find . -type f \( -name "*.rb" -o -name "*.js" -o -name "*.py" \) 2>/dev/null | wc -lRepository: urkishan/jwt_sessions
Length of output: 67
🏁 Script executed:
find . -type f \( -name "*.rb" -o -name "*.js" -o -name "*.py" \) 2>/dev/null | head -100Repository: urkishan/jwt_sessions
Length of output: 3318
🏁 Script executed:
rg "def all_refresh_tokens" -A 10 2>/dev/nullRepository: urkishan/jwt_sessions
Length of output: 2957
🏁 Script executed:
rg "all_refresh_tokens" 2>/dev/nullRepository: urkishan/jwt_sessions
Length of output: 659
🏁 Script executed:
rg "refresh_key" 2>/dev/nullRepository: urkishan/jwt_sessions
Length of output: 1502
🏁 Script executed:
rg "MemoryStoreAdapter" 2>/dev/nullRepository: urkishan/jwt_sessions
Length of output: 561
🏁 Script executed:
rg "RedisStoreAdapter" 2>/dev/nullRepository: urkishan/jwt_sessions
Length of output: 1985
🏁 Script executed:
git log --oneline -10 2>/dev/null || echo "Not a git repo or no history"Repository: urkishan/jwt_sessions
Length of output: 128
🏁 Script executed:
file . && stat . | head -5Repository: urkishan/jwt_sessions
Length of output: 109
🏁 Script executed:
echo "Current directory:" && pwd && echo "Contents:" && ls -laRepository: urkishan/jwt_sessions
Length of output: 1012
🏁 Script executed:
cat lib/jwt_sessions/store_adapters/redis_store_adapter.rb 2>&1Repository: urkishan/jwt_sessions
Length of output: 5604
🏁 Script executed:
test -f "lib/jwt_sessions/store_adapters/redis_store_adapter.rb" && echo "File exists" || echo "File not found"Repository: urkishan/jwt_sessions
Length of output: 76
🏁 Script executed:
ls lib/ 2>&1 || ls . 2>&1 | head -20Repository: urkishan/jwt_sessions
Length of output: 93
🏁 Script executed:
cat -n lib/jwt_sessions/store_adapters/redis_store_adapter.rb | sed -n '139,157p'Repository: urkishan/jwt_sessions
Length of output: 825
🏁 Script executed:
cat -n lib/jwt_sessions/store_adapters/memory_store_adapter.rbRepository: urkishan/jwt_sessions
Length of output: 4283
🏁 Script executed:
cat -n lib/jwt_sessions/store_adapters/redis_store_adapter.rb | sed -n '1,180p'Repository: urkishan/jwt_sessions
Length of output: 6857
🏁 Script executed:
cat -n test/units/jwt_sessions/store_adapters/test_redis_store_adapter.rbRepository: urkishan/jwt_sessions
Length of output: 3285
🏁 Script executed:
cat -n test/units/jwt_sessions/store_adapters/test_memory_store_adapter.rbRepository: urkishan/jwt_sessions
Length of output: 3841
🏁 Script executed:
rg "all_refresh_tokens" -B2 -A8 test/Repository: urkishan/jwt_sessions
Length of output: 2100
🏁 Script executed:
rg "all_refresh_tokens.*nil" test/Repository: urkishan/jwt_sessions
Length of output: 47
🏁 Script executed:
cat -n lib/jwt_sessions/refresh_token.rbRepository: urkishan/jwt_sessions
Length of output: 4019
🏁 Script executed:
python3 << 'EOF'
# Simulate the behavior
def full_refresh_key(prefix, uuid, namespace):
return f"{prefix}_{namespace}_refresh_{uuid}"
def refresh_key(uuid, namespace):
if not namespace or namespace == "":
namespace = "*"
return full_refresh_key("jwt", uuid, namespace)
# Simulating scan results when calling refresh_key("*", None)
print("SCAN pattern when namespace=None:")
pattern = refresh_key("*", None)
print(f" Pattern: {pattern}")
print(f" Would match keys like: jwt_ns1_refresh_uuid1, jwt_ns2_refresh_uuid2")
# Simulating fetch when called with original namespace=None
print("\nFETCH key when namespace=None (passed to fetch_refresh):")
fetch_pattern = full_refresh_key("jwt", "uuid1", None)
print(f" Key: {fetch_pattern}")
print(f" Expects: jwt__refresh_uuid1 (empty namespace)")
print("\nMismatch: SCAN found 'jwt_ns1_refresh_uuid1' but FETCH looks for 'jwt__refresh_uuid1'")
EOFRepository: urkishan/jwt_sessions
Length of output: 151
🏁 Script executed:
python3 << 'EOF'
# The key format is: prefix_namespace_refresh_uuid
# To extract namespace, we'd need to parse it differently
key_example1 = "jwt_ns1_refresh_uuid123"
key_example2 = "jwt_ns2_refresh_uuid456"
key_example3 = "jwt__refresh_uuid789" # empty namespace
# Current implementation (wrong approach):
def uuid_from_key_current(key):
return key.split("_").last # last part after splitting
# Would need something like:
# 1. Extract parts: jwt, namespace, refresh, uuid
# 2. Or store namespace as part of the hash
# 3. Or derive namespace from the pattern match
print("Current implementation extracts only UUID from the key")
print("It doesn't know which namespace each key belongs to")
print("This is why the review comment suggests deriving namespace from key")
EOFRepository: urkishan/jwt_sessions
Length of output: 151
🏁 Script executed:
grep -n "namespace.nil?" lib/jwt_sessions/store_adapters/memory_store_adapter.rb -B2 -A5Repository: urkishan/jwt_sessions
Length of output: 351
all_refresh_tokens(nil) may not return tokens from all namespaces
When namespace is nil, the scan pattern via refresh_key("*", nil) converts to refresh_key("*", "*"), producing pattern prefix_*_refresh_* that correctly scans keys across all namespaces. However, fetch_refresh(uuid, namespace) is then called with the original namespace parameter (nil), not the namespace extracted from each scanned key.
This causes a mismatch: keys like prefix_ns1_refresh_uuid1 are scanned, but fetch_refresh constructs the key as prefix__refresh_uuid1 (with empty namespace from nil), which doesn't exist. All fetches return {}, so all_refresh_tokens(nil) returns an empty hash instead of tokens from all namespaces. The MemoryStoreAdapter correctly handles this by explicitly iterating through all namespaces when namespace is nil.
Extract the namespace from each scanned key (parsing prefix_namespace_refresh_uuid) rather than reusing the inbound parameter, or skip entries where fetch_refresh returns {}.
🤖 Prompt for AI Agents
In lib/jwt_sessions/store_adapters/redis_store_adapter.rb around lines 70 to 75,
all_refresh_tokens(namespace) scans keys across namespaces when namespace is nil
but then calls fetch_refresh(uuid, namespace) using the original nil namespace,
causing lookups to fail; change the logic to parse each scanned key to extract
the namespace and uuid (e.g. split the key by the known prefix/segments to get
namespace and uuid) and call fetch_refresh(uuid, extracted_namespace) for each
entry, skipping entries that still return empty results; ensure the parsing
handles the expected key format and nil namespace case consistently.
| def refresh_uuid(key) | ||
| storage.get(jti_key(key)) | ||
| end | ||
|
|
||
| def set_refresh_uuid(key, uuid, expiration) | ||
| storage.set(jti_key(key), uuid, ex: expiration) | ||
| end |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files | grep -i redisRepository: urkishan/jwt_sessions
Length of output: 186
🏁 Script executed:
find . -name "*redis*" -type fRepository: urkishan/jwt_sessions
Length of output: 190
🏁 Script executed:
cat -n lib/jwt_sessions/store_adapters/redis_store_adapter.rbRepository: urkishan/jwt_sessions
Length of output: 6857
🏁 Script executed:
rg -n "storage\.set.*ex:" lib/jwt_sessions/store_adapters/redis_store_adapter.rbRepository: urkishan/jwt_sessions
Length of output: 123
🏁 Script executed:
rg -n "\.set\(" lib/jwt_sessions/store_adapters/redis_store_adapter.rbRepository: urkishan/jwt_sessions
Length of output: 157
🏁 Script executed:
rg -n "ex:" lib/jwt_sessions/Repository: urkishan/jwt_sessions
Length of output: 178
🏁 Script executed:
rg -n "set_refresh_uuid" lib/jwt_sessions/Repository: urkishan/jwt_sessions
Length of output: 552
🏁 Script executed:
cat -n test/units/jwt_sessions/store_adapters/test_redis_store_adapter.rb | head -200Repository: urkishan/jwt_sessions
Length of output: 3285
🏁 Script executed:
sed -n '30,50p' lib/jwt_sessions/session.rbRepository: urkishan/jwt_sessions
Length of output: 569
🏁 Script executed:
sed -n '140,150p' lib/jwt_sessions/session.rbRepository: urkishan/jwt_sessions
Length of output: 254
🏁 Script executed:
sed -n '75,90p' lib/jwt_sessions/store_adapters/memory_store_adapter.rbRepository: urkishan/jwt_sessions
Length of output: 658
🏁 Script executed:
rg -n "def create_refresh_token|def login" lib/jwt_sessions/session.rb -A 5Repository: urkishan/jwt_sessions
Length of output: 378
🏁 Script executed:
rg -n "renew_tokens\|reset_refresh_uuid" lib/jwt_sessions/ -B 2 -A 2 | head -60Repository: urkishan/jwt_sessions
Length of output: 47
🏁 Script executed:
find . -name "*.rb" -type f | xargs grep -l "class RefreshToken"Repository: urkishan/jwt_sessions
Length of output: 100
🏁 Script executed:
rg -n "class RefreshToken" lib/jwt_sessions/ -A 20Repository: urkishan/jwt_sessions
Length of output: 1780
🏁 Script executed:
rg -n "reset_refresh_uuid" lib/jwt_sessions/ -B 3 -A 1Repository: urkishan/jwt_sessions
Length of output: 673
🏁 Script executed:
rg -n "refresh_expiration" lib/jwt_sessions/ -B 2 -A 2Repository: urkishan/jwt_sessions
Length of output: 1655
🏁 Script executed:
rg -n "def refresh_expiration" lib/jwt_sessions/Repository: urkishan/jwt_sessions
Length of output: 47
🏁 Script executed:
cat lib/jwt_sessions.rbRepository: urkishan/jwt_sessions
Length of output: 4404
set_refresh_uuid uses TTL semantics inconsistent with the rest of the adapter
Line 92 calls storage.set(jti_key(key), uuid, ex: expiration) where the ex: parameter expects a TTL in seconds. However, everywhere else in this adapter (persist_access, persist_refresh, update_refresh_expiry), the expiration parameter is an absolute Unix timestamp passed to expireat. Since callers always pass absolute timestamps (computed as Time.now.to_i + offset), the JTI→UUID mapping will live for decades instead of the intended expiration time.
Align with the rest of the adapter:
storage.set(jti_key(key), uuid)
storage.expireat(jti_key(key), expiration)🤖 Prompt for AI Agents
In lib/jwt_sessions/store_adapters/redis_store_adapter.rb around lines 87 to 93,
set_refresh_uuid currently calls storage.set with ex: expiration (treating
expiration as TTL seconds) while the rest of the adapter uses absolute Unix
timestamps and expireat; change set_refresh_uuid to set the key without the ex
option and then call storage.expireat(jti_key(key), expiration) so the stored
JTI→UUID mapping uses the same absolute-expiry semantics as
persist_access/persist_refresh/update_refresh_expiry.
2 participants
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Documentation
uuidterminology and token management changes.✏️ Tip: You can customize this high-level summary in your review settings.