From 885402987df21fb31c1c8b8635f2eca4d3e95b1a Mon Sep 17 00:00:00 2001 From: Krish Suchak Date: Wed, 30 Apr 2025 11:29:49 -0400 Subject: [PATCH 01/10] deprecation warning in docs --- docs/Configuring.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/Configuring.md b/docs/Configuring.md index 389b56e997..9ab7fecafb 100644 --- a/docs/Configuring.md +++ b/docs/Configuring.md @@ -78,7 +78,7 @@ Root level key `server` | `auth.cache_refresh` | Interval in which the IDP jwks should be refreshed | `15m` | OPENTDF_SERVER_AUTH_CACHE_REFRESH | | `auth.dpopskew` | The amount of time drift allowed between when the client generated a dpop proof and the server time. | `1h` | OPENTDF_SERVER_AUTH | | `auth.skew` | The amount of time drift allowed between a tokens `exp` claim and the server time. | `1m` | OPENTDF_SERVER_AUTH_SKEW | -| `auth.public_client_id` | The oidc client id. This is leveraged by otdfctl. | | OPENTDF_SERVER_AUTH_PUBLIC_CLIENT_ID | +| `auth.public_client_id` | [DEPRECATED] The oidc client id. This is leveraged by otdfctl. | | OPENTDF_SERVER_AUTH_PUBLIC_CLIENT_ID | | `auth.enforceDPoP` | If true, DPoP bindings on Access Tokens are enforced. | `false` | OPENTDF_SERVER_AUTH_ENFORCEDPOP | | `cryptoProvider` | A list of public/private keypairs and their use. Described [below](#crypto-provider) | empty | | | `enable_pprof` | Enable golang performance profiling | `false` | OPENTDF_SERVER_ENABLE_PPROF | @@ -285,7 +285,7 @@ server: auth: enabled: true enforceDPoP: false - public_client_id: 'opentdf-public' + public_client_id: 'opentdf-public' # DEPRECATED audience: 'http://localhost:8080' issuer: http://keycloak:8888/auth/realms/opentdf policy: From 1a1e4e1417fcecf41095b4f1e6d6967f35ea23f3 Mon Sep 17 00:00:00 2001 From: Krish Suchak Date: Wed, 7 May 2025 03:48:14 -0400 Subject: [PATCH 02/10] remove public_client_id --- opentdf-core-mode.yaml | 1 - opentdf-dev.yaml | 1 - opentdf-ers-mode.yaml | 1 - opentdf-example.yaml | 1 - opentdf-kas-mode.yaml | 1 - sdk/kas_client_test.go | 1 - sdk/platformconfig.go | 9 -------- sdk/sdk.go | 21 +++++++++---------- sdk/sdk_test.go | 10 --------- service/internal/auth/authn.go | 2 -- service/internal/auth/config.go | 19 +++++++---------- service/internal/auth/discovery.go | 1 - .../pkg/server/testdata/all-no-config.yaml | 1 - 13 files changed, 17 insertions(+), 52 deletions(-) diff --git a/opentdf-core-mode.yaml b/opentdf-core-mode.yaml index ef440c64b0..40aafc19d4 100644 --- a/opentdf-core-mode.yaml +++ b/opentdf-core-mode.yaml @@ -21,7 +21,6 @@ server: auth: enabled: false enforceDPoP: false - public_client_id: 'opentdf-public' audience: 'http://localhost:8080' issuer: http://localhost:8888/auth/realms/tdf cors: diff --git a/opentdf-dev.yaml b/opentdf-dev.yaml index a437b3d9c9..b2d674e932 100644 --- a/opentdf-dev.yaml +++ b/opentdf-dev.yaml @@ -45,7 +45,6 @@ server: auth: enabled: true enforceDPoP: false - public_client_id: "opentdf-public" audience: "http://localhost:8080" issuer: http://localhost:8888/auth/realms/opentdf policy: diff --git a/opentdf-ers-mode.yaml b/opentdf-ers-mode.yaml index 22ddb798f4..838ccc4091 100644 --- a/opentdf-ers-mode.yaml +++ b/opentdf-ers-mode.yaml @@ -21,7 +21,6 @@ server: auth: enabled: true enforceDPoP: false - public_client_id: "opentdf-public" audience: "http://localhost:8080" issuer: http://localhost:8888/auth/realms/opentdf policy: diff --git a/opentdf-example.yaml b/opentdf-example.yaml index 311b1e1d13..56f986eba6 100644 --- a/opentdf-example.yaml +++ b/opentdf-example.yaml @@ -32,7 +32,6 @@ server: auth: enabled: true enforceDPoP: false - public_client_id: "opentdf-public" audience: "http://localhost:8080" issuer: http://keycloak:8888/auth/realms/opentdf policy: diff --git a/opentdf-kas-mode.yaml b/opentdf-kas-mode.yaml index 9340ea1c16..08a92d4a10 100644 --- a/opentdf-kas-mode.yaml +++ b/opentdf-kas-mode.yaml @@ -32,7 +32,6 @@ server: auth: enabled: true enforceDPoP: false - public_client_id: 'opentdf-public' audience: 'http://localhost:8080' issuer: http://localhost:8888/auth/realms/opentdf policy: diff --git a/sdk/kas_client_test.go b/sdk/kas_client_test.go index 496c195f5d..2026a0e2a5 100644 --- a/sdk/kas_client_test.go +++ b/sdk/kas_client_test.go @@ -131,7 +131,6 @@ func Test_StoreKASKeys(t *testing.T) { "issuer": "https://example.org", "authorization_endpoint": "https://example.org/auth", "token_endpoint": "https://example.org/token", - "public_client_id": "myclient", }, }), ) diff --git a/sdk/platformconfig.go b/sdk/platformconfig.go index 384537b50d..2627811a32 100644 --- a/sdk/platformconfig.go +++ b/sdk/platformconfig.go @@ -35,15 +35,6 @@ func (c PlatformConfiguration) TokenEndpoint() (string, error) { return value, nil } -func (c PlatformConfiguration) PublicClientID() (string, error) { - idpCfg := c.getIdpConfig() - value, ok := idpCfg["public_client_id"].(string) - if !ok { - return "", ErrPlatformPublicClientIDNotFound - } - return value, nil -} - func (c PlatformConfiguration) platformEndpoint() (string, error) { value, ok := c["platform_endpoint"].(string) if !ok { diff --git a/sdk/sdk.go b/sdk/sdk.go index b06e49ddc8..14b9579a50 100644 --- a/sdk/sdk.go +++ b/sdk/sdk.go @@ -40,17 +40,16 @@ import ( const ( // Failure while connecting to a service. // Check your configuration and/or retry. - ErrGrpcDialFailed = Error("failed to dial grpc endpoint") - ErrShutdownFailed = Error("failed to shutdown sdk") - ErrPlatformUnreachable = Error("platform unreachable or not responding") - ErrPlatformConfigFailed = Error("failed to retrieve platform configuration") - ErrPlatformEndpointMalformed = Error("platform endpoint is malformed") - ErrPlatformIssuerNotFound = Error("issuer not found in well-known idp configuration") - ErrPlatformAuthzEndpointNotFound = Error("authorization_endpoint not found in well-known idp configuration") - ErrPlatformTokenEndpointNotFound = Error("token_endpoint not found in well-known idp configuration") - ErrPlatformPublicClientIDNotFound = Error("public_client_id not found in well-known idp configuration") - ErrPlatformEndpointNotFound = Error("platform_endpoint not found in well-known configuration") - ErrAccessTokenInvalid = Error("access token is invalid") + ErrGrpcDialFailed = Error("failed to dial grpc endpoint") + ErrShutdownFailed = Error("failed to shutdown sdk") + ErrPlatformUnreachable = Error("platform unreachable or not responding") + ErrPlatformConfigFailed = Error("failed to retrieve platform configuration") + ErrPlatformEndpointMalformed = Error("platform endpoint is malformed") + ErrPlatformIssuerNotFound = Error("issuer not found in well-known idp configuration") + ErrPlatformAuthzEndpointNotFound = Error("authorization_endpoint not found in well-known idp configuration") + ErrPlatformTokenEndpointNotFound = Error("token_endpoint not found in well-known idp configuration") + ErrPlatformEndpointNotFound = Error("platform_endpoint not found in well-known configuration") + ErrAccessTokenInvalid = Error("access token is invalid") ) type Error string diff --git a/sdk/sdk_test.go b/sdk/sdk_test.go index da6d4fdf8a..7eac9d9500 100644 --- a/sdk/sdk_test.go +++ b/sdk/sdk_test.go @@ -36,7 +36,6 @@ func TestNew_ShouldCreateSDK(t *testing.T) { "issuer": "https://example.org", "authorization_endpoint": "https://example.org/auth", "token_endpoint": "https://example.org/token", - "public_client_id": "myclient", }, }), sdk.WithClientCredentials("myid", "mysecret", nil), @@ -60,11 +59,6 @@ func TestNew_ShouldCreateSDK(t *testing.T) { assert.Equal(t, "https://example.org/token", tokenEndpoint) require.NoError(t, err) - // Check platform public client id - publicClientID, err := s.PlatformConfiguration.PublicClientID() - assert.Equal(t, "myclient", publicClientID) - require.NoError(t, err) - // check if the clients are available assert.NotNil(t, s.Attributes) assert.NotNil(t, s.ResourceMapping) @@ -85,10 +79,6 @@ func Test_PlatformConfiguration_BadCases(t *testing.T) { tokenEndpoint, err := s.PlatformConfiguration.TokenEndpoint() assert.Equal(t, "", tokenEndpoint) require.ErrorIs(t, err, sdk.ErrPlatformTokenEndpointNotFound) - - publicClientID, err := s.PlatformConfiguration.PublicClientID() - assert.Equal(t, "", publicClientID) - require.ErrorIs(t, err, sdk.ErrPlatformPublicClientIDNotFound) } noIdpValsSDK, err := sdk.New(goodPlatformEndpoint, diff --git a/service/internal/auth/authn.go b/service/internal/auth/authn.go index b7587a5eab..ff5d6dc5c2 100644 --- a/service/internal/auth/authn.go +++ b/service/internal/auth/authn.go @@ -113,8 +113,6 @@ func NewAuthenticator(ctx context.Context, cfg Config, logger *logger.Logger, we if err != nil { return nil, err } - // Assign configured public_client_id - oidcConfig.PublicClientID = cfg.PublicClientID // If the issuer is different from the one in the configuration, update the configuration // This could happen if we are hitting an internal endpoint. Example we might point to https://keycloak.opentdf.svc/realms/opentdf diff --git a/service/internal/auth/config.go b/service/internal/auth/config.go index e98b8ae7ba..a4cdea516f 100644 --- a/service/internal/auth/config.go +++ b/service/internal/auth/config.go @@ -19,14 +19,13 @@ type Config struct { // AuthNConfig is the configuration need for the platform to validate tokens type AuthNConfig struct { //nolint:revive // AuthNConfig is a valid name - EnforceDPoP bool `mapstructure:"enforceDPoP" json:"enforceDPoP" default:"false"` - Issuer string `mapstructure:"issuer" json:"issuer"` - Audience string `mapstructure:"audience" json:"audience"` - Policy PolicyConfig `mapstructure:"policy" json:"policy"` - CacheRefresh string `mapstructure:"cache_refresh_interval"` - DPoPSkew time.Duration `mapstructure:"dpopskew" default:"1h"` - TokenSkew time.Duration `mapstructure:"skew" default:"1m"` - PublicClientID string `mapstructure:"public_client_id" json:"public_client_id,omitempty"` + EnforceDPoP bool `mapstructure:"enforceDPoP" json:"enforceDPoP" default:"false"` + Issuer string `mapstructure:"issuer" json:"issuer"` + Audience string `mapstructure:"audience" json:"audience"` + Policy PolicyConfig `mapstructure:"policy" json:"policy"` + CacheRefresh string `mapstructure:"cache_refresh_interval"` + DPoPSkew time.Duration `mapstructure:"dpopskew" default:"1h"` + TokenSkew time.Duration `mapstructure:"skew" default:"1m"` } type PolicyConfig struct { @@ -57,10 +56,6 @@ func (c AuthNConfig) validateAuthNConfig(logger *logger.Logger) error { return fmt.Errorf("config Auth.Audience is required") } - if c.PublicClientID == "" { - logger.Warn("config Auth.PublicClientID is empty and is required for discovery via well-known configuration.") - } - if !c.EnforceDPoP { logger.Warn("config Auth.EnforceDPoP is false. DPoP will not be enforced.") } diff --git a/service/internal/auth/discovery.go b/service/internal/auth/discovery.go index e77988626a..9cbff5a3b7 100644 --- a/service/internal/auth/discovery.go +++ b/service/internal/auth/discovery.go @@ -26,7 +26,6 @@ type OIDCConfiguration struct { SubjectTypesSupported []string `json:"subject_types_supported"` IDTokenSigningAlgValuesSupported []string `json:"id_token_signing_alg_values_supported"` RequireRequestURIRegistration bool `json:"require_request_uri_registration"` - PublicClientID string `json:"public_client_id,omitempty"` } // DiscoverOPENIDConfiguration discovers the openid configuration for the issuer provided diff --git a/service/pkg/server/testdata/all-no-config.yaml b/service/pkg/server/testdata/all-no-config.yaml index ff4e1f2998..909b0b53d3 100644 --- a/service/pkg/server/testdata/all-no-config.yaml +++ b/service/pkg/server/testdata/all-no-config.yaml @@ -36,7 +36,6 @@ server: auth: enabled: true enforceDPoP: false - public_client_id: 'opentdf-public' audience: 'http://localhost:8080' issuer: http://localhost:8888/auth/realms/opentdf policy: From 373036af429e7619e3009ad929c11843ba0bbc4e Mon Sep 17 00:00:00 2001 From: Krish Suchak Date: Fri, 23 May 2025 11:04:02 -0400 Subject: [PATCH 03/10] change otdfctl-ref --- .github/workflows/checks.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/checks.yaml b/.github/workflows/checks.yaml index 4a4ace3686..5fab8c748c 100644 --- a/.github/workflows/checks.yaml +++ b/.github/workflows/checks.yaml @@ -342,6 +342,7 @@ jobs: uses: opentdf/tests/.github/workflows/xtest.yml@main with: focus-sdk: go + otdfctl-ref: test/platform # use commit instead of ref so we can "go get" specific sdk version platform-ref: ${{ github.event.pull_request.head.sha || github.sha }} lts From d78b34f74d664b717b07693361a1e0663b35269a Mon Sep 17 00:00:00 2001 From: Krish Suchak Date: Fri, 23 May 2025 11:17:00 -0400 Subject: [PATCH 04/10] don't use replace From e86cc81aa5cb23ee83d5354539753924efac99d3 Mon Sep 17 00:00:00 2001 From: Krish Suchak Date: Fri, 23 May 2025 11:23:19 -0400 Subject: [PATCH 05/10] use commit --- .github/workflows/checks.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/checks.yaml b/.github/workflows/checks.yaml index 5fab8c748c..cb3f2ea3f7 100644 --- a/.github/workflows/checks.yaml +++ b/.github/workflows/checks.yaml @@ -342,7 +342,7 @@ jobs: uses: opentdf/tests/.github/workflows/xtest.yml@main with: focus-sdk: go - otdfctl-ref: test/platform + otdfctl-ref: 64a94245bf663ef4e1916aaa621a1039b05f92cc # use commit instead of ref so we can "go get" specific sdk version platform-ref: ${{ github.event.pull_request.head.sha || github.sha }} lts From ae148ea0d44fc19b2c58425ccb83b54a2079fe4d Mon Sep 17 00:00:00 2001 From: Krish Suchak Date: Fri, 23 May 2025 11:29:17 -0400 Subject: [PATCH 06/10] lts --- .github/workflows/checks.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/checks.yaml b/.github/workflows/checks.yaml index cb3f2ea3f7..1002f52ae0 100644 --- a/.github/workflows/checks.yaml +++ b/.github/workflows/checks.yaml @@ -342,7 +342,7 @@ jobs: uses: opentdf/tests/.github/workflows/xtest.yml@main with: focus-sdk: go - otdfctl-ref: 64a94245bf663ef4e1916aaa621a1039b05f92cc + otdfctl-ref: 64a94245bf663ef4e1916aaa621a1039b05f92cc lts # use commit instead of ref so we can "go get" specific sdk version platform-ref: ${{ github.event.pull_request.head.sha || github.sha }} lts From 72b6b50544a53e64613e3550f9bd64a5eb8cd179 Mon Sep 17 00:00:00 2001 From: Krish Suchak Date: Fri, 23 May 2025 12:27:10 -0400 Subject: [PATCH 07/10] try ref again --- .github/workflows/checks.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/checks.yaml b/.github/workflows/checks.yaml index 1002f52ae0..e464334aff 100644 --- a/.github/workflows/checks.yaml +++ b/.github/workflows/checks.yaml @@ -342,7 +342,7 @@ jobs: uses: opentdf/tests/.github/workflows/xtest.yml@main with: focus-sdk: go - otdfctl-ref: 64a94245bf663ef4e1916aaa621a1039b05f92cc lts + otdfctl-ref: refs/heads/test/platform # use commit instead of ref so we can "go get" specific sdk version platform-ref: ${{ github.event.pull_request.head.sha || github.sha }} lts From 58953b3c579691ace454c6cc501faf1a321d53d5 Mon Sep 17 00:00:00 2001 From: Krish Suchak Date: Fri, 23 May 2025 12:32:58 -0400 Subject: [PATCH 08/10] again --- .github/workflows/checks.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/checks.yaml b/.github/workflows/checks.yaml index e464334aff..7195c8d7a1 100644 --- a/.github/workflows/checks.yaml +++ b/.github/workflows/checks.yaml @@ -342,7 +342,7 @@ jobs: uses: opentdf/tests/.github/workflows/xtest.yml@main with: focus-sdk: go - otdfctl-ref: refs/heads/test/platform + otdfctl-ref: refs/remotes/origin/test/platform # use commit instead of ref so we can "go get" specific sdk version platform-ref: ${{ github.event.pull_request.head.sha || github.sha }} lts From 974f84096e3ff8b1680dfd42f701797ec2a1698a Mon Sep 17 00:00:00 2001 From: Krish Suchak Date: Fri, 23 May 2025 12:44:22 -0400 Subject: [PATCH 09/10] pr ref --- .github/workflows/checks.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/checks.yaml b/.github/workflows/checks.yaml index 7195c8d7a1..8901ef99f2 100644 --- a/.github/workflows/checks.yaml +++ b/.github/workflows/checks.yaml @@ -342,7 +342,7 @@ jobs: uses: opentdf/tests/.github/workflows/xtest.yml@main with: focus-sdk: go - otdfctl-ref: refs/remotes/origin/test/platform + otdfctl-ref: refs/pull/562 # use commit instead of ref so we can "go get" specific sdk version platform-ref: ${{ github.event.pull_request.head.sha || github.sha }} lts From be48f19069cff7a5b62b7ad1570dafd5ac6163fd Mon Sep 17 00:00:00 2001 From: Krish Suchak Date: Wed, 28 May 2025 11:57:12 -0400 Subject: [PATCH 10/10] revert workflow change --- .github/workflows/checks.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/checks.yaml b/.github/workflows/checks.yaml index 8901ef99f2..4a4ace3686 100644 --- a/.github/workflows/checks.yaml +++ b/.github/workflows/checks.yaml @@ -342,7 +342,6 @@ jobs: uses: opentdf/tests/.github/workflows/xtest.yml@main with: focus-sdk: go - otdfctl-ref: refs/pull/562 # use commit instead of ref so we can "go get" specific sdk version platform-ref: ${{ github.event.pull_request.head.sha || github.sha }} lts