Fix Azure DevOps Keycloak PKCE config#676
Merged
Merged
Conversation
Contributor
|
✅ Review complete. This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here. |
all-hands-bot
approved these changes
May 29, 2026
Contributor
all-hands-bot
left a comment
There was a problem hiding this comment.
Code Review: Fix Azure DevOps Keycloak PKCE config
Clean, well-scoped fix. The root cause analysis in the PR description is clear, and all three changes (realm template, error-guard script, and version bump) are minimal and correct. The new test suite adds solid regression coverage for both the PKCE invariant and the error-message detection logic.
Two minor inline observations below (neither is a blocker).
This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation
…oak-pkce # Conflicts: # charts/openhands/Chart.yaml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
pkceMethod: S256to the Azure DevOps Keycloak identity provider configkeycloak_api_callfail on Keycloak Admin APIerrorMessageresponses, not onlyerror0.7.36Root Cause
The Azure DevOps IdP was added with
pkceEnabled: "true"but nopkceMethod. Keycloak rejects that withPKCE Method not supported: null.On fresh installs, the full realm import fails loudly. On upgrades, the per-resource identity provider create call returns an
errorMessageresponse, but the script only checked.error, so it logged a false-positive create and then skipped Azure mappers after a 404.Validation
jq empty charts/openhands/files/allhands-realm-github-provider.json.tmplpkceEnabled == "true"withoutpkceMethoderrorMessagevalueshelm template openhands charts/openhands --set enabled=true --set keycloak.enabled=truehelm lint charts/openhandsmake manifestsmake chartsmake lintpassed after one retry; first attempt failed on a transient Docker Hub OCI connection resetLive replicated-02 validation:
pkceMethod; Keycloak returned404forazure_devops; init log showed false-positiveCreated identity provider: azure_devopsfollowed by mapper 404keycloak-configexited 0, createdazure_devops, created both Azure mappers, and Keycloak returnedpkceMethod: S256for the Azure IdPNote: the replicated-02 live ConfigMap patch was only for validation; the durable fix is this chart change.