Skip to content

KNOX-3312 - Client Credentials Flow with HTTP Basic needs Unwrapped S…#1219

Open
lmccay wants to merge 2 commits intoapache:masterfrom
lmccay:KNOX-3312
Open

KNOX-3312 - Client Credentials Flow with HTTP Basic needs Unwrapped S…#1219
lmccay wants to merge 2 commits intoapache:masterfrom
lmccay:KNOX-3312

Conversation

@lmccay
Copy link
Copy Markdown
Contributor

@lmccay lmccay commented May 2, 2026

KNOX-1234 - Client Credentials Flow with HTTP Basic needs Unwrapped Servlet Request

What changes were proposed in this pull request?

Current implementation can't get to the grant_type request param.

Unit tests mock out the requests and make it hard to tease this out as an issue.

When we know that there is an Authorization header and that it is Basic then we need to check whether there is the hardcoded username of token or passcode and if not, unwrap the request to check for a grant_type for OAuth client_credentials and handle it appropriately.

Current implementation tries to check that but the params are hidden by the wrappers.

How was this patch tested?

All existing unit and integration tests were run.
Manually tested the flow.

Also added a request wrapper to the unit tests of the mock http request.
This ensures that the access to the request params for OAuth flows does not regress.

@lmccay lmccay marked this pull request as draft May 2, 2026 14:11
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 2, 2026

Test Results

21 tests   21 ✅  1s ⏱️
 1 suites   0 💤
 1 files     0 ❌

Results for commit 4183f70.

♻️ This comment has been updated with latest results.

@lmccay lmccay marked this pull request as ready for review May 2, 2026 16:46
@lmccay lmccay requested review from pzampino and smolnar82 May 2, 2026 16:46
Copy link
Copy Markdown
Contributor

@pzampino pzampino left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants