refactor: move SAML SP from issuer to apigw #213
Merged
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
Move SAML Service Provider implementation from internal/issuer to internal/apigw to align with architectural separation of concerns. ## Why This Change The APIGW component handles all user authentication flows (OAuth2, OIDC). SAML authentication should follow the same pattern rather than being in the issuer, which should focus solely on credential creation and signing. ## Changes **APIGW (new SAML location):** - Add SAML endpoints to internal/apigw/httpserver/endpoints_saml.go - Add SAML service integration and initialization - Update httpserver.Service to include SAMLService - Add SAML route registration (build tag conditional) - Add SAML configuration to model.APIGW struct - Move integration tests to internal/apigw/integration/ **Issuer (SAML removed):** - Remove SAML endpoints from internal/issuer/httpserver/ - Remove SAMLService from issuer httpserver.Service - Remove SAML initialization from cmd/issuer/main.go - Update httpserver.New() signature (no longer takes SAMLService) **Implementation Details:** - SAML endpoints now call issuer via gRPC (same pattern as OAuth flows) - Maintains build tag support (-tags=saml) for conditional compilation - Both enabled and disabled paths tested and working - All SAML integration tests passing (0.346s) ## Testing ✅ go build -tags=saml ./cmd/apigw/ # SAML enabled ✅ go build -tags=saml ./cmd/issuer/ # No SAML in issuer ✅ go build ./cmd/apigw/ # SAML disabled ✅ go build ./cmd/issuer/ # No SAML references ✅ go test -tags=saml ./internal/apigw/integration/ # All tests pass ## Benefits 1. **Architectural Consistency**: APIGW now handles all authentication (OAuth, OIDC, SAML) 2. **Service Independence**: Issuer is pure credential creation service 3. **Future-Proofing**: Sets pattern for additional auth methods (e.g., OpenID Connect - Priority 11) 4. **Code Reuse**: SAML uses same issuer client interface as OAuth Related to upstream maintainer feedback and ROADMAP.md Priority 11.
masv3971
reviewed
Nov 25, 2025
Add support for configuring a single static Identity Provider (IdP)
instead of using an MDQ (Metadata Query Protocol) service. This is
useful for simple deployments with a single IdP, testing environments,
or scenarios where IdP metadata rarely changes.
Changes:
- Add StaticIDPConfig to SAMLConfig with entity_id, metadata_path, and metadata_url
- Add SAMLConfig.Validate() to ensure MDQ and static IdP are mutually exclusive
- Extend MDQClient to support static metadata loading from file or URL
- Update SAML service initialization to support both MDQ and static modes
- Add comprehensive tests for static IdP functionality
- Add configuration validation tests
- Update route registration to use proper gin.RouterGroup types
- Add SAML_STATIC_IDP.md documentation
Configuration example:
saml:
enabled: true
static_idp_metadata:
entity_id: "https://idp.example.com"
metadata_path: "/path/to/idp-metadata.xml"
Tests:
- Static metadata loading from file and URL
- Configuration validation (mutual exclusivity, required fields)
- MDQ client behavior in static mode
- Service initialization with static IdP
All tests passing:
- go test -tags=saml ./pkg/saml/... (35/35 passed)
- go test ./pkg/model/... (9/9 passed)
- go build -tags=saml ./cmd/apigw/ (success)
- go build ./cmd/apigw/ (success, without tags)
Contributor
Author
|
I will add one more fix: we need a simple way to register a single IdP (SAML metadata file or URL) to make it easier to deploy in common cases |
SAML has been moved to APIGW, so the SAML configuration should only exist in the APIGW struct, not in Issuer. Addresses PR review comment from @masv3971
# Conflicts: # pkg/saml/mdq_static_test.go
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.
Move SAML Service Provider implementation from internal/issuer to internal/apigw to align with architectural separation of concerns.
Why This Change
The APIGW component handles all user authentication flows (OAuth2, OIDC). SAML authentication should follow the same pattern rather than being in the issuer, which should focus solely on credential creation and signing.
Changes
APIGW (new SAML location):
Issuer (SAML removed):
Implementation Details:
Testing
✅ go build -tags=saml ./cmd/apigw/ # SAML enabled
✅ go build -tags=saml ./cmd/issuer/ # No SAML in issuer
✅ go build ./cmd/apigw/ # SAML disabled
✅ go build ./cmd/issuer/ # No SAML references
✅ go test -tags=saml ./internal/apigw/integration/ # All tests pass
Benefits
Related to upstream maintainer feedback and ROADMAP.md Priority 11.