tweak rate limiting and error handling in auth0 user sync. #329
Conversation
Added warnings about API call frequency and potential rate limit issues. Implemented pagination control and detailed logging for error tracking during user processing.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRefactors Auth0 user sync to use a per-run SyncContext with thread-safe JWKS caching, per-user retry/backoff for role fetches, pagination rate-limit handling, granular logging, DB pre/post statistics, and admin ownership assignment; public sync_users_to_db signature preserved but now returns None. Changes
sequenceDiagram
participant Sync as SyncContext
participant Auth0 as Auth0 API
participant DB as Database
participant Log as Logger
Sync->>Sync: init context, counters, capture DB stats
loop pages
Sync->>Auth0: list users (page, per_page)
alt 429 RateLimit
Auth0-->>Sync: RateLimitError
Sync->>Sync: increment pagination_retry, backoff sleep
alt retries exceeded
Sync->>Log: log pagination error_info
Sync-->>Sync: raise RuntimeError
else
Sync->>Auth0: retry list users
end
else users returned
Auth0-->>Sync: user_list
loop each user
Sync->>Auth0: fetch user roles (with retry/backoff)
alt role fetch 429
Auth0-->>Sync: RateLimitError
Sync->>Sync: record failure or retry per-user
else roles returned
Auth0-->>Sync: roles
Sync->>DB: add_or_update_user
Sync->>Sync: update new/updated/admin counters
alt admin transitions
Sync->>DB: assign/remove project ownership
end
end
Sync->>Sync: sleep ~200ms between role requests
Sync->>Log: periodic progress logging
end
Sync->>Sync: determine end-of-pagination
end
end
Sync->>Sync: capture final DB stats
Sync->>Log: log final summary and deltas
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
…l database user counts, along with counts of new, updated, and admin users during synchronization. Improved progress logging for better monitoring of user processing.
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
python/mdvtools/auth/auth0_provider.py (1)
479-485: 🛠️ Refactor suggestion | 🟠 MajorClosure over loop variable
auth0_idin retry decorator — latent security risk.The
get_user_rolesclosure capturesauth0_idfrom the enclosingforloop. Currently this is safe because retries are synchronous, but this pattern is fragile: any future move to deferred/async retries would cause the wrong user's roles to be fetched, leading to privilege escalation or denial.A safer approach is to pass
auth0_idas a parameter:♻️ Suggested refactor
- try: - # Fetch user's roles with retry mechanism - `@retry_with_exponential_backoff` - def get_user_roles(): - return auth0.users.list_roles(auth0_id) - - roles = get_user_roles() + try: + `@retry_with_exponential_backoff` + def get_user_roles(uid): + return auth0.users.list_roles(uid) + + roles = get_user_roles(auth0_id)This also avoids re-creating the decorated function on each iteration — consider defining it once before the loop.
Based on learnings: "Closure issues in authentication/authorization systems that capture loop variables in retry decorators represent critical security vulnerabilities. When retries delay execution and loop variables change, users can receive wrong permissions, leading to privilege escalation or access denial."
🤖 Fix all issues with AI agents
In `@python/mdvtools/auth/auth0_provider.py`:
- Around line 505-528: The RateLimitError handler inside the user-processing
loop (the except RateLimitError as e block that wraps calls to list_roles)
should not re-raise because that bubbles out to the outer pagination handler;
instead log the detailed error, increment the per-user rate-limit metric (not
pagination_rate_limit_count), and use continue to skip this user so
processed_users and pagination behavior remain correct; update the logging
message in that block to note it's a per-user rate limit and ensure attributes
from the RateLimitError are included as before.
- Around line 439-451: The Auth0 API call using auth0.users.list is returning a
list when include_totals is not set, so calling users.get(...) will raise
AttributeError; update the auth0.users.list invocation (the call referenced in
the users variable) to pass include_totals=True so users is a dict and
users.get('users', []) works, and then you can safely access users['total'] for
more robust pagination handling (adjust any pagination logic that expects
users['total'] accordingly).
🧹 Nitpick comments (1)
python/mdvtools/auth/auth0_provider.py (1)
507-519: Duplicatederror_infoconstruction could be extracted into a helper.The error-info dict assembly appears twice (lines 507–519 and 540–553) with near-identical
getattrcalls. A small helper like_build_rate_limit_info(e, **extra)would reduce duplication and ensure consistent fields across both handlers.Also applies to: 540-553
|
@coderabbitai a bit more of a review/audit on this file would be useful. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
python/mdvtools/auth/auth0_provider.py (1)
500-506: 🛠️ Refactor suggestion | 🟠 MajorClosure captures loop variable
auth0_idinside retry decorator — fragile pattern.While currently safe (synchronous execution blocks until retries complete), this pattern is fragile: if execution becomes async or the call is deferred, the captured
auth0_idcould change between retries, leading to role lookups for the wrong user — a privilege escalation risk.Pass
auth0_idas a parameter to decouple the closure from the loop variable:Proposed fix
- `@retry_with_exponential_backoff` - def get_user_roles(): - return auth0.users.list_roles(auth0_id) - - roles = get_user_roles() + `@retry_with_exponential_backoff` + def get_user_roles(uid): + return auth0.users.list_roles(uid) + + roles = get_user_roles(auth0_id)Based on learnings: "Closure issues in authentication/authorization systems that capture loop variables in retry decorators represent critical security vulnerabilities. When retries delay execution and loop variables change, users can receive wrong permissions, leading to privilege escalation or access denial."
🤖 Fix all issues with AI agents
In `@python/mdvtools/auth/auth0_provider.py`:
- Around line 478-495: Remove the redundant pre-query
User.query.filter_by(auth_id=...) and instead have
UserService.add_or_update_user return a definitive result indicating whether the
call created or updated a user (e.g., a tuple or object with created: bool and
the db_user). Use that returned created flag to increment new_users or
updated_users so the existence check and actual upsert logic are consistent
(ensure add_or_update_user resolves by the same identity criteria you
need—either email or auth_id—or accept both and reconcile them inside the
service). Also align counter semantics: only increment
processed_users/new_users/updated_users in the same success path (after role
processing succeeds) or else increment all three in a consistent place so failed
role handling doesn't leave totals skewed. Reference: add_or_update_user in
UserService, the local new_users/updated_users/processed_users counters, and the
previous User.query.filter_by(auth_id=...) call to remove.
🧹 Nitpick comments (1)
python/mdvtools/auth/auth0_provider.py (1)
510-515:admin_users_syncedonly counts new admin transitions, not all admins.The counter increments only when
is_admin and not was_admin(line 514), so it tracks users promoted to admin during this sync, not total admin users processed. The final log label"admins synced"(line 612) may mislead operators into thinking it's the total. Consider renaming toadmin_promotionsornew_admins, or alternatively count all users whereis_adminis true.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain
🏁 Script executed: Length of output: 28447
🔴 Critical Issues1. Race Condition in JWKS Cache (Lines 331-347)The global Recommendation: Use thread-safe caching (e.g., 2. Dual Retry Mechanisms (Lines 24-45, 639-672)The code has two separate retry implementations with different parameters:
This inconsistency is confusing and error-prone. The nested usage (decorator on Recommendation: Standardize on one retry mechanism with configurable parameters, or clearly document why different strategies are needed. 3. Database Commit Without Error Handling (Line 588)db.session.commit()This could fail due to database errors, connection issues, or constraint violations, but there's no try-except around it. Recommendation: Wrap in try-except with proper rollback logic. 🟡 High Priority Issues4. Debug Print Statement (Line 120)print(redirect_uri)This should be removed or converted to logging for production code. 5. Long Method - sync_users_to_db (206 lines)This method handles pagination, rate limiting, user processing, role fetching, admin assignment, and statistics - violating Single Responsibility Principle. Recommendation: Extract into smaller functions:
6. Magic Numbers and Hardcoded Values
Recommendation: Extract as named constants at module level with documentation explaining the rationale. 7. Duplicate Error Info Collection (Lines 629-645, 648-665)Nearly identical code constructs Recommendation: Create a helper function: def _collect_rate_limit_error_info(e: RateLimitError, **context) -> dict:
"""Extract and format rate limit error information."""
error_info = {
'status': getattr(e, 'status_code', None),
'code': getattr(e, 'error_code', None),
'message': getattr(e, 'message', str(e)),
'reset_at': getattr(e, 'reset_at', None),
'remaining': getattr(e, 'remaining', None),
'limit': getattr(e, 'limit', None),
**context
}
return {k: v for k, v in error_info.items() if v is not None}8. Insufficient Delay Between Requests (Line 576)time.sleep(0.2) # 200ms delay between requestsThe comment on Line 510 mentions "typically 2 req/sec for free tier" which means minimum 0.5s between requests. The 0.2s delay might still trigger rate limits. Recommendation: Use at least 0.6s delay or make it configurable based on Auth0 tier. 🟠 Medium Priority Issues9. Missing functools.wraps in Decorator (Line 24)def retry_with_exponential_backoff(func):
def wrapper(*args, **kwargs):This doesn't preserve function metadata ( Recommendation: from functools import wraps
def retry_with_exponential_backoff(func):
`@wraps`(func)
def wrapper(*args, **kwargs):10. N+1 Query Problem (Lines 594-598)For each admin user, the code queries all projects inside a loop: for project in Project.query.all():If there are many admin users, this repeatedly fetches all projects. Recommendation: Query projects once before the user loop and reuse the list. 11. Incomplete State on Partial SyncIf Recommendation: Consider using database transactions or implementing idempotent sync with tracking. 12. Potential Double-Processing RiskWhen a RateLimitError occurs during pagination, the page counter doesn't increment (correct), but there's no guarantee that retrying the same page returns identical results if users were added/modified in Auth0 during the delay. Recommendation: Add logging to detect if user sets differ between retries. 13. No Validation of Required Config (Line 542)auth0_db_connection = self.app.config['AUTH0_DB_CONNECTION']If this config is missing or empty, the query will fail or return unexpected results. Recommendation: Add validation in 🟢 Low Priority / Style Issues14. Inconsistent Type HintsSome methods have complete type hints (e.g., Recommendation: Add complete type hints throughout for better IDE support and type checking. 15. Commented Import (Line 6)# from flask import ResponseShould be removed if not needed. 16. Convoluted Control Flow (Lines 598-608)The pagination end detection uses Recommendation: Use 17. Inconsistent Error Return Types
Recommendation: Standardize error handling approach across the class. 18. Session Modified Flag (Line 458)session.modified = TrueThis is called after setting session data, but Flask typically auto-detects modifications. Is this necessary? 📊 Performance Considerations19. Sequential Processing at ScaleFor large user bases (1000+ users), the sequential processing with delays means:
Recommendation: Consider batch role fetching if Auth0 API supports it, or implement async processing. 20. Redundant Database Query (Line 551)existing_user = User.query.filter_by(auth_id=auth0_id).first()Then Recommendation: Pass ✅ Positive Observations
🎯 Immediate Action Items
Would you like me to create issues for any of these findings, or would you prefer I help implement fixes for specific items? |
…retrying. add some other comments on things that need attention.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
python/mdvtools/auth/auth0_provider.py (1)
521-528: 🛠️ Refactor suggestion | 🟠 Major
Project.query.all()is called inside the user loop — cache it once.Every admin user triggers a full table scan of projects. Since the project list doesn't change during sync, fetch it once before the loop.
Proposed fix
+ all_projects = Project.query.all() + while has_more_users: ... if is_admin: - for project in Project.query.all(): + for project in all_projects: UserProjectService.add_or_update_user_project(
🤖 Fix all issues with AI agents
In `@python/mdvtools/auth/auth0_provider.py`:
- Line 500: The 200ms sleep between requests (time.sleep(0.2)) is too short for
Auth0 free-tier limits; change it to a safer default (e.g. 0.5–0.6s) and make it
configurable rather than hard-coded. Update the place where time.sleep(0.2) is
used (in the Auth0Provider request loop) to use a configurable attribute or
parameter (e.g., self.request_delay or REQUEST_DELAY_DEFAULT) and read that
value when sleeping, and add an init/config option to set the delay so callers
can override the default.
- Around line 514-516: Wrap the db.session.commit() call (around where
db_user.is_admin = is_admin is set) in a try/except that catches database
errors, calls db.session.rollback() on failure, logs or re-raises the error, and
ensures the loop can continue or abort appropriately; specifically, in the
function/method containing db_user and the loop, replace the bare commit with a
guarded commit: try: db.session.commit() except Exception as e:
db.session.rollback(); handle/log e (or raise a custom error) so the session
state is reset before the next iteration.
- Around line 274-275: The JWKS globals (_jwks_cache and _jwks_cache_expiry) are
not synchronized and can race during the expiry check and update and while being
iterated; add a threading.Lock (e.g., _jwks_cache_lock) and acquire it around
all reads, writes and any iteration of _jwks_cache in functions like the JWKS
fetcher (the code paths around the expiry check and the cache update and the
place that iterates the cache), or alternatively move these into Auth0Provider
instance attributes and protect them with an instance-level lock; ensure you
lock before checking expiry, before updating the cache, and while iterating to
prevent concurrent mutation/crash.
🧹 Nitpick comments (5)
python/mdvtools/auth/auth0_provider.py (5)
28-51: Missing@functools.wraps(func)on the wrapper function.Without
@wraps, the decorated function loses its__name__,__doc__, and other metadata, which hampers debugging and introspection (e.g., stack traces will showwrapperinstead of the actual function name).Proposed fix
+import functools + def retry_with_exponential_backoff(func): """ Decorator to implement retry logic with exponential backoff.""" + `@functools.wraps`(func) def wrapper(*args, **kwargs):
502-509: Moveget_user_rolesdefinition outside the loop.The decorated function is redefined (and re-decorated) on every iteration. Since it already takes
idas a parameter (correctly avoiding the closure-capture bug from PR#229), it can be defined once before the loop.Proposed fix — define once before the while loop
+ `@retry_with_exponential_backoff` + def get_user_roles(user_id): + return auth0.users.list_roles(user_id) + while has_more_users: try: ... for user in user_list: ... try: - `@retry_with_exponential_backoff` - def get_user_roles(id): - # this is the real place where a RateLimitError may be raised - return auth0.users.list_roles(id) - - roles = get_user_roles(auth0_id) + roles = get_user_roles(auth0_id)Based on learnings: "Closure issues in authentication/authorization systems that capture loop variables in retry decorators represent critical security vulnerabilities." The current parameterized approach is correct for safety; moving it outside the loop preserves that safety while avoiding redundant decoration.
538-551: Duplicate error-info construction — extract a helper.The error_info dict (lines 538-551) is nearly identical to lines 571-585. Extract a small helper to avoid repetition and ensure consistent fields.
Example helper
def _rate_limit_error_info(e, **extra): info = { 'status': getattr(e, 'status_code', None), 'code': getattr(e, 'error_code', None), 'message': getattr(e, 'message', str(e)), 'reset_at': getattr(e, 'reset_at', None), 'remaining': getattr(e, 'remaining', None), 'limit': getattr(e, 'limit', None), **extra, } return {k: v for k, v in info.items() if v is not None}
464-467: Usebreakinstead ofcontinuewhen pagination is done.When
user_listis empty,has_more_usersis set toFalseand thencontinueloops back to check the condition. Abreakis more direct and communicates intent clearly.Proposed fix
if not user_list: logging.info(f"Reached end of pagination at page {page} (empty user list returned)") - has_more_users = False - continue # Skip processing, exit loop on next iteration + break
439-447: Consider extracting pagination/retry magic numbers to module-level constants.
per_page,max_pagination_rate_limit_retries, andpagination_retry_delayare hardcoded. For consistency with the existingMAX_RETRIES,BASE_DELAY,MAX_DELAYconstants at the top of the file, these should also be module-level constants (and ideally configurable).
…chanism. add tests.
…ance and reliability during token validation. Introduced double-checked locking pattern for cache access, ensuring safe reads and writes in a multi-threaded environment.
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@python/mdvtools/auth/auth0_provider.py`:
- Around line 595-602: The backoff logic in _handle_pagination_rate_limit logs
"Retrying in {context.pagination_retry_delay}s" but then immediately doubles
context.pagination_retry_delay before the caller sleeps, causing the logged
delay to be inaccurate and the first delay value to be skipped; fix by
reordering so you perform the sleep using the current
context.pagination_retry_delay, then after sleeping update
context.pagination_retry_delay = min(context.pagination_retry_delay * 2, 60.0)
for the next retry. Update both _handle_pagination_rate_limit and the
corresponding retry point in sync_users_to_db (the caller that sleeps using
context.pagination_retry_delay) so the log, sleep, and backoff update occur in
the correct sequence.
- Around line 536-543: The loop that assigns projects to admins using
UserProjectService.add_or_update_user_project can raise and abort the entire
sync via _process_single_user; wrap the project-assignment loop (iterating
context.all_projects) in a try/except that catches exceptions from
add_or_update_user_project, logs the error (including user id and project id)
and continues so one failing project or user doesn’t stop the pagination;
optionally handle per-project errors inside the loop to continue other projects,
or catch around the whole loop to skip this user and proceed with remaining
users.
- Around line 690-703: Wrap the call to _process_single_user(user, sync_context)
inside a per-user try/except so a single bad user won’t abort the whole sync:
try calling _process_single_user, except RateLimitError: re-raise so rate
limiting is still handled by the existing outer logic, and except Exception as
e: log the error (include identifying user info such as user.get("user_id") or
user.get("email"), the exception message, and sync_context state), increment a
per-user failure counter on sync_context if present, and continue to the next
user; this ensures normal progress logging (sync_context.processed_users,
new_users, updated_users, admin_users_synced) still occurs for successful users.
- Around line 494-521: _process_single_user currently uses unguarded dict
indexing (user['user_id'] and roles['roles']) which can raise KeyError and abort
the whole sync; change these accesses to defensive lookups (e.g., use
user.get('user_id') and roles.get('roles') or validate presence) and handle
missing values by logging the issue and returning False (or skipping the user)
before calling UserService.add_or_update_user or iterating roles; ensure the
retry method _fetch_user_roles_with_retry is checked for a valid dict shape and
that is_admin uses a safe iteration over an empty list when roles are absent so
the sync loop won't crash.
In `@python/mdvtools/auth/tests/test_auth.py`:
- Line 217: The assertion using
mock_user_project_service.add_or_update_user_project.call_count >= 0 is vacuous
(always true); update the test to either assert a meaningful expectation (e.g.,
assert mock_user_project_service.add_or_update_user_project.call_count > 0 or
use assert_called_once()/assert_called() if you expect a specific or
at-least-one call) when you require project assignments to occur, or remove the
assertion (or replace it with a short explanatory comment) if the call is
genuinely optional; locate the usage of
mock_user_project_service.add_or_update_user_project in the failing test and
apply the appropriate change.
- Around line 98-142: The assertion using
mock_user_project_service.add_or_update_user_project.call_count is a no-op;
change the test to either (A) set mock_project_model.query.all.return_value to a
non-empty list of fake project objects and then assert
mock_user_project_service.add_or_update_user_project.call_count equals the
expected number of assignments (e.g., len(projects)), referencing
mock_project_model.query.all and context.all_projects so the admin-assignment
loop in the code runs, or (B) if you want to test the empty-project case,
explicitly assert call_count == 0 (not >= 0) and keep
mock_project_model.query.all.return_value = [] to verify no assignments occur.
Ensure you update the assertion that references
mock_user_project_service.add_or_update_user_project.call_count accordingly.
🧹 Nitpick comments (4)
python/mdvtools/auth/auth0_provider.py (1)
410-413: Duplicated default value forpagination_retry_delay.The value
2.0appears both in__init__(line 384) and here. If the default changes in one place but not the other, behavior will diverge. Consider using a class-level constant.Suggested fix
+ DEFAULT_PAGINATION_RETRY_DELAY = 2.0 + class SyncContext: """Context object for user sync operations.""" def __init__( self, auth0: Auth0, all_projects: List['Project'], page: int, ... - pagination_retry_delay: float = 2.0, + pagination_retry_delay: float = Auth0Provider.DEFAULT_PAGINATION_RETRY_DELAY, ... ): ... def reset_pagination_rate_limit(self) -> None: """Reset pagination rate limit counters after successful request.""" self.pagination_rate_limit_count = 0 - self.pagination_retry_delay = 2.0 + self.pagination_retry_delay = Auth0Provider.DEFAULT_PAGINATION_RETRY_DELAYpython/mdvtools/auth/tests/test_auth.py (3)
337-338: Consider patchingtime.sleepfor consistency with other tests.Other test methods in this class patch
time.sleepto avoid unnecessary delays. If any code path before or after the empty-list check callssleep(e.g., an inter-page delay), this test will be slower than necessary.
160-211: Extract common provider creation into a shared fixture to reduce duplication.Every test method repeats the same ~15 lines: patching
requests.get, enteringapp_context, constructingAuth0Provider, and patchinglogging+time.sleep. A single fixture (e.g.,auth0_provider) that yields a ready-to-use provider instance would reduce boilerplate and make future test changes easier.Example fixture sketch
`@pytest.fixture` def auth0_provider(self, auth0_app, mock_get_token, mock_auth0_class): """Create an Auth0Provider with all external calls mocked.""" with patch('mdvtools.auth.auth0_provider.requests.get') as mock_get: mock_get.return_value.status_code = 200 mock_get.return_value.json.return_value = { 'jwks_uri': 'https://test.auth0.com/.well-known/jwks.json' } with auth0_app.app_context(): provider = Auth0Provider( auth0_app, oauth=MagicMock(), client_id="test_id", client_secret="test_secret", domain="test.auth0.com" ) with patch('mdvtools.auth.auth0_provider.logging'): with patch('mdvtools.auth.auth0_provider.time.sleep'): yield provider
72-77: Consider adding a test for loop-variable capture in per-user retry logic.A previous review identified that closure issues capturing loop variables in retry decorators can cause privilege escalation (e.g., wrong admin roles assigned). While the retry decorator was reportedly removed, it would be valuable to add a test that verifies user-role assignments remain correct when retries occur mid-iteration — e.g., assert that after a rate-limit retry for user N, the roles assigned still belong to user N, not user N+1. Based on learnings, this pattern was flagged as a critical security vulnerability in Auth0 user sync operations.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@python/mdvtools/auth/auth0_provider.py`:
- Around line 533-536: The counters for context.new_users and
context.updated_users are incremented unconditionally while
context.processed_users is only incremented on successful completion, causing
skew if role fetching (around the role fetch call) fails; change the logic so
that the increments of context.new_users/context.updated_users (based on
is_new_user) occur after the full user processing succeeds (i.e., move those
increments into the success path after the block that currently increments
context.processed_users), or alternatively update the final logging to
distinguish "attempted" vs "completed" counts; reference the is_new_user
variable, context.new_users, context.updated_users, and context.processed_users
to locate and adjust the code accordingly.
- Around line 266-302: The code is doing network I/O (requests.get) while
holding _jwks_cache_lock inside is_token_valid, which can block other threads;
fix by performing the HTTP fetch outside the with _jwks_cache_lock block (call
requests.get(self.app.config['AUTH0_PUBLIC_KEY_URI'], timeout=...) before
acquiring the lock), then re-acquire _jwks_cache_lock to re-check
_jwks_cache_expiry and, if still expired, swap in the fetched _jwks_cache and
update _jwks_cache_expiry = current_time + JWKS_CACHE_DURATION (and log/return
appropriately); also add a reasonable timeout to requests.get and handle non-200
or exceptions before attempting the cache swap so threads only hold the lock
briefly for the assignment.
1 participant
Added warnings about API call frequency and potential rate limit issues.
Changed control-flow for retries to hopefully be more coherent and predictable, add a bunch of general logging.
Also made changes which should make the JWKS cache thread-safe.
Note: some things that could change behaviour, like not re-querying db during processing...
Summary by CodeRabbit
Bug Fixes
Documentation
Tests