-
-
Notifications
You must be signed in to change notification settings - Fork 87
Fix OAuth2 Basic Authentication Base64 Encoding Issue #825
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix OAuth2 Basic Authentication Base64 Encoding Issue #825
Conversation
- Add base64_encode_standard() function for OAuth2 Basic auth headers - Keep base64_encode() as URL-safe for PKCE and JWT use cases - Update oauth.lua to use standard Base64 for Authorization: Basic headers This fixes the OAuth2 client credentials authentication issue where kulala was incorrectly using URL-safe Base64 encoding (+ -> -, / -> _) instead of standard Base64 encoding as required by RFC 6749 Section 2.3.1. Fixes authentication failures with OAuth2 providers that strictly validate Base64 encoding in Authorization: Basic headers.
…ntials - Add unit tests for base64_encode_standard() and base64_encode() functions - Add integration tests for OAuth2 Basic auth with safe test credentials - Add regression test ensuring PKCE still uses URL-safe encoding All test credentials are randomly generated safe values that produce the required Base64 patterns (+ and / characters) without exposing real secrets. Tests verify: - Standard Base64 encoding for OAuth2 Basic auth (fixes "invalid_client" errors) - URL-safe Base64 encoding still works for PKCE/JWT (no regression) - Proper character substitutions: + vs -, / vs _, padding handling This prevents regression of the OAuth2 client credentials authentication bug where kulala incorrectly used URL-safe Base64 instead of standard Base64 for Authorization: Basic headers per RFC 7617. Co-Authored-By: Claude (claude-sonnet-4) <noreply@anthropic.com>
|
Oh, good catch! Thank you for the PR. |
The existing test expected URL-safe Base64 encoding (Y2xpZW50X2lkOmNsaWVudF9zZWNyZXQ) but the fix correctly changed OAuth2 Basic auth to use standard Base64 encoding (Y2xpZW50X2lkOmNsaWVudF9zZWNyZXQ=) per RFC 7617. For the test credentials "client_id:client_secret", the only difference is padding (= character) since this string doesn't produce + or / characters. This fixes the CI test failure caused by the Base64 encoding fix. Co-Authored-By: Claude (claude-sonnet-4) <noreply@anthropic.com>
b73ddc0 to
a365437
Compare
… env The test failures were caused by Client Secret values being overridden by the private environment configuration. The update_env() function requires a second parameter `true` to update the private environment where "Client Secret" is defined. Changes: - Update "Client Secret" in private environment for + character test - Update "Client Secret" in private environment for / character test - Keep "Client ID" and other settings in public environment This ensures the test credentials (test:> and user123:?pass) are used instead of the default client_secret, allowing proper testing of Base64 character handling. Co-Authored-By: Claude (claude-sonnet-4) <noreply@anthropic.com>
a365437 to
29d9ab5
Compare
b77f3b0 to
062f1d4
Compare
|
Sure, I'm very happy to contribute :) |
|
No problem, that's what CI is for. You can also download our test docker image and run it locally if you want. |
|
Yeah I got it to work now. But now the tests are already green. |
In CI they also pass. Thanks 🙏🏾👍🏾 |
* release 5.3.4 * feat(ui): add `max_request_size` opt * Fix OAuth2 Basic Authentication Base64 Encoding Issue (#825) * fix: use standard Base64 encoding for OAuth2 Basic authentication * fix(lsp): do not execute lsp.folding if parser is not loaded --------- Co-authored-by: 🚀 Niklas Arens <niklas.arens@mercedes-benz.com> Co-authored-by: Marco Kellershoff <1384938+gorillamoe@users.noreply.github.com>
Description
Fixes OAuth2 client credentials authentication failure by using standard Base64 encoding instead of URL-safe Base64 for
Authorization: Basicheaders.Problem: kulala.nvim was incorrectly using URL-safe Base64 encoding (
+→-,/→_) for HTTP Basic authentication headers, violating RFC 7617 which requires standard Base64 encoding. This causedinvalid_clienterrors with (some/strict) OAuth2 providers when client credentials contained+or/characters in their Base64 representation.Solution: Added
base64_encode_standard()function for OAuth2 Basic authentication while preserving existing URL-safe encoding for PKCE/JWT use cases.Evidence:
+//in Base64 worked by coincidence+//failed authentication (e.g., position 83:+vs-)Files changed:
lua/kulala/cmd/crypto.lua: Addedbase64_encode_standard()functionlua/kulala/cmd/oauth.lua: Updated OAuth2 Basic auth to use standard encodingNote
Please open your PR against
developbranch. It will be merged into develop and in ~5-7 days merged into mainas part of
Weekly updates PR.Type of Change
Checklist
./scripts/lint.sh check-code)make test)./scripts/lint.sh check-docs)Note
Neovim help files will be generated automatically from
.mddocumentation, so no need to edit.txtfiles manually.If this PR includes tree-sitter grammar changes:
n.a.
Related Issues
This PR addresses OAuth2 authentication failures reported by users where client credentials authentication would fail with
invalid_clienterrors despite correct credentials.References: