fix: allow POST for connector redirect route - #1612
Open
DanielAuerX wants to merge 2 commits into
Open
DanielAuerX wants to merge 2 commits into
DanielAuerX wants to merge 2 commits into
Conversation
4 tasks
Connector plugins that collect credentials in a login form (e.g. an LDAP connector) need to submit them via POST rather than GET, since GET params get logged in server/proxy access logs and browser history. ConnectorRedirectDispatcher itself is already method-agnostic (it just reads ctx.Request), so this only needed a route registration change.
DanielAuerX
force-pushed
the
fix/connector-redirect-post-route
branch
from
September 15, 2026 15:54
11c7468 to
43d4508
Compare
+ added an optional ConnectorStateRequired interface. Connectors that implement it now get their consumed state checked in ConnectorRedirect. A missing or invalid state is rejected. Its optional, so no existing connectors behavior changes unless it opts in. + required by the LDAP connector, whose login form is a same-site POST rather than an external OAuth redirect, so state doesn't naturally round-trip without it
DanielAuerX
added a commit
to DanielAuerX/answer-plugins
that referenced
this pull request
Sep 17, 2026
+ ConnectorSender now reads state via ctx.Request.URL.Query() instead of ctx.Query(), which returns a stale cached value here since core injects state into the raw query after already having called ctx.Query() once. appends it to the receiver url so it survives the login forms post. + implemented ConnectorRequireState() to opt into cores strict state validation (cf pr apache/answer#1612). missing or invalid state is now rejected instead of treated as a normal login. + ExternalID now comes from a configurable, stable attribute (default entryUUID; objectGUID supported for Active Directory, including proper binary/mixed-endian decoding) instead of the mutable uid or sAMAccountName. + added tests (mocked dependencies) + dialWithTLS now returns the ldap.Client interface instead of the concrete *ldap.Conn enabling a mock ldap client in tests
DanielAuerX
added a commit
to DanielAuerX/answer-plugins
that referenced
this pull request
Sep 17, 2026
+ ConnectorSender now reads state via ctx.Request.URL.Query() instead of ctx.Query(), which returns a stale cached value here since core injects state into the raw query after already having called ctx.Query() once. appends it to the receiver url so it survives the login forms post. + implemented ConnectorRequireState() to opt into cores strict state validation (cf pr apache/answer#1612). missing or invalid state is now rejected instead of treated as a normal login. + ExternalID now comes from a configurable, stable attribute (default entryUUID; objectGUID supported for Active Directory, including proper binary/mixed-endian decoding) instead of the mutable uid or sAMAccountName. + added tests (mocked dependencies) + dialWithTLS now returns the ldap.Client interface instead of the concrete *ldap.Conn enabling a mock ldap client in tests
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
1 participant
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.
Connector plugins that collect credentials in a login form (e.g. an LDAP connector) need to submit them via POST rather than GET.
ConnectorRedirectDispatcher itself is already method-agnostic (it just reads ctx.Request), so this only needed a route registration change.
Related to apache/answer-plugins#294 (LDAP connector), which needs this to submit login credentials safely.