Oauth support client id and secret - #107

Open
raboof wants to merge 3 commits into
apache:mainfrom
raboof:oauth-support-client-id-and-secret
Open

Oauth support client id and secret#107
raboof wants to merge 3 commits into
apache:mainfrom
raboof:oauth-support-client-id-and-secret

Conversation

@raboof

@raboofraboof commented Jun 10, 2026

Copy link
Copy Markdown
Member

The initial motivation to support this was to support authentication against a local test oauth server instead of the deployed server.

This might also be usable to authenticate against Authentik directly instead of going via oauth.apache.org . That possibly provides the oauth_data information in a different format, though, so that might need some massaging either here or in the configuration.

Draft but interested in feedback on the general idea/approach!

@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch from d4e3a95 to 12ef88cCompareJune 11, 2026 13:01
@raboof
raboof marked this pull request as draft June 11, 2026 13:08
Comment threadsrc/asfquart/generics.py Outdated
token = await rv.json()
jwks_client = PyJWKClient(OAUTH_URL_JWKS)
id_token = token["id_token"]
signing_key = jwks_client.get_signing_key_from_jwt(id_token)

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.

Pretty sure that get_signing_key_from_jwt is blocking (synchronous), but this is an async function.

Comment threadsrc/asfquart/generics.py Outdated
# someplace else. This counts as a samesite request.
return quart.Response(
status=200,
response=f"Successfully logged in! Welcome, {oauth_data['uid']}\n",

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.

I don't think this would work in the new mode because OAuth 2.0 uses sub:

sub
REQUIRED. Subject Identifier. A locally unique and never reassigned identifier within the Issuer for the End-User, which is intended to be consumed by the Client, e.g., 24400320 or AItOawmwtWwcT0k51BayewNvutrJUqsvl6qs7A4. It MUST NOT exceed 255 ASCII [RFC20] characters in length. The sub value is a case-sensitive string.

Maybe we could have a test or two to ensure that it works?

Comment threadsrc/asfquart/generics.py Outdated
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch 2 times, most recently from 5c42e65 to 2acd3eeCompareJuly 23, 2026 07:40
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch 8 times, most recently from 5377085 to 0366feaCompareAugust 5, 2026 15:07
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch 7 times, most recently from e66e09a to 9f0e633CompareAugust 20, 2026 09:12
raboof added a commit to raboof/apache-security-dash that referenced this pull request Aug 20, 2026
This allows using 'official' OAuth2/OIDC rather than the ASF dialect,
and enables more detailed logging from mfa.apache.org
requires apache/infrastructure-asfquart#107
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch from 9f0e633 to 10ed333CompareAugust 20, 2026 09:27
@raboof

Copy link
Copy Markdown
MemberAuthor

Thanks for the feedback here and on slack. Made some updates and tested with apache/security-dash#6 , ready for another round of review.

@raboof
raboof marked this pull request as ready for review August 20, 2026 09:30
self.mfa = raw_data.get("mfa", False)
self.isRole = raw_data.get("roleaccount", False)
self.metadata = raw_data.get("metadata", {}) # This can contain whatever specific metadata the app needs
if "sub" in raw_data:

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.

In this branch, several properties (isChair, isRole, dn, metadata) aren't set, but auth.py:42 reads client_session.isChair, for example, and auth.py:60 reads client_session.isRole, so this will crash if they are used, e.g. through @require(Requirements.chair) or Requirements.roleaccount. The consequence would be a 500 due to the AttributeError.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Yes, that's semi-intentional: AFAICT the OAuth profile data currently doesn't give us a way to tell whether this is a chair or role account, so AttributeError seems like it'd be preferable to making an assumption here. For reference, for me it looks like:

{
"iss": "https://mfa-dev.apache.org/application/o/local_testing-apache-org/",
"sub": "5d96dad49cb7590a117ef6de7f2506279802a87433ee34d1aa29c76db69e9a5b",
"aud": "local_testing.apache.org",
"exp": 1787239136,
"iat": 1787238536,
"auth_time": 1787125488,
"acr": "goauthentik.io/providers/oauth2/default",
"amr": [
"mfa"
],
"sid": "e4754b0ffc4191ccaa0f95e0242eaf4f5e4a04eb273a452d35534254590db816",
"email": "engelen@apache.org",
"email_verified": false,
"name": "Arnout Engelen",
"given_name": "Arnout Engelen",
"preferred_username": "engelen",
"nickname": "engelen",
"groups": [
"infrastructure-root",
"committers",
"pekko",
"commons",
"security",
"infrastructure",
"member-meta",
"infrastructure-logging",
"geode",
"infrastructure-team",
"commons-pmc",
"security-pmc",
"pekko-pmc",
"incubator"
]
}

Comment threadsrc/asfquart/generics.py Outdated
Comment threadsrc/asfquart/session.py
When CLIENT_ID is populated, use the stricter OAuth2
implementation to authenticate directly to https://mfa.apache.org
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch from 10ed333 to 9f802d3CompareAugust 20, 2026 15:47
@raboof
raboof requested a review from sbpAugust 22, 2026 09:02
raboof added a commit to raboof/apache-security-dash that referenced this pull request Aug 27, 2026
This allows using 'official' OAuth2/OIDC rather than the ASF dialect,
and enables more detailed logging from mfa.apache.org
requires apache/infrastructure-asfquart#107
raboof added a commit to raboof/apache-security-dash that referenced this pull request Sep 1, 2026
This allows using 'official' OAuth2/OIDC rather than the ASF dialect,
and enables more detailed logging from mfa.apache.org
requires apache/infrastructure-asfquart#107
Sign up for freeto 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

@raboof@sbp@dave2wave
, '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

Oauth support client id and secret - #107

Open
raboof wants to merge 3 commits into
apache:mainfrom
raboof:oauth-support-client-id-and-secret
Open

Oauth support client id and secret#107
raboof wants to merge 3 commits into
apache:mainfrom
raboof:oauth-support-client-id-and-secret

Conversation

@raboof

@raboofraboof commented Jun 10, 2026

Copy link
Copy Markdown
Member

The initial motivation to support this was to support authentication against a local test oauth server instead of the deployed server.

This might also be usable to authenticate against Authentik directly instead of going via oauth.apache.org . That possibly provides the oauth_data information in a different format, though, so that might need some massaging either here or in the configuration.

Draft but interested in feedback on the general idea/approach!

@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch from d4e3a95 to 12ef88cCompareJune 11, 2026 13:01
@raboof
raboof marked this pull request as draft June 11, 2026 13:08
Comment threadsrc/asfquart/generics.py Outdated
token = await rv.json()
jwks_client = PyJWKClient(OAUTH_URL_JWKS)
id_token = token["id_token"]
signing_key = jwks_client.get_signing_key_from_jwt(id_token)

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.

Pretty sure that get_signing_key_from_jwt is blocking (synchronous), but this is an async function.

Comment threadsrc/asfquart/generics.py Outdated
# someplace else. This counts as a samesite request.
return quart.Response(
status=200,
response=f"Successfully logged in! Welcome, {oauth_data['uid']}\n",

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.

I don't think this would work in the new mode because OAuth 2.0 uses sub:

sub
REQUIRED. Subject Identifier. A locally unique and never reassigned identifier within the Issuer for the End-User, which is intended to be consumed by the Client, e.g., 24400320 or AItOawmwtWwcT0k51BayewNvutrJUqsvl6qs7A4. It MUST NOT exceed 255 ASCII [RFC20] characters in length. The sub value is a case-sensitive string.

Maybe we could have a test or two to ensure that it works?

Comment threadsrc/asfquart/generics.py Outdated
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch 2 times, most recently from 5c42e65 to 2acd3eeCompareJuly 23, 2026 07:40
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch 8 times, most recently from 5377085 to 0366feaCompareAugust 5, 2026 15:07
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch 7 times, most recently from e66e09a to 9f0e633CompareAugust 20, 2026 09:12
raboof added a commit to raboof/apache-security-dash that referenced this pull request Aug 20, 2026
This allows using 'official' OAuth2/OIDC rather than the ASF dialect,
and enables more detailed logging from mfa.apache.org
requires apache/infrastructure-asfquart#107
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch from 9f0e633 to 10ed333CompareAugust 20, 2026 09:27
@raboof

Copy link
Copy Markdown
MemberAuthor

Thanks for the feedback here and on slack. Made some updates and tested with apache/security-dash#6 , ready for another round of review.

@raboof
raboof marked this pull request as ready for review August 20, 2026 09:30
self.mfa = raw_data.get("mfa", False)
self.isRole = raw_data.get("roleaccount", False)
self.metadata = raw_data.get("metadata", {}) # This can contain whatever specific metadata the app needs
if "sub" in raw_data:

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.

In this branch, several properties (isChair, isRole, dn, metadata) aren't set, but auth.py:42 reads client_session.isChair, for example, and auth.py:60 reads client_session.isRole, so this will crash if they are used, e.g. through @require(Requirements.chair) or Requirements.roleaccount. The consequence would be a 500 due to the AttributeError.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Yes, that's semi-intentional: AFAICT the OAuth profile data currently doesn't give us a way to tell whether this is a chair or role account, so AttributeError seems like it'd be preferable to making an assumption here. For reference, for me it looks like:

{
"iss": "https://mfa-dev.apache.org/application/o/local_testing-apache-org/",
"sub": "5d96dad49cb7590a117ef6de7f2506279802a87433ee34d1aa29c76db69e9a5b",
"aud": "local_testing.apache.org",
"exp": 1787239136,
"iat": 1787238536,
"auth_time": 1787125488,
"acr": "goauthentik.io/providers/oauth2/default",
"amr": [
"mfa"
],
"sid": "e4754b0ffc4191ccaa0f95e0242eaf4f5e4a04eb273a452d35534254590db816",
"email": "engelen@apache.org",
"email_verified": false,
"name": "Arnout Engelen",
"given_name": "Arnout Engelen",
"preferred_username": "engelen",
"nickname": "engelen",
"groups": [
"infrastructure-root",
"committers",
"pekko",
"commons",
"security",
"infrastructure",
"member-meta",
"infrastructure-logging",
"geode",
"infrastructure-team",
"commons-pmc",
"security-pmc",
"pekko-pmc",
"incubator"
]
}

Comment threadsrc/asfquart/generics.py Outdated
Comment threadsrc/asfquart/session.py
When CLIENT_ID is populated, use the stricter OAuth2
implementation to authenticate directly to https://mfa.apache.org
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch from 10ed333 to 9f802d3CompareAugust 20, 2026 15:47
@raboof
raboof requested a review from sbpAugust 22, 2026 09:02
raboof added a commit to raboof/apache-security-dash that referenced this pull request Aug 27, 2026
This allows using 'official' OAuth2/OIDC rather than the ASF dialect,
and enables more detailed logging from mfa.apache.org
requires apache/infrastructure-asfquart#107
raboof added a commit to raboof/apache-security-dash that referenced this pull request Sep 1, 2026
This allows using 'official' OAuth2/OIDC rather than the ASF dialect,
and enables more detailed logging from mfa.apache.org
requires apache/infrastructure-asfquart#107
Sign up for freeto 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

@raboof@sbp@dave2wave
, '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

Oauth support client id and secret - #107

Open
raboof wants to merge 3 commits into
apache:mainfrom
raboof:oauth-support-client-id-and-secret
Open

Oauth support client id and secret#107
raboof wants to merge 3 commits into
apache:mainfrom
raboof:oauth-support-client-id-and-secret

Conversation

@raboof

@raboofraboof commented Jun 10, 2026

Copy link
Copy Markdown
Member

The initial motivation to support this was to support authentication against a local test oauth server instead of the deployed server.

This might also be usable to authenticate against Authentik directly instead of going via oauth.apache.org . That possibly provides the oauth_data information in a different format, though, so that might need some massaging either here or in the configuration.

Draft but interested in feedback on the general idea/approach!

@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch from d4e3a95 to 12ef88cCompareJune 11, 2026 13:01
@raboof
raboof marked this pull request as draft June 11, 2026 13:08
Comment threadsrc/asfquart/generics.py Outdated
token = await rv.json()
jwks_client = PyJWKClient(OAUTH_URL_JWKS)
id_token = token["id_token"]
signing_key = jwks_client.get_signing_key_from_jwt(id_token)

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.

Pretty sure that get_signing_key_from_jwt is blocking (synchronous), but this is an async function.

Comment threadsrc/asfquart/generics.py Outdated
# someplace else. This counts as a samesite request.
return quart.Response(
status=200,
response=f"Successfully logged in! Welcome, {oauth_data['uid']}\n",

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.

I don't think this would work in the new mode because OAuth 2.0 uses sub:

sub
REQUIRED. Subject Identifier. A locally unique and never reassigned identifier within the Issuer for the End-User, which is intended to be consumed by the Client, e.g., 24400320 or AItOawmwtWwcT0k51BayewNvutrJUqsvl6qs7A4. It MUST NOT exceed 255 ASCII [RFC20] characters in length. The sub value is a case-sensitive string.

Maybe we could have a test or two to ensure that it works?

Comment threadsrc/asfquart/generics.py Outdated
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch 2 times, most recently from 5c42e65 to 2acd3eeCompareJuly 23, 2026 07:40
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch 8 times, most recently from 5377085 to 0366feaCompareAugust 5, 2026 15:07
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch 7 times, most recently from e66e09a to 9f0e633CompareAugust 20, 2026 09:12
raboof added a commit to raboof/apache-security-dash that referenced this pull request Aug 20, 2026
This allows using 'official' OAuth2/OIDC rather than the ASF dialect,
and enables more detailed logging from mfa.apache.org
requires apache/infrastructure-asfquart#107
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch from 9f0e633 to 10ed333CompareAugust 20, 2026 09:27
@raboof

Copy link
Copy Markdown
MemberAuthor

Thanks for the feedback here and on slack. Made some updates and tested with apache/security-dash#6 , ready for another round of review.

@raboof
raboof marked this pull request as ready for review August 20, 2026 09:30
self.mfa = raw_data.get("mfa", False)
self.isRole = raw_data.get("roleaccount", False)
self.metadata = raw_data.get("metadata", {}) # This can contain whatever specific metadata the app needs
if "sub" in raw_data:

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.

In this branch, several properties (isChair, isRole, dn, metadata) aren't set, but auth.py:42 reads client_session.isChair, for example, and auth.py:60 reads client_session.isRole, so this will crash if they are used, e.g. through @require(Requirements.chair) or Requirements.roleaccount. The consequence would be a 500 due to the AttributeError.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Yes, that's semi-intentional: AFAICT the OAuth profile data currently doesn't give us a way to tell whether this is a chair or role account, so AttributeError seems like it'd be preferable to making an assumption here. For reference, for me it looks like:

{
"iss": "https://mfa-dev.apache.org/application/o/local_testing-apache-org/",
"sub": "5d96dad49cb7590a117ef6de7f2506279802a87433ee34d1aa29c76db69e9a5b",
"aud": "local_testing.apache.org",
"exp": 1787239136,
"iat": 1787238536,
"auth_time": 1787125488,
"acr": "goauthentik.io/providers/oauth2/default",
"amr": [
"mfa"
],
"sid": "e4754b0ffc4191ccaa0f95e0242eaf4f5e4a04eb273a452d35534254590db816",
"email": "engelen@apache.org",
"email_verified": false,
"name": "Arnout Engelen",
"given_name": "Arnout Engelen",
"preferred_username": "engelen",
"nickname": "engelen",
"groups": [
"infrastructure-root",
"committers",
"pekko",
"commons",
"security",
"infrastructure",
"member-meta",
"infrastructure-logging",
"geode",
"infrastructure-team",
"commons-pmc",
"security-pmc",
"pekko-pmc",
"incubator"
]
}

Comment threadsrc/asfquart/generics.py Outdated
Comment threadsrc/asfquart/session.py
When CLIENT_ID is populated, use the stricter OAuth2
implementation to authenticate directly to https://mfa.apache.org
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch from 10ed333 to 9f802d3CompareAugust 20, 2026 15:47
@raboof
raboof requested a review from sbpAugust 22, 2026 09:02
raboof added a commit to raboof/apache-security-dash that referenced this pull request Aug 27, 2026
This allows using 'official' OAuth2/OIDC rather than the ASF dialect,
and enables more detailed logging from mfa.apache.org
requires apache/infrastructure-asfquart#107
raboof added a commit to raboof/apache-security-dash that referenced this pull request Sep 1, 2026
This allows using 'official' OAuth2/OIDC rather than the ASF dialect,
and enables more detailed logging from mfa.apache.org
requires apache/infrastructure-asfquart#107
Sign up for freeto 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

@raboof@sbp@dave2wave
, '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

Oauth support client id and secret - #107

Open
raboof wants to merge 3 commits into
apache:mainfrom
raboof:oauth-support-client-id-and-secret
Open

Oauth support client id and secret#107
raboof wants to merge 3 commits into
apache:mainfrom
raboof:oauth-support-client-id-and-secret

Conversation

@raboof

@raboofraboof commented Jun 10, 2026

Copy link
Copy Markdown
Member

The initial motivation to support this was to support authentication against a local test oauth server instead of the deployed server.

This might also be usable to authenticate against Authentik directly instead of going via oauth.apache.org . That possibly provides the oauth_data information in a different format, though, so that might need some massaging either here or in the configuration.

Draft but interested in feedback on the general idea/approach!

@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch from d4e3a95 to 12ef88cCompareJune 11, 2026 13:01
@raboof
raboof marked this pull request as draft June 11, 2026 13:08
Comment threadsrc/asfquart/generics.py Outdated
token = await rv.json()
jwks_client = PyJWKClient(OAUTH_URL_JWKS)
id_token = token["id_token"]
signing_key = jwks_client.get_signing_key_from_jwt(id_token)

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.

Pretty sure that get_signing_key_from_jwt is blocking (synchronous), but this is an async function.

Comment threadsrc/asfquart/generics.py Outdated
# someplace else. This counts as a samesite request.
return quart.Response(
status=200,
response=f"Successfully logged in! Welcome, {oauth_data['uid']}\n",

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.

I don't think this would work in the new mode because OAuth 2.0 uses sub:

sub
REQUIRED. Subject Identifier. A locally unique and never reassigned identifier within the Issuer for the End-User, which is intended to be consumed by the Client, e.g., 24400320 or AItOawmwtWwcT0k51BayewNvutrJUqsvl6qs7A4. It MUST NOT exceed 255 ASCII [RFC20] characters in length. The sub value is a case-sensitive string.

Maybe we could have a test or two to ensure that it works?

Comment threadsrc/asfquart/generics.py Outdated
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch 2 times, most recently from 5c42e65 to 2acd3eeCompareJuly 23, 2026 07:40
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch 8 times, most recently from 5377085 to 0366feaCompareAugust 5, 2026 15:07
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch 7 times, most recently from e66e09a to 9f0e633CompareAugust 20, 2026 09:12
raboof added a commit to raboof/apache-security-dash that referenced this pull request Aug 20, 2026
This allows using 'official' OAuth2/OIDC rather than the ASF dialect,
and enables more detailed logging from mfa.apache.org
requires apache/infrastructure-asfquart#107
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch from 9f0e633 to 10ed333CompareAugust 20, 2026 09:27
@raboof

Copy link
Copy Markdown
MemberAuthor

Thanks for the feedback here and on slack. Made some updates and tested with apache/security-dash#6 , ready for another round of review.

@raboof
raboof marked this pull request as ready for review August 20, 2026 09:30
self.mfa = raw_data.get("mfa", False)
self.isRole = raw_data.get("roleaccount", False)
self.metadata = raw_data.get("metadata", {}) # This can contain whatever specific metadata the app needs
if "sub" in raw_data:

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.

In this branch, several properties (isChair, isRole, dn, metadata) aren't set, but auth.py:42 reads client_session.isChair, for example, and auth.py:60 reads client_session.isRole, so this will crash if they are used, e.g. through @require(Requirements.chair) or Requirements.roleaccount. The consequence would be a 500 due to the AttributeError.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Yes, that's semi-intentional: AFAICT the OAuth profile data currently doesn't give us a way to tell whether this is a chair or role account, so AttributeError seems like it'd be preferable to making an assumption here. For reference, for me it looks like:

{
"iss": "https://mfa-dev.apache.org/application/o/local_testing-apache-org/",
"sub": "5d96dad49cb7590a117ef6de7f2506279802a87433ee34d1aa29c76db69e9a5b",
"aud": "local_testing.apache.org",
"exp": 1787239136,
"iat": 1787238536,
"auth_time": 1787125488,
"acr": "goauthentik.io/providers/oauth2/default",
"amr": [
"mfa"
],
"sid": "e4754b0ffc4191ccaa0f95e0242eaf4f5e4a04eb273a452d35534254590db816",
"email": "engelen@apache.org",
"email_verified": false,
"name": "Arnout Engelen",
"given_name": "Arnout Engelen",
"preferred_username": "engelen",
"nickname": "engelen",
"groups": [
"infrastructure-root",
"committers",
"pekko",
"commons",
"security",
"infrastructure",
"member-meta",
"infrastructure-logging",
"geode",
"infrastructure-team",
"commons-pmc",
"security-pmc",
"pekko-pmc",
"incubator"
]
}

Comment threadsrc/asfquart/generics.py Outdated
Comment threadsrc/asfquart/session.py
When CLIENT_ID is populated, use the stricter OAuth2
implementation to authenticate directly to https://mfa.apache.org
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch from 10ed333 to 9f802d3CompareAugust 20, 2026 15:47
@raboof
raboof requested a review from sbpAugust 22, 2026 09:02
raboof added a commit to raboof/apache-security-dash that referenced this pull request Aug 27, 2026
This allows using 'official' OAuth2/OIDC rather than the ASF dialect,
and enables more detailed logging from mfa.apache.org
requires apache/infrastructure-asfquart#107
raboof added a commit to raboof/apache-security-dash that referenced this pull request Sep 1, 2026
This allows using 'official' OAuth2/OIDC rather than the ASF dialect,
and enables more detailed logging from mfa.apache.org
requires apache/infrastructure-asfquart#107
Sign up for freeto 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

@raboof@sbp@dave2wave
, '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

Oauth support client id and secret - #107

Open
raboof wants to merge 3 commits into
apache:mainfrom
raboof:oauth-support-client-id-and-secret
Open

Oauth support client id and secret#107
raboof wants to merge 3 commits into
apache:mainfrom
raboof:oauth-support-client-id-and-secret

Conversation

@raboof

@raboofraboof commented Jun 10, 2026

Copy link
Copy Markdown
Member

The initial motivation to support this was to support authentication against a local test oauth server instead of the deployed server.

This might also be usable to authenticate against Authentik directly instead of going via oauth.apache.org . That possibly provides the oauth_data information in a different format, though, so that might need some massaging either here or in the configuration.

Draft but interested in feedback on the general idea/approach!

@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch from d4e3a95 to 12ef88cCompareJune 11, 2026 13:01
@raboof
raboof marked this pull request as draft June 11, 2026 13:08
Comment threadsrc/asfquart/generics.py Outdated
token = await rv.json()
jwks_client = PyJWKClient(OAUTH_URL_JWKS)
id_token = token["id_token"]
signing_key = jwks_client.get_signing_key_from_jwt(id_token)

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.

Pretty sure that get_signing_key_from_jwt is blocking (synchronous), but this is an async function.

Comment threadsrc/asfquart/generics.py Outdated
# someplace else. This counts as a samesite request.
return quart.Response(
status=200,
response=f"Successfully logged in! Welcome, {oauth_data['uid']}\n",

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.

I don't think this would work in the new mode because OAuth 2.0 uses sub:

sub
REQUIRED. Subject Identifier. A locally unique and never reassigned identifier within the Issuer for the End-User, which is intended to be consumed by the Client, e.g., 24400320 or AItOawmwtWwcT0k51BayewNvutrJUqsvl6qs7A4. It MUST NOT exceed 255 ASCII [RFC20] characters in length. The sub value is a case-sensitive string.

Maybe we could have a test or two to ensure that it works?

Comment threadsrc/asfquart/generics.py Outdated
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch 2 times, most recently from 5c42e65 to 2acd3eeCompareJuly 23, 2026 07:40
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch 8 times, most recently from 5377085 to 0366feaCompareAugust 5, 2026 15:07
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch 7 times, most recently from e66e09a to 9f0e633CompareAugust 20, 2026 09:12
raboof added a commit to raboof/apache-security-dash that referenced this pull request Aug 20, 2026
This allows using 'official' OAuth2/OIDC rather than the ASF dialect,
and enables more detailed logging from mfa.apache.org
requires apache/infrastructure-asfquart#107
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch from 9f0e633 to 10ed333CompareAugust 20, 2026 09:27
@raboof

Copy link
Copy Markdown
MemberAuthor

Thanks for the feedback here and on slack. Made some updates and tested with apache/security-dash#6 , ready for another round of review.

@raboof
raboof marked this pull request as ready for review August 20, 2026 09:30
self.mfa = raw_data.get("mfa", False)
self.isRole = raw_data.get("roleaccount", False)
self.metadata = raw_data.get("metadata", {}) # This can contain whatever specific metadata the app needs
if "sub" in raw_data:

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.

In this branch, several properties (isChair, isRole, dn, metadata) aren't set, but auth.py:42 reads client_session.isChair, for example, and auth.py:60 reads client_session.isRole, so this will crash if they are used, e.g. through @require(Requirements.chair) or Requirements.roleaccount. The consequence would be a 500 due to the AttributeError.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Yes, that's semi-intentional: AFAICT the OAuth profile data currently doesn't give us a way to tell whether this is a chair or role account, so AttributeError seems like it'd be preferable to making an assumption here. For reference, for me it looks like:

{
"iss": "https://mfa-dev.apache.org/application/o/local_testing-apache-org/",
"sub": "5d96dad49cb7590a117ef6de7f2506279802a87433ee34d1aa29c76db69e9a5b",
"aud": "local_testing.apache.org",
"exp": 1787239136,
"iat": 1787238536,
"auth_time": 1787125488,
"acr": "goauthentik.io/providers/oauth2/default",
"amr": [
"mfa"
],
"sid": "e4754b0ffc4191ccaa0f95e0242eaf4f5e4a04eb273a452d35534254590db816",
"email": "engelen@apache.org",
"email_verified": false,
"name": "Arnout Engelen",
"given_name": "Arnout Engelen",
"preferred_username": "engelen",
"nickname": "engelen",
"groups": [
"infrastructure-root",
"committers",
"pekko",
"commons",
"security",
"infrastructure",
"member-meta",
"infrastructure-logging",
"geode",
"infrastructure-team",
"commons-pmc",
"security-pmc",
"pekko-pmc",
"incubator"
]
}

Comment threadsrc/asfquart/generics.py Outdated
Comment threadsrc/asfquart/session.py
When CLIENT_ID is populated, use the stricter OAuth2
implementation to authenticate directly to https://mfa.apache.org
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch from 10ed333 to 9f802d3CompareAugust 20, 2026 15:47
@raboof
raboof requested a review from sbpAugust 22, 2026 09:02
raboof added a commit to raboof/apache-security-dash that referenced this pull request Aug 27, 2026
This allows using 'official' OAuth2/OIDC rather than the ASF dialect,
and enables more detailed logging from mfa.apache.org
requires apache/infrastructure-asfquart#107
raboof added a commit to raboof/apache-security-dash that referenced this pull request Sep 1, 2026
This allows using 'official' OAuth2/OIDC rather than the ASF dialect,
and enables more detailed logging from mfa.apache.org
requires apache/infrastructure-asfquart#107
Sign up for freeto 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

@raboof@sbp@dave2wave
, '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

Oauth support client id and secret - #107

Open
raboof wants to merge 3 commits into
apache:mainfrom
raboof:oauth-support-client-id-and-secret
Open

Oauth support client id and secret#107
raboof wants to merge 3 commits into
apache:mainfrom
raboof:oauth-support-client-id-and-secret

Conversation

@raboof

@raboofraboof commented Jun 10, 2026

Copy link
Copy Markdown
Member

The initial motivation to support this was to support authentication against a local test oauth server instead of the deployed server.

This might also be usable to authenticate against Authentik directly instead of going via oauth.apache.org . That possibly provides the oauth_data information in a different format, though, so that might need some massaging either here or in the configuration.

Draft but interested in feedback on the general idea/approach!

@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch from d4e3a95 to 12ef88cCompareJune 11, 2026 13:01
@raboof
raboof marked this pull request as draft June 11, 2026 13:08
Comment threadsrc/asfquart/generics.py Outdated
token = await rv.json()
jwks_client = PyJWKClient(OAUTH_URL_JWKS)
id_token = token["id_token"]
signing_key = jwks_client.get_signing_key_from_jwt(id_token)

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.

Pretty sure that get_signing_key_from_jwt is blocking (synchronous), but this is an async function.

Comment threadsrc/asfquart/generics.py Outdated
# someplace else. This counts as a samesite request.
return quart.Response(
status=200,
response=f"Successfully logged in! Welcome, {oauth_data['uid']}\n",

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.

I don't think this would work in the new mode because OAuth 2.0 uses sub:

sub
REQUIRED. Subject Identifier. A locally unique and never reassigned identifier within the Issuer for the End-User, which is intended to be consumed by the Client, e.g., 24400320 or AItOawmwtWwcT0k51BayewNvutrJUqsvl6qs7A4. It MUST NOT exceed 255 ASCII [RFC20] characters in length. The sub value is a case-sensitive string.

Maybe we could have a test or two to ensure that it works?

Comment threadsrc/asfquart/generics.py Outdated
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch 2 times, most recently from 5c42e65 to 2acd3eeCompareJuly 23, 2026 07:40
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch 8 times, most recently from 5377085 to 0366feaCompareAugust 5, 2026 15:07
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch 7 times, most recently from e66e09a to 9f0e633CompareAugust 20, 2026 09:12
raboof added a commit to raboof/apache-security-dash that referenced this pull request Aug 20, 2026
This allows using 'official' OAuth2/OIDC rather than the ASF dialect,
and enables more detailed logging from mfa.apache.org
requires apache/infrastructure-asfquart#107
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch from 9f0e633 to 10ed333CompareAugust 20, 2026 09:27
@raboof

Copy link
Copy Markdown
MemberAuthor

Thanks for the feedback here and on slack. Made some updates and tested with apache/security-dash#6 , ready for another round of review.

@raboof
raboof marked this pull request as ready for review August 20, 2026 09:30
self.mfa = raw_data.get("mfa", False)
self.isRole = raw_data.get("roleaccount", False)
self.metadata = raw_data.get("metadata", {}) # This can contain whatever specific metadata the app needs
if "sub" in raw_data:

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.

In this branch, several properties (isChair, isRole, dn, metadata) aren't set, but auth.py:42 reads client_session.isChair, for example, and auth.py:60 reads client_session.isRole, so this will crash if they are used, e.g. through @require(Requirements.chair) or Requirements.roleaccount. The consequence would be a 500 due to the AttributeError.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Yes, that's semi-intentional: AFAICT the OAuth profile data currently doesn't give us a way to tell whether this is a chair or role account, so AttributeError seems like it'd be preferable to making an assumption here. For reference, for me it looks like:

{
"iss": "https://mfa-dev.apache.org/application/o/local_testing-apache-org/",
"sub": "5d96dad49cb7590a117ef6de7f2506279802a87433ee34d1aa29c76db69e9a5b",
"aud": "local_testing.apache.org",
"exp": 1787239136,
"iat": 1787238536,
"auth_time": 1787125488,
"acr": "goauthentik.io/providers/oauth2/default",
"amr": [
"mfa"
],
"sid": "e4754b0ffc4191ccaa0f95e0242eaf4f5e4a04eb273a452d35534254590db816",
"email": "engelen@apache.org",
"email_verified": false,
"name": "Arnout Engelen",
"given_name": "Arnout Engelen",
"preferred_username": "engelen",
"nickname": "engelen",
"groups": [
"infrastructure-root",
"committers",
"pekko",
"commons",
"security",
"infrastructure",
"member-meta",
"infrastructure-logging",
"geode",
"infrastructure-team",
"commons-pmc",
"security-pmc",
"pekko-pmc",
"incubator"
]
}

Comment threadsrc/asfquart/generics.py Outdated
Comment threadsrc/asfquart/session.py
When CLIENT_ID is populated, use the stricter OAuth2
implementation to authenticate directly to https://mfa.apache.org
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch from 10ed333 to 9f802d3CompareAugust 20, 2026 15:47
@raboof
raboof requested a review from sbpAugust 22, 2026 09:02
raboof added a commit to raboof/apache-security-dash that referenced this pull request Aug 27, 2026
This allows using 'official' OAuth2/OIDC rather than the ASF dialect,
and enables more detailed logging from mfa.apache.org
requires apache/infrastructure-asfquart#107
raboof added a commit to raboof/apache-security-dash that referenced this pull request Sep 1, 2026
This allows using 'official' OAuth2/OIDC rather than the ASF dialect,
and enables more detailed logging from mfa.apache.org
requires apache/infrastructure-asfquart#107
Sign up for freeto 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

@raboof@sbp@dave2wave
, '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

Oauth support client id and secret - #107

Open
raboof wants to merge 3 commits into
apache:mainfrom
raboof:oauth-support-client-id-and-secret
Open

Oauth support client id and secret#107
raboof wants to merge 3 commits into
apache:mainfrom
raboof:oauth-support-client-id-and-secret

Conversation

@raboof

@raboofraboof commented Jun 10, 2026

Copy link
Copy Markdown
Member

The initial motivation to support this was to support authentication against a local test oauth server instead of the deployed server.

This might also be usable to authenticate against Authentik directly instead of going via oauth.apache.org . That possibly provides the oauth_data information in a different format, though, so that might need some massaging either here or in the configuration.

Draft but interested in feedback on the general idea/approach!

@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch from d4e3a95 to 12ef88cCompareJune 11, 2026 13:01
@raboof
raboof marked this pull request as draft June 11, 2026 13:08
Comment threadsrc/asfquart/generics.py Outdated
token = await rv.json()
jwks_client = PyJWKClient(OAUTH_URL_JWKS)
id_token = token["id_token"]
signing_key = jwks_client.get_signing_key_from_jwt(id_token)

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.

Pretty sure that get_signing_key_from_jwt is blocking (synchronous), but this is an async function.

Comment threadsrc/asfquart/generics.py Outdated
# someplace else. This counts as a samesite request.
return quart.Response(
status=200,
response=f"Successfully logged in! Welcome, {oauth_data['uid']}\n",

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.

I don't think this would work in the new mode because OAuth 2.0 uses sub:

sub
REQUIRED. Subject Identifier. A locally unique and never reassigned identifier within the Issuer for the End-User, which is intended to be consumed by the Client, e.g., 24400320 or AItOawmwtWwcT0k51BayewNvutrJUqsvl6qs7A4. It MUST NOT exceed 255 ASCII [RFC20] characters in length. The sub value is a case-sensitive string.

Maybe we could have a test or two to ensure that it works?

Comment threadsrc/asfquart/generics.py Outdated
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch 2 times, most recently from 5c42e65 to 2acd3eeCompareJuly 23, 2026 07:40
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch 8 times, most recently from 5377085 to 0366feaCompareAugust 5, 2026 15:07
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch 7 times, most recently from e66e09a to 9f0e633CompareAugust 20, 2026 09:12
raboof added a commit to raboof/apache-security-dash that referenced this pull request Aug 20, 2026
This allows using 'official' OAuth2/OIDC rather than the ASF dialect,
and enables more detailed logging from mfa.apache.org
requires apache/infrastructure-asfquart#107
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch from 9f0e633 to 10ed333CompareAugust 20, 2026 09:27
@raboof

Copy link
Copy Markdown
MemberAuthor

Thanks for the feedback here and on slack. Made some updates and tested with apache/security-dash#6 , ready for another round of review.

@raboof
raboof marked this pull request as ready for review August 20, 2026 09:30
self.mfa = raw_data.get("mfa", False)
self.isRole = raw_data.get("roleaccount", False)
self.metadata = raw_data.get("metadata", {}) # This can contain whatever specific metadata the app needs
if "sub" in raw_data:

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.

In this branch, several properties (isChair, isRole, dn, metadata) aren't set, but auth.py:42 reads client_session.isChair, for example, and auth.py:60 reads client_session.isRole, so this will crash if they are used, e.g. through @require(Requirements.chair) or Requirements.roleaccount. The consequence would be a 500 due to the AttributeError.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Yes, that's semi-intentional: AFAICT the OAuth profile data currently doesn't give us a way to tell whether this is a chair or role account, so AttributeError seems like it'd be preferable to making an assumption here. For reference, for me it looks like:

{
"iss": "https://mfa-dev.apache.org/application/o/local_testing-apache-org/",
"sub": "5d96dad49cb7590a117ef6de7f2506279802a87433ee34d1aa29c76db69e9a5b",
"aud": "local_testing.apache.org",
"exp": 1787239136,
"iat": 1787238536,
"auth_time": 1787125488,
"acr": "goauthentik.io/providers/oauth2/default",
"amr": [
"mfa"
],
"sid": "e4754b0ffc4191ccaa0f95e0242eaf4f5e4a04eb273a452d35534254590db816",
"email": "engelen@apache.org",
"email_verified": false,
"name": "Arnout Engelen",
"given_name": "Arnout Engelen",
"preferred_username": "engelen",
"nickname": "engelen",
"groups": [
"infrastructure-root",
"committers",
"pekko",
"commons",
"security",
"infrastructure",
"member-meta",
"infrastructure-logging",
"geode",
"infrastructure-team",
"commons-pmc",
"security-pmc",
"pekko-pmc",
"incubator"
]
}

Comment threadsrc/asfquart/generics.py Outdated
Comment threadsrc/asfquart/session.py
When CLIENT_ID is populated, use the stricter OAuth2
implementation to authenticate directly to https://mfa.apache.org
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch from 10ed333 to 9f802d3CompareAugust 20, 2026 15:47
@raboof
raboof requested a review from sbpAugust 22, 2026 09:02
raboof added a commit to raboof/apache-security-dash that referenced this pull request Aug 27, 2026
This allows using 'official' OAuth2/OIDC rather than the ASF dialect,
and enables more detailed logging from mfa.apache.org
requires apache/infrastructure-asfquart#107
raboof added a commit to raboof/apache-security-dash that referenced this pull request Sep 1, 2026
This allows using 'official' OAuth2/OIDC rather than the ASF dialect,
and enables more detailed logging from mfa.apache.org
requires apache/infrastructure-asfquart#107
Sign up for freeto 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

@raboof@sbp@dave2wave
, '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

Oauth support client id and secret - #107

Open
raboof wants to merge 3 commits into
apache:mainfrom
raboof:oauth-support-client-id-and-secret
Open

Oauth support client id and secret#107
raboof wants to merge 3 commits into
apache:mainfrom
raboof:oauth-support-client-id-and-secret

Conversation

@raboof

@raboofraboof commented Jun 10, 2026

Copy link
Copy Markdown
Member

The initial motivation to support this was to support authentication against a local test oauth server instead of the deployed server.

This might also be usable to authenticate against Authentik directly instead of going via oauth.apache.org . That possibly provides the oauth_data information in a different format, though, so that might need some massaging either here or in the configuration.

Draft but interested in feedback on the general idea/approach!

@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch from d4e3a95 to 12ef88cCompareJune 11, 2026 13:01
@raboof
raboof marked this pull request as draft June 11, 2026 13:08
Comment threadsrc/asfquart/generics.py Outdated
token = await rv.json()
jwks_client = PyJWKClient(OAUTH_URL_JWKS)
id_token = token["id_token"]
signing_key = jwks_client.get_signing_key_from_jwt(id_token)

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.

Pretty sure that get_signing_key_from_jwt is blocking (synchronous), but this is an async function.

Comment threadsrc/asfquart/generics.py Outdated
# someplace else. This counts as a samesite request.
return quart.Response(
status=200,
response=f"Successfully logged in! Welcome, {oauth_data['uid']}\n",

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.

I don't think this would work in the new mode because OAuth 2.0 uses sub:

sub
REQUIRED. Subject Identifier. A locally unique and never reassigned identifier within the Issuer for the End-User, which is intended to be consumed by the Client, e.g., 24400320 or AItOawmwtWwcT0k51BayewNvutrJUqsvl6qs7A4. It MUST NOT exceed 255 ASCII [RFC20] characters in length. The sub value is a case-sensitive string.

Maybe we could have a test or two to ensure that it works?

Comment threadsrc/asfquart/generics.py Outdated
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch 2 times, most recently from 5c42e65 to 2acd3eeCompareJuly 23, 2026 07:40
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch 8 times, most recently from 5377085 to 0366feaCompareAugust 5, 2026 15:07
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch 7 times, most recently from e66e09a to 9f0e633CompareAugust 20, 2026 09:12
raboof added a commit to raboof/apache-security-dash that referenced this pull request Aug 20, 2026
This allows using 'official' OAuth2/OIDC rather than the ASF dialect,
and enables more detailed logging from mfa.apache.org
requires apache/infrastructure-asfquart#107
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch from 9f0e633 to 10ed333CompareAugust 20, 2026 09:27
@raboof

Copy link
Copy Markdown
MemberAuthor

Thanks for the feedback here and on slack. Made some updates and tested with apache/security-dash#6 , ready for another round of review.

@raboof
raboof marked this pull request as ready for review August 20, 2026 09:30
self.mfa = raw_data.get("mfa", False)
self.isRole = raw_data.get("roleaccount", False)
self.metadata = raw_data.get("metadata", {}) # This can contain whatever specific metadata the app needs
if "sub" in raw_data:

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.

In this branch, several properties (isChair, isRole, dn, metadata) aren't set, but auth.py:42 reads client_session.isChair, for example, and auth.py:60 reads client_session.isRole, so this will crash if they are used, e.g. through @require(Requirements.chair) or Requirements.roleaccount. The consequence would be a 500 due to the AttributeError.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Yes, that's semi-intentional: AFAICT the OAuth profile data currently doesn't give us a way to tell whether this is a chair or role account, so AttributeError seems like it'd be preferable to making an assumption here. For reference, for me it looks like:

{
"iss": "https://mfa-dev.apache.org/application/o/local_testing-apache-org/",
"sub": "5d96dad49cb7590a117ef6de7f2506279802a87433ee34d1aa29c76db69e9a5b",
"aud": "local_testing.apache.org",
"exp": 1787239136,
"iat": 1787238536,
"auth_time": 1787125488,
"acr": "goauthentik.io/providers/oauth2/default",
"amr": [
"mfa"
],
"sid": "e4754b0ffc4191ccaa0f95e0242eaf4f5e4a04eb273a452d35534254590db816",
"email": "engelen@apache.org",
"email_verified": false,
"name": "Arnout Engelen",
"given_name": "Arnout Engelen",
"preferred_username": "engelen",
"nickname": "engelen",
"groups": [
"infrastructure-root",
"committers",
"pekko",
"commons",
"security",
"infrastructure",
"member-meta",
"infrastructure-logging",
"geode",
"infrastructure-team",
"commons-pmc",
"security-pmc",
"pekko-pmc",
"incubator"
]
}

Comment threadsrc/asfquart/generics.py Outdated
Comment threadsrc/asfquart/session.py
When CLIENT_ID is populated, use the stricter OAuth2
implementation to authenticate directly to https://mfa.apache.org
@raboof
raboofforce-pushed the oauth-support-client-id-and-secret branch from 10ed333 to 9f802d3CompareAugust 20, 2026 15:47
@raboof
raboof requested a review from sbpAugust 22, 2026 09:02
raboof added a commit to raboof/apache-security-dash that referenced this pull request Aug 27, 2026
This allows using 'official' OAuth2/OIDC rather than the ASF dialect,
and enables more detailed logging from mfa.apache.org
requires apache/infrastructure-asfquart#107
raboof added a commit to raboof/apache-security-dash that referenced this pull request Sep 1, 2026
This allows using 'official' OAuth2/OIDC rather than the ASF dialect,
and enables more detailed logging from mfa.apache.org
requires apache/infrastructure-asfquart#107
Sign up for freeto 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

@raboof@sbp@dave2wave