feat(auth): CredentialStore trait - #542

Merged
alexhancock merged 1 commit into
mainfrom
alexhancock/credential-store
Nov 17, 2025
Merged

feat(auth): CredentialStore trait#542
alexhancock merged 1 commit into
mainfrom
alexhancock/credential-store

Conversation

@alexhancock

@alexhancockalexhancock commented Nov 14, 2025

Copy link
Copy Markdown
Contributor

Description

Adds the concept of a configurable CredentialStore for AuthorizationManager to use

The current implementation writes to credentials in memory. This change gives the flexibility for users of AuthorizationManager to provide a custom implementation that persists credentials (to keychain, disk, etc).

There is no change to default behavior, as a default implementation is provided which does the same as now (InMemoryCredentialStore)

Motivation

The motivation came from this bug report to goose that new refresh tokens sent from the server during token exchange were not being saved/used aaif-goose/goose#5259

We store access tokens and refresh tokens via keyring, but we only did it on the initial auth and previously had no way to be informed of new refresh tokens received when rmcp exchanges tokens.

This allows for a provided CredentialStore to always load/save/clear token information from the storage mechanism of choice, and will fixaaif-goose/goose#5259

@github-actionsgithub-actionsBot added T-dependencies Dependencies related changes T-config Configuration file changes T-core Core library changes T-transport Transport layer changes labels Nov 14, 2025
@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch from aec2b30 to adb0f89CompareNovember 14, 2025 18:10
@alexhancockalexhancock changed the title feat: credential store traitfeat(auth): CredentialStore traitNov 14, 2025
@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch 2 times, most recently from 36ef601 to c3e6d71CompareNovember 14, 2025 18:18
Comment threadcrates/rmcp/src/transport/auth.rs
Comment threadcrates/rmcp/src/transport/auth.rs
rustdoc-args = ["--cfg", "docsrs"]

[dependencies]
async-trait = "0.1.89"

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.

not sure how big a concern it is to have this, but if you want to avoid adding this dependency you can have the trait return Futures

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I often use this dependency and I am planning to introduce it in the future to make the code more concise. Do you want to change other implementations to this dependency implementation as well?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes, we can use it more - it seems nice

@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch from c3e6d71 to a9b1962CompareNovember 14, 2025 20:24
jamadeo
jamadeo previously approved these changes Nov 14, 2025
jokemanfire
jokemanfire previously approved these changes Nov 17, 2025

@jokemanfirejokemanfire left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@alexhancock
alexhancock dismissed stale reviews from jokemanfire and jamadeo via 479c0ceNovember 17, 2025 14:31
@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch from a9b1962 to 479c0ceCompareNovember 17, 2025 14:31
@alexhancock
alexhancock merged commit 31be9b2 into mainNov 17, 2025
11 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Nov 17, 2025
takumi-earth pushed a commit to earthlings-dev/rmcp that referenced this pull request Jan 27, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-configConfiguration file changesT-coreCore library changesT-dependenciesDependencies related changesT-transportTransport layer changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MCP OAuth refresh_token not used correctly

3 participants

@alexhancock@jamadeo@jokemanfire
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

feat(auth): CredentialStore trait - #542

Merged
alexhancock merged 1 commit into
mainfrom
alexhancock/credential-store
Nov 17, 2025
Merged

feat(auth): CredentialStore trait#542
alexhancock merged 1 commit into
mainfrom
alexhancock/credential-store

Conversation

@alexhancock

@alexhancockalexhancock commented Nov 14, 2025

Copy link
Copy Markdown
Contributor

Description

Adds the concept of a configurable CredentialStore for AuthorizationManager to use

The current implementation writes to credentials in memory. This change gives the flexibility for users of AuthorizationManager to provide a custom implementation that persists credentials (to keychain, disk, etc).

There is no change to default behavior, as a default implementation is provided which does the same as now (InMemoryCredentialStore)

Motivation

The motivation came from this bug report to goose that new refresh tokens sent from the server during token exchange were not being saved/used aaif-goose/goose#5259

We store access tokens and refresh tokens via keyring, but we only did it on the initial auth and previously had no way to be informed of new refresh tokens received when rmcp exchanges tokens.

This allows for a provided CredentialStore to always load/save/clear token information from the storage mechanism of choice, and will fixaaif-goose/goose#5259

@github-actionsgithub-actionsBot added T-dependencies Dependencies related changes T-config Configuration file changes T-core Core library changes T-transport Transport layer changes labels Nov 14, 2025
@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch from aec2b30 to adb0f89CompareNovember 14, 2025 18:10
@alexhancockalexhancock changed the title feat: credential store traitfeat(auth): CredentialStore traitNov 14, 2025
@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch 2 times, most recently from 36ef601 to c3e6d71CompareNovember 14, 2025 18:18
Comment threadcrates/rmcp/src/transport/auth.rs
Comment threadcrates/rmcp/src/transport/auth.rs
rustdoc-args = ["--cfg", "docsrs"]

[dependencies]
async-trait = "0.1.89"

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.

not sure how big a concern it is to have this, but if you want to avoid adding this dependency you can have the trait return Futures

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I often use this dependency and I am planning to introduce it in the future to make the code more concise. Do you want to change other implementations to this dependency implementation as well?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes, we can use it more - it seems nice

@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch from c3e6d71 to a9b1962CompareNovember 14, 2025 20:24
jamadeo
jamadeo previously approved these changes Nov 14, 2025
jokemanfire
jokemanfire previously approved these changes Nov 17, 2025

@jokemanfirejokemanfire left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@alexhancock
alexhancock dismissed stale reviews from jokemanfire and jamadeo via 479c0ceNovember 17, 2025 14:31
@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch from a9b1962 to 479c0ceCompareNovember 17, 2025 14:31
@alexhancock
alexhancock merged commit 31be9b2 into mainNov 17, 2025
11 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Nov 17, 2025
takumi-earth pushed a commit to earthlings-dev/rmcp that referenced this pull request Jan 27, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-configConfiguration file changesT-coreCore library changesT-dependenciesDependencies related changesT-transportTransport layer changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MCP OAuth refresh_token not used correctly

3 participants

@alexhancock@jamadeo@jokemanfire
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(auth): CredentialStore trait - #542

Merged
alexhancock merged 1 commit into
mainfrom
alexhancock/credential-store
Nov 17, 2025
Merged

feat(auth): CredentialStore trait#542
alexhancock merged 1 commit into
mainfrom
alexhancock/credential-store

Conversation

@alexhancock

@alexhancockalexhancock commented Nov 14, 2025

Copy link
Copy Markdown
Contributor

Description

Adds the concept of a configurable CredentialStore for AuthorizationManager to use

The current implementation writes to credentials in memory. This change gives the flexibility for users of AuthorizationManager to provide a custom implementation that persists credentials (to keychain, disk, etc).

There is no change to default behavior, as a default implementation is provided which does the same as now (InMemoryCredentialStore)

Motivation

The motivation came from this bug report to goose that new refresh tokens sent from the server during token exchange were not being saved/used aaif-goose/goose#5259

We store access tokens and refresh tokens via keyring, but we only did it on the initial auth and previously had no way to be informed of new refresh tokens received when rmcp exchanges tokens.

This allows for a provided CredentialStore to always load/save/clear token information from the storage mechanism of choice, and will fixaaif-goose/goose#5259

@github-actionsgithub-actionsBot added T-dependencies Dependencies related changes T-config Configuration file changes T-core Core library changes T-transport Transport layer changes labels Nov 14, 2025
@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch from aec2b30 to adb0f89CompareNovember 14, 2025 18:10
@alexhancockalexhancock changed the title feat: credential store traitfeat(auth): CredentialStore traitNov 14, 2025
@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch 2 times, most recently from 36ef601 to c3e6d71CompareNovember 14, 2025 18:18
Comment threadcrates/rmcp/src/transport/auth.rs
Comment threadcrates/rmcp/src/transport/auth.rs
rustdoc-args = ["--cfg", "docsrs"]

[dependencies]
async-trait = "0.1.89"

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.

not sure how big a concern it is to have this, but if you want to avoid adding this dependency you can have the trait return Futures

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I often use this dependency and I am planning to introduce it in the future to make the code more concise. Do you want to change other implementations to this dependency implementation as well?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes, we can use it more - it seems nice

@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch from c3e6d71 to a9b1962CompareNovember 14, 2025 20:24
jamadeo
jamadeo previously approved these changes Nov 14, 2025
jokemanfire
jokemanfire previously approved these changes Nov 17, 2025

@jokemanfirejokemanfire left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@alexhancock
alexhancock dismissed stale reviews from jokemanfire and jamadeo via 479c0ceNovember 17, 2025 14:31
@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch from a9b1962 to 479c0ceCompareNovember 17, 2025 14:31
@alexhancock
alexhancock merged commit 31be9b2 into mainNov 17, 2025
11 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Nov 17, 2025
takumi-earth pushed a commit to earthlings-dev/rmcp that referenced this pull request Jan 27, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-configConfiguration file changesT-coreCore library changesT-dependenciesDependencies related changesT-transportTransport layer changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MCP OAuth refresh_token not used correctly

3 participants

@alexhancock@jamadeo@jokemanfire
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(auth): CredentialStore trait - #542

Merged
alexhancock merged 1 commit into
mainfrom
alexhancock/credential-store
Nov 17, 2025
Merged

feat(auth): CredentialStore trait#542
alexhancock merged 1 commit into
mainfrom
alexhancock/credential-store

Conversation

@alexhancock

@alexhancockalexhancock commented Nov 14, 2025

Copy link
Copy Markdown
Contributor

Description

Adds the concept of a configurable CredentialStore for AuthorizationManager to use

The current implementation writes to credentials in memory. This change gives the flexibility for users of AuthorizationManager to provide a custom implementation that persists credentials (to keychain, disk, etc).

There is no change to default behavior, as a default implementation is provided which does the same as now (InMemoryCredentialStore)

Motivation

The motivation came from this bug report to goose that new refresh tokens sent from the server during token exchange were not being saved/used aaif-goose/goose#5259

We store access tokens and refresh tokens via keyring, but we only did it on the initial auth and previously had no way to be informed of new refresh tokens received when rmcp exchanges tokens.

This allows for a provided CredentialStore to always load/save/clear token information from the storage mechanism of choice, and will fixaaif-goose/goose#5259

@github-actionsgithub-actionsBot added T-dependencies Dependencies related changes T-config Configuration file changes T-core Core library changes T-transport Transport layer changes labels Nov 14, 2025
@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch from aec2b30 to adb0f89CompareNovember 14, 2025 18:10
@alexhancockalexhancock changed the title feat: credential store traitfeat(auth): CredentialStore traitNov 14, 2025
@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch 2 times, most recently from 36ef601 to c3e6d71CompareNovember 14, 2025 18:18
Comment threadcrates/rmcp/src/transport/auth.rs
Comment threadcrates/rmcp/src/transport/auth.rs
rustdoc-args = ["--cfg", "docsrs"]

[dependencies]
async-trait = "0.1.89"

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.

not sure how big a concern it is to have this, but if you want to avoid adding this dependency you can have the trait return Futures

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I often use this dependency and I am planning to introduce it in the future to make the code more concise. Do you want to change other implementations to this dependency implementation as well?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes, we can use it more - it seems nice

@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch from c3e6d71 to a9b1962CompareNovember 14, 2025 20:24
jamadeo
jamadeo previously approved these changes Nov 14, 2025
jokemanfire
jokemanfire previously approved these changes Nov 17, 2025

@jokemanfirejokemanfire left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@alexhancock
alexhancock dismissed stale reviews from jokemanfire and jamadeo via 479c0ceNovember 17, 2025 14:31
@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch from a9b1962 to 479c0ceCompareNovember 17, 2025 14:31
@alexhancock
alexhancock merged commit 31be9b2 into mainNov 17, 2025
11 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Nov 17, 2025
takumi-earth pushed a commit to earthlings-dev/rmcp that referenced this pull request Jan 27, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-configConfiguration file changesT-coreCore library changesT-dependenciesDependencies related changesT-transportTransport layer changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MCP OAuth refresh_token not used correctly

3 participants

@alexhancock@jamadeo@jokemanfire
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

feat(auth): CredentialStore trait - #542

Merged
alexhancock merged 1 commit into
mainfrom
alexhancock/credential-store
Nov 17, 2025
Merged

feat(auth): CredentialStore trait#542
alexhancock merged 1 commit into
mainfrom
alexhancock/credential-store

Conversation

@alexhancock

@alexhancockalexhancock commented Nov 14, 2025

Copy link
Copy Markdown
Contributor

Description

Adds the concept of a configurable CredentialStore for AuthorizationManager to use

The current implementation writes to credentials in memory. This change gives the flexibility for users of AuthorizationManager to provide a custom implementation that persists credentials (to keychain, disk, etc).

There is no change to default behavior, as a default implementation is provided which does the same as now (InMemoryCredentialStore)

Motivation

The motivation came from this bug report to goose that new refresh tokens sent from the server during token exchange were not being saved/used aaif-goose/goose#5259

We store access tokens and refresh tokens via keyring, but we only did it on the initial auth and previously had no way to be informed of new refresh tokens received when rmcp exchanges tokens.

This allows for a provided CredentialStore to always load/save/clear token information from the storage mechanism of choice, and will fixaaif-goose/goose#5259

@github-actionsgithub-actionsBot added T-dependencies Dependencies related changes T-config Configuration file changes T-core Core library changes T-transport Transport layer changes labels Nov 14, 2025
@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch from aec2b30 to adb0f89CompareNovember 14, 2025 18:10
@alexhancockalexhancock changed the title feat: credential store traitfeat(auth): CredentialStore traitNov 14, 2025
@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch 2 times, most recently from 36ef601 to c3e6d71CompareNovember 14, 2025 18:18
Comment threadcrates/rmcp/src/transport/auth.rs
Comment threadcrates/rmcp/src/transport/auth.rs
rustdoc-args = ["--cfg", "docsrs"]

[dependencies]
async-trait = "0.1.89"

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.

not sure how big a concern it is to have this, but if you want to avoid adding this dependency you can have the trait return Futures

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I often use this dependency and I am planning to introduce it in the future to make the code more concise. Do you want to change other implementations to this dependency implementation as well?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes, we can use it more - it seems nice

@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch from c3e6d71 to a9b1962CompareNovember 14, 2025 20:24
jamadeo
jamadeo previously approved these changes Nov 14, 2025
jokemanfire
jokemanfire previously approved these changes Nov 17, 2025

@jokemanfirejokemanfire left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@alexhancock
alexhancock dismissed stale reviews from jokemanfire and jamadeo via 479c0ceNovember 17, 2025 14:31
@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch from a9b1962 to 479c0ceCompareNovember 17, 2025 14:31
@alexhancock
alexhancock merged commit 31be9b2 into mainNov 17, 2025
11 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Nov 17, 2025
takumi-earth pushed a commit to earthlings-dev/rmcp that referenced this pull request Jan 27, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-configConfiguration file changesT-coreCore library changesT-dependenciesDependencies related changesT-transportTransport layer changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MCP OAuth refresh_token not used correctly

3 participants

@alexhancock@jamadeo@jokemanfire
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(auth): CredentialStore trait - #542

Merged
alexhancock merged 1 commit into
mainfrom
alexhancock/credential-store
Nov 17, 2025
Merged

feat(auth): CredentialStore trait#542
alexhancock merged 1 commit into
mainfrom
alexhancock/credential-store

Conversation

@alexhancock

@alexhancockalexhancock commented Nov 14, 2025

Copy link
Copy Markdown
Contributor

Description

Adds the concept of a configurable CredentialStore for AuthorizationManager to use

The current implementation writes to credentials in memory. This change gives the flexibility for users of AuthorizationManager to provide a custom implementation that persists credentials (to keychain, disk, etc).

There is no change to default behavior, as a default implementation is provided which does the same as now (InMemoryCredentialStore)

Motivation

The motivation came from this bug report to goose that new refresh tokens sent from the server during token exchange were not being saved/used aaif-goose/goose#5259

We store access tokens and refresh tokens via keyring, but we only did it on the initial auth and previously had no way to be informed of new refresh tokens received when rmcp exchanges tokens.

This allows for a provided CredentialStore to always load/save/clear token information from the storage mechanism of choice, and will fixaaif-goose/goose#5259

@github-actionsgithub-actionsBot added T-dependencies Dependencies related changes T-config Configuration file changes T-core Core library changes T-transport Transport layer changes labels Nov 14, 2025
@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch from aec2b30 to adb0f89CompareNovember 14, 2025 18:10
@alexhancockalexhancock changed the title feat: credential store traitfeat(auth): CredentialStore traitNov 14, 2025
@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch 2 times, most recently from 36ef601 to c3e6d71CompareNovember 14, 2025 18:18
Comment threadcrates/rmcp/src/transport/auth.rs
Comment threadcrates/rmcp/src/transport/auth.rs
rustdoc-args = ["--cfg", "docsrs"]

[dependencies]
async-trait = "0.1.89"

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.

not sure how big a concern it is to have this, but if you want to avoid adding this dependency you can have the trait return Futures

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I often use this dependency and I am planning to introduce it in the future to make the code more concise. Do you want to change other implementations to this dependency implementation as well?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes, we can use it more - it seems nice

@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch from c3e6d71 to a9b1962CompareNovember 14, 2025 20:24
jamadeo
jamadeo previously approved these changes Nov 14, 2025
jokemanfire
jokemanfire previously approved these changes Nov 17, 2025

@jokemanfirejokemanfire left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@alexhancock
alexhancock dismissed stale reviews from jokemanfire and jamadeo via 479c0ceNovember 17, 2025 14:31
@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch from a9b1962 to 479c0ceCompareNovember 17, 2025 14:31
@alexhancock
alexhancock merged commit 31be9b2 into mainNov 17, 2025
11 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Nov 17, 2025
takumi-earth pushed a commit to earthlings-dev/rmcp that referenced this pull request Jan 27, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-configConfiguration file changesT-coreCore library changesT-dependenciesDependencies related changesT-transportTransport layer changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MCP OAuth refresh_token not used correctly

3 participants

@alexhancock@jamadeo@jokemanfire
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(auth): CredentialStore trait - #542

Merged
alexhancock merged 1 commit into
mainfrom
alexhancock/credential-store
Nov 17, 2025
Merged

feat(auth): CredentialStore trait#542
alexhancock merged 1 commit into
mainfrom
alexhancock/credential-store

Conversation

@alexhancock

@alexhancockalexhancock commented Nov 14, 2025

Copy link
Copy Markdown
Contributor

Description

Adds the concept of a configurable CredentialStore for AuthorizationManager to use

The current implementation writes to credentials in memory. This change gives the flexibility for users of AuthorizationManager to provide a custom implementation that persists credentials (to keychain, disk, etc).

There is no change to default behavior, as a default implementation is provided which does the same as now (InMemoryCredentialStore)

Motivation

The motivation came from this bug report to goose that new refresh tokens sent from the server during token exchange were not being saved/used aaif-goose/goose#5259

We store access tokens and refresh tokens via keyring, but we only did it on the initial auth and previously had no way to be informed of new refresh tokens received when rmcp exchanges tokens.

This allows for a provided CredentialStore to always load/save/clear token information from the storage mechanism of choice, and will fixaaif-goose/goose#5259

@github-actionsgithub-actionsBot added T-dependencies Dependencies related changes T-config Configuration file changes T-core Core library changes T-transport Transport layer changes labels Nov 14, 2025
@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch from aec2b30 to adb0f89CompareNovember 14, 2025 18:10
@alexhancockalexhancock changed the title feat: credential store traitfeat(auth): CredentialStore traitNov 14, 2025
@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch 2 times, most recently from 36ef601 to c3e6d71CompareNovember 14, 2025 18:18
Comment threadcrates/rmcp/src/transport/auth.rs
Comment threadcrates/rmcp/src/transport/auth.rs
rustdoc-args = ["--cfg", "docsrs"]

[dependencies]
async-trait = "0.1.89"

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.

not sure how big a concern it is to have this, but if you want to avoid adding this dependency you can have the trait return Futures

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I often use this dependency and I am planning to introduce it in the future to make the code more concise. Do you want to change other implementations to this dependency implementation as well?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes, we can use it more - it seems nice

@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch from c3e6d71 to a9b1962CompareNovember 14, 2025 20:24
jamadeo
jamadeo previously approved these changes Nov 14, 2025
jokemanfire
jokemanfire previously approved these changes Nov 17, 2025

@jokemanfirejokemanfire left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@alexhancock
alexhancock dismissed stale reviews from jokemanfire and jamadeo via 479c0ceNovember 17, 2025 14:31
@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch from a9b1962 to 479c0ceCompareNovember 17, 2025 14:31
@alexhancock
alexhancock merged commit 31be9b2 into mainNov 17, 2025
11 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Nov 17, 2025
takumi-earth pushed a commit to earthlings-dev/rmcp that referenced this pull request Jan 27, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-configConfiguration file changesT-coreCore library changesT-dependenciesDependencies related changesT-transportTransport layer changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MCP OAuth refresh_token not used correctly

3 participants

@alexhancock@jamadeo@jokemanfire
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

feat(auth): CredentialStore trait - #542

Merged
alexhancock merged 1 commit into
mainfrom
alexhancock/credential-store
Nov 17, 2025
Merged

feat(auth): CredentialStore trait#542
alexhancock merged 1 commit into
mainfrom
alexhancock/credential-store

Conversation

@alexhancock

@alexhancockalexhancock commented Nov 14, 2025

Copy link
Copy Markdown
Contributor

Description

Adds the concept of a configurable CredentialStore for AuthorizationManager to use

The current implementation writes to credentials in memory. This change gives the flexibility for users of AuthorizationManager to provide a custom implementation that persists credentials (to keychain, disk, etc).

There is no change to default behavior, as a default implementation is provided which does the same as now (InMemoryCredentialStore)

Motivation

The motivation came from this bug report to goose that new refresh tokens sent from the server during token exchange were not being saved/used aaif-goose/goose#5259

We store access tokens and refresh tokens via keyring, but we only did it on the initial auth and previously had no way to be informed of new refresh tokens received when rmcp exchanges tokens.

This allows for a provided CredentialStore to always load/save/clear token information from the storage mechanism of choice, and will fixaaif-goose/goose#5259

@github-actionsgithub-actionsBot added T-dependencies Dependencies related changes T-config Configuration file changes T-core Core library changes T-transport Transport layer changes labels Nov 14, 2025
@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch from aec2b30 to adb0f89CompareNovember 14, 2025 18:10
@alexhancockalexhancock changed the title feat: credential store traitfeat(auth): CredentialStore traitNov 14, 2025
@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch 2 times, most recently from 36ef601 to c3e6d71CompareNovember 14, 2025 18:18
Comment threadcrates/rmcp/src/transport/auth.rs
Comment threadcrates/rmcp/src/transport/auth.rs
rustdoc-args = ["--cfg", "docsrs"]

[dependencies]
async-trait = "0.1.89"

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.

not sure how big a concern it is to have this, but if you want to avoid adding this dependency you can have the trait return Futures

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I often use this dependency and I am planning to introduce it in the future to make the code more concise. Do you want to change other implementations to this dependency implementation as well?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes, we can use it more - it seems nice

@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch from c3e6d71 to a9b1962CompareNovember 14, 2025 20:24
jamadeo
jamadeo previously approved these changes Nov 14, 2025
jokemanfire
jokemanfire previously approved these changes Nov 17, 2025

@jokemanfirejokemanfire left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@alexhancock
alexhancock dismissed stale reviews from jokemanfire and jamadeo via 479c0ceNovember 17, 2025 14:31
@alexhancock
alexhancockforce-pushed the alexhancock/credential-store branch from a9b1962 to 479c0ceCompareNovember 17, 2025 14:31
@alexhancock
alexhancock merged commit 31be9b2 into mainNov 17, 2025
11 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Nov 17, 2025
takumi-earth pushed a commit to earthlings-dev/rmcp that referenced this pull request Jan 27, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-configConfiguration file changesT-coreCore library changesT-dependenciesDependencies related changesT-transportTransport layer changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MCP OAuth refresh_token not used correctly

3 participants

@alexhancock@jamadeo@jokemanfire