refactor: log warnings on startup about oidc secrets (#5047)

pull/5049/head
James Elliott 2023-03-09 18:26:52 +11:00 committed by GitHub
parent fdd9901361
commit b490396c60
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 62 additions and 16 deletions

View File

@ -140,6 +140,20 @@ type PasswordDigest struct {
algorithm.Digest
}
// IsPlainText returns true if the underlying algorithm.Digest is a *plaintext.Digest.
func (d *PasswordDigest) IsPlainText() bool {
if d == nil || d.Digest == nil {
return false
}
switch d.Digest.(type) {
case *plaintext.Digest:
return true
default:
return false
}
}
// NewX509CertificateChain creates a new *X509CertificateChain from a given string, parsing each PEM block one by one.
func NewX509CertificateChain(in string) (chain *X509CertificateChain, err error) {
if in == "" {

View File

@ -160,6 +160,7 @@ const (
"an empty id"
errFmtOIDCClientInvalidSecret = "identity_providers: oidc: client '%s': option 'secret' is required"
errFmtOIDCClientInvalidSecretPlainText = "identity_providers: oidc: client '%s': option 'secret' is plaintext but it should be a hashed value as plaintext values are deprecated and will be removed when oidc becomes stable"
errFmtOIDCClientPublicInvalidSecret = "identity_providers: oidc: client '%s': option 'secret' is " +
"required to be empty when option 'public' is true"
errFmtOIDCClientRedirectURICantBeParsed = "identity_providers: oidc: client '%s': option 'redirect_uris' has an " +

View File

@ -166,6 +166,8 @@ func validateOIDCClients(config *schema.OpenIDConnectConfiguration, val *schema.
} else {
if client.Secret == nil {
val.Push(fmt.Errorf(errFmtOIDCClientInvalidSecret, client.ID))
} else if client.Secret.IsPlainText() {
val.PushWarning(fmt.Errorf(errFmtOIDCClientInvalidSecretPlainText, client.ID))
}
}

View File

@ -179,8 +179,8 @@ func TestShouldRaiseErrorWhenOIDCServerClientBadValues(t *testing.T) {
},
},
Errors: []string{
fmt.Sprintf(errFmtOIDCClientInvalidSecret, ""),
errFmtOIDCClientsWithEmptyID,
"identity_providers: oidc: client '': option 'secret' is required",
"identity_providers: oidc: one or more clients have been configured with an empty id",
},
},
{
@ -195,7 +195,7 @@ func TestShouldRaiseErrorWhenOIDCServerClientBadValues(t *testing.T) {
},
},
},
Errors: []string{fmt.Sprintf(errFmtOIDCClientInvalidPolicy, "client-1", "a-policy")},
Errors: []string{"identity_providers: oidc: client 'client-1': option 'policy' must be 'one_factor' or 'two_factor' but it is configured as 'a-policy'"},
},
{
Name: "ClientIDDuplicated",
@ -427,7 +427,7 @@ func TestShouldRaiseErrorWhenOIDCClientConfiguredWithBadGrantTypes(t *testing.T)
Clients: []schema.OpenIDConnectClientConfiguration{
{
ID: "good_id",
Secret: MustDecodeSecret("$plaintext$good_secret"),
Secret: MustDecodeSecret(goodOpenIDConnectClientSecret),
Policy: "two_factor",
GrantTypes: []string{"bad_grant_type"},
RedirectURIs: []string{
@ -454,7 +454,7 @@ func TestShouldNotErrorOnCertificateValid(t *testing.T) {
Clients: []schema.OpenIDConnectClientConfiguration{
{
ID: "good_id",
Secret: MustDecodeSecret("$plaintext$good_secret"),
Secret: MustDecodeSecret(goodOpenIDConnectClientSecret),
Policy: "two_factor",
RedirectURIs: []string{
"https://google.com/callback",
@ -480,7 +480,7 @@ func TestShouldRaiseErrorOnCertificateNotValid(t *testing.T) {
Clients: []schema.OpenIDConnectClientConfiguration{
{
ID: "good_id",
Secret: MustDecodeSecret("$plaintext$good_secret"),
Secret: MustDecodeSecret(goodOpenIDConnectClientSecret),
Policy: "two_factor",
RedirectURIs: []string{
"https://google.com/callback",
@ -507,7 +507,7 @@ func TestShouldRaiseErrorOnKeySizeTooSmall(t *testing.T) {
Clients: []schema.OpenIDConnectClientConfiguration{
{
ID: "good_id",
Secret: MustDecodeSecret("$plaintext$good_secret"),
Secret: MustDecodeSecret(goodOpenIDConnectClientSecret),
Policy: "two_factor",
RedirectURIs: []string{
"https://google.com/callback",
@ -587,7 +587,7 @@ func TestValidateIdentityProvidersShouldRaiseWarningOnSecurityIssue(t *testing.T
Clients: []schema.OpenIDConnectClientConfiguration{
{
ID: "good_id",
Secret: MustDecodeSecret("$plaintext$good_secret"),
Secret: MustDecodeSecret(goodOpenIDConnectClientSecret),
Policy: "two_factor",
RedirectURIs: []string{
"https://google.com/callback",
@ -623,7 +623,7 @@ func TestValidateIdentityProvidersShouldRaiseErrorsOnInvalidClientTypes(t *testi
},
{
ID: "client-with-bad-redirect-uri",
Secret: MustDecodeSecret("$plaintext$a-secret"),
Secret: MustDecodeSecret(goodOpenIDConnectClientSecret),
Public: false,
Policy: "two_factor",
RedirectURIs: []string{
@ -702,6 +702,33 @@ func TestValidateIdentityProvidersShouldNotRaiseErrorsOnValidClientOptions(t *te
assert.Len(t, validator.Warnings(), 0)
}
func TestValidateIdentityProvidersShouldRaiseWarningOnPlainTextClients(t *testing.T) {
validator := schema.NewStructValidator()
config := &schema.IdentityProvidersConfiguration{
OIDC: &schema.OpenIDConnectConfiguration{
HMACSecret: "hmac1",
IssuerPrivateKey: MustParseRSAPrivateKey(testKey1),
Clients: []schema.OpenIDConnectClientConfiguration{
{
ID: "client-with-invalid-secret_standard",
Secret: MustDecodeSecret("$plaintext$a-secret"),
Policy: "two_factor",
RedirectURIs: []string{
"https://localhost",
},
},
},
},
}
ValidateIdentityProviders(config, validator)
assert.Len(t, validator.Errors(), 0)
require.Len(t, validator.Warnings(), 1)
assert.EqualError(t, validator.Warnings()[0], "identity_providers: oidc: client 'client-with-invalid-secret_standard': option 'secret' is plaintext but it should be a hashed value as plaintext values are deprecated and will be removed when oidc becomes stable")
}
func TestValidateIdentityProvidersShouldSetDefaultValues(t *testing.T) {
timeDay := time.Hour * 24
@ -713,7 +740,7 @@ func TestValidateIdentityProvidersShouldSetDefaultValues(t *testing.T) {
Clients: []schema.OpenIDConnectClientConfiguration{
{
ID: "a-client",
Secret: MustDecodeSecret("$plaintext$a-client-secret"),
Secret: MustDecodeSecret(goodOpenIDConnectClientSecret),
RedirectURIs: []string{
"https://google.com",
},
@ -722,7 +749,7 @@ func TestValidateIdentityProvidersShouldSetDefaultValues(t *testing.T) {
{
ID: "b-client",
Description: "Normal Description",
Secret: MustDecodeSecret("$plaintext$b-client-secret"),
Secret: MustDecodeSecret(goodOpenIDConnectClientSecret),
Policy: policyOneFactor,
UserinfoSigningAlgorithm: "RS256",
RedirectURIs: []string{
@ -745,7 +772,7 @@ func TestValidateIdentityProvidersShouldSetDefaultValues(t *testing.T) {
},
{
ID: "c-client",
Secret: MustDecodeSecret("$plaintext$a-client-secret"),
Secret: MustDecodeSecret(goodOpenIDConnectClientSecret),
RedirectURIs: []string{
"https://google.com",
},
@ -753,7 +780,7 @@ func TestValidateIdentityProvidersShouldSetDefaultValues(t *testing.T) {
},
{
ID: "d-client",
Secret: MustDecodeSecret("$plaintext$a-client-secret"),
Secret: MustDecodeSecret(goodOpenIDConnectClientSecret),
RedirectURIs: []string{
"https://google.com",
},
@ -761,7 +788,7 @@ func TestValidateIdentityProvidersShouldSetDefaultValues(t *testing.T) {
},
{
ID: "e-client",
Secret: MustDecodeSecret("$plaintext$a-client-secret"),
Secret: MustDecodeSecret(goodOpenIDConnectClientSecret),
RedirectURIs: []string{
"https://google.com",
},
@ -1019,4 +1046,6 @@ AQmB98tdGLggbyXiODV2h+Rd37aFGb0QHzerIIsVNtMwlPCcp733D4kWJqTUYWZ+
KBL3XEahgs6Os5EYZ4aBAkEAjKE+2/nBYUdHVusjMXeNsE5rqwJND5zvYzmToG7+
xhv4RUAe4dHL4IDQoQRjhr3Nw+JYvtzBx0Iq/178xMnGKg==
-----END RSA PRIVATE KEY-----`
goodOpenIDConnectClientSecret = "$pbkdf2-sha512$310000$c8p78n7pUMln0jzvd4aK4Q$JNRBzwAo0ek5qKn50cFzzvE9RXV88h1wJn5KGiHrD0YKtZaR/nCb2CJPOsKaPK0hjf.9yHxzQGZziziccp6Yng" //nolint:gosec
)