Skip to content

fix(plugins): keep open edx credentials on the server - #969

Merged
abdulrafey1 merged 23 commits into
rafe/lms-settings-canvas-moodlefrom
rafe/lms-settings-openedx
Oct 26, 2026
Merged

abdulrafey1 merged 23 commits into
rafe/lms-settings-canvas-moodlefrom
rafe/lms-settings-openedx

Conversation

@abdulrafey1

@abdulrafey1 abdulrafey1 commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator

What

Open edX tools no longer take a password, an access token or LMS URLs from the model. The Open edX
client gets and caches its own access token with the user's stored settings, so the password never
reaches the chat prompt and the token never passes through the model.

Changes

  • fix(plugins): OpenEdxClient(config, user_id) runs the OAuth2 password grant with the stored
    username and password on first use, then caches the token in Redis under
    openedx:access-token:<user_id>:<digest>. The digest is an HMAC-SHA256, keyed with SECRET_KEY, of
    the stored LMS URL, username and password. Saving new settings moves the user to a new key, and the
    old token expires on its own. The key shows none of the settings. The TTL is expires_in minus 60 s,
    or the cache default when Open edX sends no expiry.
  • fix(plugins): on a 401 the client drops the cached token, grants a new one and retries once. A
    second 401 is reported. A token-response error never includes the response body, in the message or
    in a chained cause.
  • fix(plugins): OpenEdxClient.studio_url gives the Studio base URL from the stored settings. The
    tools open a client through one _open_client() factory, which loads the caller's settings and user
    id. The tool helpers (create_basic_component, update_xblock_content, …) take the open client.
  • fix(plugins): the openedx_authenticate and openedx_refresh_access_token tools are removed, and so
    is every auth field. openedx_get_user_info becomes openedx_check_settings (category
    openedx-settings): it takes no arguments, checks the stored settings and returns the signed-in account.
    openedx_add_plugin_content loads settings before resolving the contributor, so an unconfigured user
    never creates contributor content.
  • fix(plugins): OpenEdxConfig drops to_lms_credentials_hint.
  • feat(core): sparkth.lib.cache exposes CacheService and get_cache_service to plugins. Their
    docstrings now describe the API, and the API reference lists the module.
  • test(core): sparkth.lib.testing gains FakeRedis and an opt-in fake_redis fixture that routes the
    shared CacheService to it. The email-verification test uses the same FakeRedis.

How to Test

  1. uv run pytest sparkth/plugins/openedx tests/api/v1/test_auth_email_verification.py -v.
  2. test_client.py checks that:
    • a token is granted once and reused;
    • two users never share a token;
    • saving new settings grants a new token under a new key;
    • the cache key shows none of the stored settings;
    • a 401 retries once with a fresh token, and a second 401 is reported;
    • the TTL never outlives expires_in;
    • an unreachable Redis still works;
    • a malformed token response never echoes its body.
  3. Manual: with Open edX set up, create a course, then a section and unit in a later turn. No
    authenticate step happens, and every call succeeds.
  4. Manual: save different Open edX settings, then call a tool. It acts as the new account.

Notes

Breaking for MCP clients: the two auth tools are gone and the Open edX tool schemas no longer accept
auth. On /ai/mcp no user is bound, so these tools fail with NoAuthenticatedUser.
The token is cached in Redis (REDIS_URL, already required) as plain text. It is a short-lived access
token whose cache entry expires with it, and the password is never cached. No new env var, no migration.

This description was written with the assistance of an LLM (Claude).

🤖 Generated with Claude Code

abdulrafey1 and others added 8 commits October 22, 2026 00:23
Add a ConfigAdapter base with no-op preprocess, postprocess and sync_cache
steps. CONFIG_ADAPTERS now holds ConfigAdapter, and LLMConfigAdapter
subclasses it, so a plugin overrides only the steps it needs.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The Open edX client grants its own access token from the user's stored
settings and caches it per user in Redis, retrying once on a 401. Every
Open edX tool loads the calling user's stored settings instead of taking
URLs, credentials or tokens from the model, and the authenticate and
refresh-token tools are gone. Open edX settings never enter the prompt.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Saving Open edX settings through the config route now deletes that user's cached access
token, so the next call grants one with the new credentials. The sync_cache docs example
now shows where a plugin gets its cache.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…er-base

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Say that sync_cache runs only after the PUT config route, not the
first POST configure, and that a CONFIG_ADAPTERS entry is any
ConfigAdapter overriding only the steps it needs. Wrap the plugin guide
lines this branch edited to 120 characters and give sync_cache a
docstring body in place of its redundant return.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The plugin-facing CacheService methods and get_cache_service now say
what they return, how a None or 0 ttl falls back to the default, and
that a Redis error is logged and swallowed.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The config route test now also checks that the response returns the
saved Open edX config unchanged while the cached token is dropped.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@abdulrafey1
abdulrafey1 added this pull request to stack #971 October 21, 2026 23:51
@abdulrafey1 abdulrafey1 self-assigned this Oct 22, 2026

@hamza-56 hamza-56 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 point:

Comment thread sparkth/plugins/openedx/tools.py Outdated
Comment thread sparkth/plugins/openedx/client.py Outdated
Comment thread sparkth/plugins/openedx/client.py Outdated
Comment thread sparkth/plugins/openedx/client.py
Comment thread sparkth/plugins/openedx/tools.py Outdated
Comment thread sparkth/plugins/openedx/tools.py Outdated
Comment thread sparkth/plugins/openedx/tests/conftest.py Outdated
abdulrafey1 and others added 14 commits October 26, 2026 16:19
…er-base

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…er-base

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…est helpers

openedx_get_user_info is now openedx_check_settings in the openedx-settings category, following the
<lms>_check_settings convention the chat rules name; it still returns the signed-in account.
The stored-settings fixture is opt-in, so the real-storage test needs no patch-back, and the
credential and real-storage tests use the sparkth.lib.testing helpers.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ocstring

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
OpenEdxClient.studio_url normalises the stored Studio URL once, and the tools and helpers use it
and the LMS base_url, so no call site rebuilds or forgets the trailing-slash strip. The helpers
that took config or studio_url only to rebuild that URL no longer take them.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
OpenEdxClient now takes the Sparkth user it acts for, so its signature says it needs one and it
no longer reads current_user_id itself. _open_client is the one place the tools read the caller
and their stored settings; the create and update helpers take its open client, so a tool that
makes several calls reuses one client.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The cached token's key now carries an HMAC of the LMS URL, username and password, keyed with the
server secret. Saving new settings moves the user to a new key, so OpenEdxConfigAdapter and the
general config adapter hook it needed are gone; the old token just expires. This also closes the
window in which a request could reuse the old token right after a settings change.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…pers

create_basic_component and update_xblock_content are helpers the tools call, not tools, so they no
longer carry the openedx_ prefix, like set_course_language.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
FakeRedis and a fake_redis fixture that installs it on the shared CacheService move to
sparkth.lib.testing. The fixture is opt-in, so only the Open edX tests that grant a token ask for
it, and the resend rate-limit test reuses the class in place of its own copy.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@abdulrafey1
abdulrafey1 removed this pull request from stack #971 October 26, 2026 13:01
@abdulrafey1
abdulrafey1 changed the base branch from rafe/config-adapter-base to rafe/lms-settings-canvas-moodle October 26, 2026 13:01
@abdulrafey1
abdulrafey1 added this pull request to stack #999 October 26, 2026 13:02
…-openedx

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@zaira-bibi zaira-bibi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks good! please try to not have more than 10 files in the PRs.

@abdulrafey1
abdulrafey1 merged commit 7149b08 into main Oct 26, 2026
10 checks passed
@abdulrafey1
abdulrafey1 deleted the rafe/lms-settings-openedx branch October 26, 2026 13:53
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.

3 participants