Add an Output io.Writer so auth flow output can be redirected - #92
Merged
Merged
Conversation
…figs so consumers can redirect (or silence, via io.Discard) the progress messages the default handlers print, instead of having output hardwired to stdout. Custom handlers still own their output entirely whenever one is supplied.
| ) | ||
|
|
||
| // A non-http(s) scheme is used for every URL below so that browser.Open rejects it during URL | ||
| // validation, before it would otherwise shell out to open a real browser window. |
Contributor
There was a problem hiding this comment.
Our pingcli tests do this (opening a browser), can we follow this same pattern there to avoid that?
Also, it would be best to point these at a localhost/127.0.0.1 instead of a domain we don't own
- Invert the Output default: the SDK no longer prints to stdout at all — progress output is opt-in. nil/omitted Output disables it entirely; os.Stdout (via the With*Output builders or *To(w)) reproduces the interactive v1300.1.0 behavior. - Point every auth-flow test URL at 127.0.0.1 instead of external domains, and state in the test comments (pingcli-style) that these tests do NOT open browsers — browser.Open rejects the non-http(s) scheme during validation, before any browser could launch.
The extension points have no consumers: the only repo requiring this SDK (terraform-provider-pingfederate) pins v1300.0.0, which predates them. Follow-up to the review feedback — with output now opt-in via Output, the override is redundant and unused. The flows now always use the default handler through *To(output). Tests that relied on a failing handler to exercise flow paths now occupy the default redirect port instead, so they fail at callback-server startup before the browser step.
henryrecker-pingidentity
approved these changes
Sep 10, 2026
This was referenced Sep 11, 2026
Merged
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
2 participants
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
Adds an
Output io.Writerfield to the authorization-code and device-code configs so consumers can route (or silence, viaio.Discard) the progress messages the default handlers write, instead of having that output locked to stdout. Hand-rolled handler implementations continue to own their output completely whenever one is supplied.Changes
config/config_oauth.go: newOutputfield on the authorization-code configconfig/config_oauth_test.go: coverage for redirecting output and forio.DiscardREADME.md: document the new fieldTesting
go test ./...go vet ./...