From 06c1be119ceb7b084f7b98e88272480ff282a609 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20J=C3=A4ger?= Date: Wed, 9 Jul 2025 18:16:49 +0200 Subject: [PATCH] Read OpenID credentials from file (#3244) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: read OpenID credentials from file Signed-off-by: Uwe Jäger * feat: allow credentials file and secret in config to keep BC Signed-off-by: Uwe Jäger --------- Signed-off-by: Uwe Jäger --- Makefile | 2 +- .../config-openid-github-credentials.json | 4 + .../config-openid-gitlab-credentials.json | 4 + .../config-openid-google-credentials.json | 4 + examples/config-openid-oidc-credentials.json | 4 + examples/config-openid.json | 12 +-- pkg/api/authn.go | 11 +-- pkg/api/config/config.go | 16 ++-- pkg/cli/server/root.go | 32 ++++++++ pkg/cli/server/root_test.go | 76 ++++++++++++++++--- test/blackbox/cloud_only.bats | 11 ++- 11 files changed, 144 insertions(+), 32 deletions(-) create mode 100644 examples/config-openid-github-credentials.json create mode 100644 examples/config-openid-gitlab-credentials.json create mode 100644 examples/config-openid-google-credentials.json create mode 100644 examples/config-openid-oidc-credentials.json diff --git a/Makefile b/Makefile index 5ccb218a..ae351e6e 100644 --- a/Makefile +++ b/Makefile @@ -402,7 +402,7 @@ verify-config: _verify-config verify-config-warnings verify-config-commited .PHONY: _verify-config _verify-config: binary rm -f output.txt - $(foreach file, $(filter-out examples/config-ldap-credentials.json, $(wildcard examples/config-*)), ./bin/zot-$(OS)-$(ARCH) verify $(file) 2>&1 | tee -a output.txt || exit 1;) + $(foreach file, $(filter-out $(wildcard examples/config-*-credentials.json), $(wildcard examples/config-*)), ./bin/zot-$(OS)-$(ARCH) verify $(file) 2>&1 | tee -a output.txt || exit 1;) .PHONY: verify-config-warnings verify-config-warnings: _verify-config diff --git a/examples/config-openid-github-credentials.json b/examples/config-openid-github-credentials.json new file mode 100644 index 00000000..b89155ad --- /dev/null +++ b/examples/config-openid-github-credentials.json @@ -0,0 +1,4 @@ +{ + "clientid": "client_id", + "clientsecret": "client_secret" +} \ No newline at end of file diff --git a/examples/config-openid-gitlab-credentials.json b/examples/config-openid-gitlab-credentials.json new file mode 100644 index 00000000..b89155ad --- /dev/null +++ b/examples/config-openid-gitlab-credentials.json @@ -0,0 +1,4 @@ +{ + "clientid": "client_id", + "clientsecret": "client_secret" +} \ No newline at end of file diff --git a/examples/config-openid-google-credentials.json b/examples/config-openid-google-credentials.json new file mode 100644 index 00000000..b89155ad --- /dev/null +++ b/examples/config-openid-google-credentials.json @@ -0,0 +1,4 @@ +{ + "clientid": "client_id", + "clientsecret": "client_secret" +} \ No newline at end of file diff --git a/examples/config-openid-oidc-credentials.json b/examples/config-openid-oidc-credentials.json new file mode 100644 index 00000000..b89155ad --- /dev/null +++ b/examples/config-openid-oidc-credentials.json @@ -0,0 +1,4 @@ +{ + "clientid": "client_id", + "clientsecret": "client_secret" +} \ No newline at end of file diff --git a/examples/config-openid.json b/examples/config-openid.json index 8f6ec1a5..20899c4c 100644 --- a/examples/config-openid.json +++ b/examples/config-openid.json @@ -18,28 +18,24 @@ "openid": { "providers": { "github": { - "clientid": "client_id", - "clientsecret": "client_secret", + "credentialsFile": "examples/config-openid-github-credentials.json", "keypath": "", "scopes": ["read:org", "user", "repo"] }, "google": { + "credentialsFile": "examples/config-openid-google-credentials.json", "issuer": "https://accounts.google.com", - "clientid": "client_id", - "clientsecret": "client_secret", "scopes": ["openid", "email"] }, "gitlab": { "issuer": "https://gitlab.com", - "clientid": "client_id", - "clientsecret": "client_secret", + "credentialsFile": "examples/config-openid-gitlab-credentials.json", "scopes": ["openid", "read_api", "read_user", "profile", "email"] }, "oidc": { "name": "Corporate SSO", "issuer": "http://127.0.0.1:5556/dex", - "clientid": "client_id", - "clientsecret": "client_secret", + "credentialsFile": "examples/config-openid-oidc-credentials.json", "scopes": ["openid", "user", "email", "groups"] } } diff --git a/pkg/api/authn.go b/pkg/api/authn.go index ab0aebb0..a36b9244 100644 --- a/pkg/api/authn.go +++ b/pkg/api/authn.go @@ -614,18 +614,19 @@ func getRelyingPartyArgs(cfg *config.Config, provider string, hashKey, encryptKe log.Panic().Err(zerr.ErrOpenIDProviderDoesNotExist).Str("provider", provider).Msg("") } - clientID := cfg.HTTP.Auth.OpenID.Providers[provider].ClientID - clientSecret := cfg.HTTP.Auth.OpenID.Providers[provider].ClientSecret + providerConfig := cfg.HTTP.Auth.OpenID.Providers[provider] + clientID := providerConfig.ClientID + clientSecret := providerConfig.ClientSecret - scopes := cfg.HTTP.Auth.OpenID.Providers[provider].Scopes + scopes := providerConfig.Scopes // openid scope must be the first one in list if !zcommon.Contains(scopes, oidc.ScopeOpenID) && config.IsOpenIDSupported(provider) { scopes = append([]string{oidc.ScopeOpenID}, scopes...) } port := cfg.HTTP.Port - issuer := cfg.HTTP.Auth.OpenID.Providers[provider].Issuer - keyPath := cfg.HTTP.Auth.OpenID.Providers[provider].KeyPath + issuer := providerConfig.Issuer + keyPath := providerConfig.KeyPath baseURL := net.JoinHostPort(cfg.HTTP.Address, port) callback := constants.CallbackBasePath + "/" + provider diff --git a/pkg/api/config/config.go b/pkg/api/config/config.go index d0d40c45..46f6acfe 100644 --- a/pkg/api/config/config.go +++ b/pkg/api/config/config.go @@ -94,13 +94,19 @@ type OpenIDConfig struct { Providers map[string]OpenIDProviderConfig } -type OpenIDProviderConfig struct { - Name string +type OpenIDCredentials struct { ClientID string ClientSecret string - KeyPath string - Issuer string - Scopes []string +} + +type OpenIDProviderConfig struct { + CredentialsFile string + Name string + ClientID string + ClientSecret string + KeyPath string + Issuer string + Scopes []string } type MethodRatelimitConfig struct { diff --git a/pkg/cli/server/root.go b/pkg/cli/server/root.go index 6171d201..1fc674c0 100644 --- a/pkg/cli/server/root.go +++ b/pkg/cli/server/root.go @@ -863,6 +863,12 @@ func LoadConfiguration(config *config.Config, configPath string) error { return err } + if err := updateOpenIDConfig(config); err != nil { + log.Error().Err(err).Msg("failed to read openid provider config file(s)") + + return err + } + if err := loadSessionKeys(config); err != nil { log.Error().Err(err).Msg("failed to read sessionKeysFile") @@ -926,6 +932,32 @@ func updateLDAPConfig(conf *config.Config) error { return nil } +func updateOpenIDConfig(conf *config.Config) error { + if conf.HTTP.Auth == nil || conf.HTTP.Auth.OpenID == nil { + return nil + } + + for name, provider := range conf.HTTP.Auth.OpenID.Providers { + if provider.CredentialsFile != "" { + var newOpenIDCredentials config.OpenIDCredentials + + if err := readSecretFile(provider.CredentialsFile, &newOpenIDCredentials, true); err != nil { + return err + } + + provider.ClientID = newOpenIDCredentials.ClientID + provider.ClientSecret = newOpenIDCredentials.ClientSecret + + conf.HTTP.Auth.OpenID.Providers[name] = provider + } else { + log.Warn().Str("provider", name). + Msg("deprecated: use the new OpenID provider credentialsfile instead of clientid and clientsecret.") + } + } + + return nil +} + func readSecretFile(path string, v any, checkUnsetFields bool) error { //nolint: varnamelen viperInstance := viper.NewWithOptions(viper.KeyDelimiter("::")) diff --git a/pkg/cli/server/root_test.go b/pkg/cli/server/root_test.go index 39d9a4c3..1abe5765 100644 --- a/pkg/cli/server/root_test.go +++ b/pkg/cli/server/root_test.go @@ -1232,7 +1232,7 @@ storage: So(err, ShouldNotBeNil) }) - Convey("Test verify oauth2 config with missing parameter", t, func(c C) { + Convey("Test verify oauth2 config with missing parameter scopes", t, func(c C) { tmpfile, err := os.CreateTemp("", "zot-test*.json") So(err, ShouldBeNil) @@ -1252,6 +1252,26 @@ storage: So(err, ShouldNotBeNil) }) + Convey("Test verify oauth2 config with missing parameter clientid", t, func(c C) { + tmpfile, err := os.CreateTemp("", "zot-test*.json") + So(err, ShouldBeNil) + + defer os.Remove(tmpfile.Name()) // clean up + + content := []byte(`{"distSpecVersion":"1.1.1","storage":{"rootDirectory":"/tmp/zot"}, + "http":{"address":"127.0.0.1","port":"8080","realm":"zot", + "auth":{"openid":{"providers":{"github":{"scopes":["openid"]}}}}}, + "log":{"level":"debug"}}`) + _, err = tmpfile.Write(content) + So(err, ShouldBeNil) + err = tmpfile.Close() + So(err, ShouldBeNil) + + os.Args = []string{"cli_test", "verify", tmpfile.Name()} + err = cli.NewServerRootCmd().Execute() + So(err, ShouldNotBeNil) + }) + Convey("Test verify openid config with unsupported provider", t, func(c C) { tmpfile, err := os.CreateTemp("", "zot-test*.json") So(err, ShouldBeNil) @@ -1278,11 +1298,27 @@ storage: defer os.Remove(tmpfile.Name()) // clean up - content := []byte(`{"distSpecVersion":"1.1.1","storage":{"rootDirectory":"/tmp/zot"}, - "http":{"address":"127.0.0.1","port":"8080","realm":"zot", - "auth":{"openid":{"providers":{"oidc":{"issuer":"http://127.0.0.1:5556/dex", - "clientid":"client_id","scopes":["openid"]}}}}}, - "log":{"level":"debug"}}`) + tmpCredsFile, err := os.CreateTemp("", "zot-cred*.json") + So(err, ShouldBeNil) + defer os.Remove(tmpCredsFile.Name()) + + content := []byte(`{ + "clientid":"client-id", + "clientsecret":"client-secret" + }`) + + _, err = tmpCredsFile.Write(content) + So(err, ShouldBeNil) + err = tmpCredsFile.Close() + So(err, ShouldBeNil) + + content = []byte(fmt.Sprintf(`{"distSpecVersion":"1.1.1","storage":{"rootDirectory":"/tmp/zot"}, + "http":{"address":"127.0.0.1","port":"8080","realm":"zot", + "auth":{"openid":{"providers":{"oidc":{"issuer":"http://127.0.0.1:5556/dex", + "credentialsFile":"%s","scopes":["openid"]}}}}}, + "log":{"level":"debug"}}`, + tmpCredsFile.Name()), + ) _, err = tmpfile.Write(content) So(err, ShouldBeNil) err = tmpfile.Close() @@ -1641,13 +1677,31 @@ func TestApiKeyConfig(t *testing.T) { tmpfile, err := os.CreateTemp("", "zot-test*.json") So(err, ShouldBeNil) + defer os.Remove(tmpfile.Name()) // clean up + + tmpCredsFile, err := os.CreateTemp("", "zot-cred*.json") + So(err, ShouldBeNil) + defer os.Remove(tmpCredsFile.Name()) + + content := []byte(`{ + "clientid":"client-id", + "clientsecret":"client-secret" + }`) + + _, err = tmpCredsFile.Write(content) + So(err, ShouldBeNil) + err = tmpCredsFile.Close() + So(err, ShouldBeNil) + defer os.Remove(tmpfile.Name()) - content := []byte(`{"distSpecVersion":"1.1.1","storage":{"rootDirectory":"/tmp/zot"}, - "http":{"address":"127.0.0.1","port":"8080","realm":"zot", - "auth":{"openid":{"providers":{"oidc":{"issuer":"http://127.0.0.1:5556/dex", - "clientid":"client_id","scopes":["openid"]}}}}}, - "log":{"level":"debug"}}`) + content = []byte(fmt.Sprintf(`{"distSpecVersion":"1.1.1","storage":{"rootDirectory":"/tmp/zot"}, + "http":{"address":"127.0.0.1","port":"8080","realm":"zot", + "auth":{"openid":{"providers":{"oidc":{"issuer":"http://127.0.0.1:5556/dex", + "credentialsFile":"%s","scopes":["openid"]}}}}}, + "log":{"level":"debug"}}`, + tmpCredsFile.Name()), + ) err = os.WriteFile(tmpfile.Name(), content, 0o0600) So(err, ShouldBeNil) diff --git a/test/blackbox/cloud_only.bats b/test/blackbox/cloud_only.bats index 633cf594..b7a5a59f 100644 --- a/test/blackbox/cloud_only.bats +++ b/test/blackbox/cloud_only.bats @@ -14,6 +14,14 @@ function setup() { # Setup zot server local zot_root_dir=${BATS_FILE_TMPDIR}/zot local zot_config_file=${BATS_FILE_TMPDIR}/zot_config.json + local zot_openid_credentials_file=${BATS_FILE_TMPDIR}/openid_credentials.json + + cat > ${zot_openid_credentials_file}<&3 @@ -57,8 +65,7 @@ function setup() { "providers": { "oidc": { "issuer": "http://127.0.0.1:5556/dex", - "clientid": "zot-client", - "clientsecret": "ZXhhbXBsZS1hcHAtc2VjcmV0", + "credentialsfile": "${zot_openid_credentials_file}", "scopes": ["openid", "email", "groups"] } }