support storage name suffix, add storage type enums #58
Conversation
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
- Delete oauth2/device.go containing commented-out device code implementation - Delete oauth2/pkce.go containing commented-out PKCE implementation - Remove PKCE tests (functionality now uses golang.org/x/oauth2 built-in support) - Add debug output to device_auth.go for troubleshooting These files contained experimental/artifact code that was never used. Auth code flow uses oauth2.GenerateVerifier() and oauth2.S256ChallengeOption() from the standard library instead of custom PKCE implementation.
Implements OAuth2 authorization code flow with PKCE for interactive authentication workflows. Features: - Authorization code flow with local callback server - PKCE (Proof Key for Code Exchange) support - Automatic browser opening for authorization - Branded HTML templates for callback pages - Configurable redirect URI (defaults to http://localhost:8080/callback) - Scope configuration support Technical: - Embedded HTML templates using go:embed - Local HTTP server for OAuth2 callback handling - Cross-platform browser launching (macOS, Linux, Windows) - Proper error handling and user feedback This enables interactive authentication flows where users authenticate via browser while the CLI waits for the OAuth2 callback.
New files: - config/client_credentials_test.go: 15 test cases achieving 100% coverage - config/device_auth_test.go: 7 test cases for device code flow validation Enhanced files: - config/auth_code_test.go: * Added 7 new test cases (scopes, custom redirect URI, callbacks) * Added context.WithTimeout() to prevent hanging on OAuth callbacks * Added callback error handling tests (missing code, no description) * Reduced timeout from 1s to 100ms for faster test execution
* add authCode variables for default redirect URI values * update AuthCodeRedirectURI json key from redirectUri to authCodeRedirectUri * move TopLevelDomain from oauth2 package to config package * remove StorageTypeFile, add StorageTypeNone * correct keychain storage method calls to use Storage.Name from struct * require storage name validation when using keychain storage * add copyright headers to config files * add top_level_domain_test.go with comprehensive test coverage
* update Storage Name to KeychainName * add StorageTypeNone to IsValid check
…sfully exchanged, display failure on token exchange error
… authorization code values, replace auth grant type environment ids with endpoints environment id
…s, add examples for authorization code and device code, update example for client credentials
Implement automatic access token refresh for device_code and authorization_code flows using refresh tokens. Tokens are automatically refreshed when expired and persisted to keychain storage via custom keychainTokenSource wrapper.
…back page (#49) * Add 3-second auto-close to authorization callback page - Automatically close the browser window 3 seconds after successful authorization - Improves user experience by removing manual window closure step - Uses setTimeout with window.close() for clean automatic cleanup * Add countdown timer and conditional auto-close for auth callback - Only auto-close window on successful authorization (not on errors) - Display countdown message: 'This window will close in X seconds...' - Countdown updates every second from 3 to 1 - Shows 'Closing now...' before final close - Failed authorization pages remain open for user review * Display error details on authorization failure page - Add ErrorDetails field to failed page template data - Pass specific error messages to returnFailedPage function - Show OAuth2 callback errors (from service error/error_description) - Show token exchange errors with full error details - Display timeout errors when token exchange takes too long - Style error details with red background box for visibility - Keep error page open (no auto-close) for user review * Fix success page template by adding ErrorDetails field - Add empty ErrorDetails field to success page data structure - Prevents template execution error when checking {{if .ErrorDetails}} - Ensures countdown timer displays correctly on success * Fix OAuth callback error handling timing issue - Increase HTTP response delay from 500ms to 1 second - Ensures HTTP error response is fully sent before server shutdown - Prevents 'connection refused' when browser receives error callback - Applies to authorization callback errors, token exchange errors, and success pages * Add top padding to closeMessage for better spacing - Add 24px margin-top to #closeMessage - Creates better visual separation from error box or subtitle above
* update html page wording
* Change local server host and port
* Add 3-second auto-close to authorization callback page
- Automatically close the browser window 3 seconds after successful authorization
- Improves user experience by removing manual window closure step
- Uses setTimeout with window.close() for clean automatic cleanup
* Add countdown timer and conditional auto-close for auth callback
- Only auto-close window on successful authorization (not on errors)
- Display countdown message: 'This window will close in X seconds...'
- Countdown updates every second from 3 to 1
- Shows 'Closing now...' before final close
- Failed authorization pages remain open for user review
* Display error details on authorization failure page
- Add ErrorDetails field to failed page template data
- Pass specific error messages to returnFailedPage function
- Show OAuth2 callback errors (from service error/error_description)
- Show token exchange errors with full error details
- Display timeout errors when token exchange takes too long
- Style error details with red background box for visibility
- Keep error page open (no auto-close) for user review
* Fix success page template by adding ErrorDetails field
- Add empty ErrorDetails field to success page data structure
- Prevents template execution error when checking {{if .ErrorDetails}}
- Ensures countdown timer displays correctly on success
* Fix OAuth callback error handling timing issue
- Increase HTTP response delay from 500ms to 1 second
- Ensures HTTP error response is fully sent before server shutdown
- Prevents 'connection refused' when browser receives error callback
- Applies to authorization callback errors, token exchange errors, and success pages
* Add top padding to closeMessage for better spacing
- Add 24px margin-top to #closeMessage
- Creates better visual separation from error box or subtitle above
…ows (#50) * Make full URL primary option for device authorization flow - Change device auth flow to prioritize complete verification URL - Show verification_uri_complete as the primary option when available - Provide alternative manual flow (visit URI + enter code) as fallback - Maintain backward compatibility for servers without complete URL support * Add browser detection and auto-open for device authorization - Create internal/browser package for browser utilities - Implement CanOpen() to detect if browser can be opened - Implement Open() to launch default browser with error handling - Update device auth flow to auto-open browser when available - Refactor device auth messaging logic for better maintainability - Update authorization code flow to use internal/browser package - Move browser functions from config package to internal for proper encapsulation * test: Add comprehensive test coverage for browser package - Add unit tests for CanOpen, validateAndSanitizeURL, and Open functions - Test URL validation including security checks (scheme validation, credential rejection, control characters) - Test command injection prevention (validates that shell metacharacters are handled safely) - Test concurrent validation to ensure thread safety - All tests designed to work in CI environments without browser dependencies - Tests skip actual browser opening, focusing on validation logic - Achieve 71.7% test coverage (93.3% for URL validation logic) The tests validate security properties: - Only http/https schemes allowed - Embedded credentials rejected - URL length limits enforced - Control characters in URLs rejected - Command injection attempts safely handled via exec.CommandContext with separate arguments * fixes * feat: add handler pattern for headless and custom UX authentication - Add DeviceCodePromptHandler function type for custom device code display - Add AuthURLHandler function type for custom browser opening logic - Add CustomHTMLSuccess and CustomHTMLError fields for custom callback pages - Update DeviceAuthTokenSource to use OnDisplayPrompt handler if provided - Update AuthorizationCodeTokenSource to use OnOpenBrowser handler if provided - Update returnSuccessPage and returnFailedPage to accept custom HTML - Maintain backward compatibility with default console output and browser opening This enables headless operation for containerized apps and custom UX for CLI tools. Implements handler pattern (Inversion of Control) for presentation layer. * refactor: make browser package public for reuse by consumers - Move internal/browser to public browser package - Update package documentation to describe public API - Update imports in config/device_auth.go and config/authorization_code.go - Allows SDK consumers to use browser detection and opening utilities - Part of handler pattern refactoring for headless authentication * Refactor authorization callback page customization to use structured data - Add AuthResultPageData struct with ProjectName, Heading, and Message fields - Replace CustomHTMLSuccess/Error string fields with CustomPageDataSuccess/Error pointers - Update HTML template to use new field names (ProjectName, Heading, Message) - SDK clients can now customize page content while using consistent HTML template - Improves maintainability and provides better type safety * fix: use IsSuccess boolean for auto-close logic instead of Heading text The template previously checked the Heading text value to determine if it should auto-close the browser window. This was fragile because SDK clients can override the Heading value for UX customization, which would break the auto-close behavior. Now uses an explicit IsSuccess boolean field that is set by the rendering function (returnSuccessPage vs returnFailedPage) rather than relying on the text content of the heading. This ensures auto-close works reliably regardless of custom heading text. * update test * use of constants * updates based on self-review * Refactor auth handlers to use exported default functions - Extract default browser handlers into public functions that consumers can use - Create DefaultAuthorizationCodeBrowserHandler for authorization code flow - Create DefaultDeviceCodePromptHandler for device code flow - Simplify handler selection logic using simple nil checks - Remove redundant unsupported platform test from browser package - Improve code reusability and documentation for consumer projects
…t creds/control chars; add justified #nosec G204 on exec.Command * suppress false-positive G101 on enum "PING_ONE_CREDENTIALS" with #nosec
- Implement PKCE (Proof Key for Code Exchange) with S256 challenge method - Add comprehensive godoc documentation explaining OAuth2 Device Authorization Grant - Document security features including PKCE, device code limitations, and polling - Add inline comments explaining PKCE verifier generation and usage - Improve code clarity with detailed security and flow documentation
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.
Change Description
Support storage name suffix, add storage type enums
Change Characteristics
Checklist
All full (or complete) PRs that need review prior to merge should have the following box checked.
If contributing a partial or incomplete change (expecting the development team to complete the remaining work) please leave the box unchecked
Required SDK Upgrades
N/a
Testing
This PR has been tested with:
Shell Command(s)
Testing Results
Expand Results
=== RUN TestWithStorageType === RUN TestWithStorageType/StorageTypeSecureLocal === RUN TestWithStorageType/StorageTypeFileSystem === RUN TestWithStorageType/StorageTypeSecureRemote === RUN TestWithStorageType/StorageTypeNone --- PASS: TestWithStorageType (0.00s) --- PASS: TestWithStorageType/StorageTypeSecureLocal (0.00s) --- PASS: TestWithStorageType/StorageTypeFileSystem (0.00s) --- PASS: TestWithStorageType/StorageTypeSecureRemote (0.00s) --- PASS: TestWithStorageType/StorageTypeNone (0.00s) === RUN TestWithStorageName --- PASS: TestWithStorageName (0.00s) === RUN TestStorageType_String === RUN TestStorageType_String/StorageTypeSecureLocal === RUN TestStorageType_String/StorageTypeFileSystem === RUN TestStorageType_String/StorageTypeSecureRemote === RUN TestStorageType_String/StorageTypeNone --- PASS: TestStorageType_String (0.00s) --- PASS: TestStorageType_String/StorageTypeSecureLocal (0.00s) --- PASS: TestStorageType_String/StorageTypeFileSystem (0.00s) --- PASS: TestStorageType_String/StorageTypeSecureRemote (0.00s) --- PASS: TestStorageType_String/StorageTypeNone (0.00s) === RUN TestStorageType_IsValid === RUN TestStorageType_IsValid/StorageTypeSecureLocal_is_valid === RUN TestStorageType_IsValid/StorageTypeFileSystem_is_valid === RUN TestStorageType_IsValid/StorageTypeSecureRemote_is_valid === RUN TestStorageType_IsValid/StorageTypeNone_is_valid === RUN TestStorageType_IsValid/Empty_string_is_invalid === RUN TestStorageType_IsValid/Random_string_is_invalid === RUN TestStorageType_IsValid/Invalid_storage_type --- PASS: TestStorageType_IsValid (0.00s) --- PASS: TestStorageType_IsValid/StorageTypeSecureLocal_is_valid (0.00s) --- PASS: TestStorageType_IsValid/StorageTypeFileSystem_is_valid (0.00s) --- PASS: TestStorageType_IsValid/StorageTypeSecureRemote_is_valid (0.00s) --- PASS: TestStorageType_IsValid/StorageTypeNone_is_valid (0.00s) --- PASS: TestStorageType_IsValid/Empty_string_is_invalid (0.00s) --- PASS: TestStorageType_IsValid/Random_string_is_invalid (0.00s) --- PASS: TestStorageType_IsValid/Invalid_storage_type (0.00s) === RUN TestStorageTypeConstants --- PASS: TestStorageTypeConstants (0.00s) PASS ok github.com/pingidentity/pingone-go-client/config 0.697s