Skip to content

Commit

Permalink
refactor!: Rework SecretProvider interface so App/Device Services hav…
Browse files Browse the repository at this point in the history
…e limited API (#508)

BREAKING CHANGE: Service that need full SecretProvider API now use SecretProviderExt. Extra APIs have been removed for App/Device Services.

closes #398

Signed-off-by: Leonard Goodell <[email protected]>
  • Loading branch information
Lenny Goodell authored Apr 11, 2023
1 parent e7fa2d7 commit d95cec1
Show file tree
Hide file tree
Showing 10 changed files with 74 additions and 33 deletions.
4 changes: 2 additions & 2 deletions bootstrap/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func RunAndReturnWaitGroup(

envVars := environment.NewVariables(lc)

var secretProvider interfaces.SecretProvider
var secretProvider interfaces.SecretProviderExt
if useSecretProvider {
secretProvider, err = secret.NewSecretProvider(serviceConfig, envVars, ctx, startupTimer, dic, serviceKey)
if err != nil {
Expand Down Expand Up @@ -180,7 +180,7 @@ func RunAndReturnWaitGroup(
// opportunity for the MetricsManager to have been created.
metricsManager := container.MetricsManagerFrom(dic.Get)
if metricsManager != nil {
secretProvider := container.SecretProviderFrom(dic.Get)
secretProvider := container.SecretProviderExtFrom(dic.Get)
if secretProvider != nil {
metrics := secretProvider.GetMetricsToRegister()
registerMetrics(metricsManager, metrics, lc)
Expand Down
6 changes: 3 additions & 3 deletions bootstrap/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func (cp *Processor) Process(
serviceType string,
configStem string,
serviceConfig interfaces.Configuration,
secretProvider interfaces.SecretProvider) error {
secretProvider interfaces.SecretProviderExt) error {

cp.overwriteConfig = cp.flags.OverwriteConfig()
configProviderUrl := cp.flags.ConfigProviderUrl()
Expand Down Expand Up @@ -407,7 +407,7 @@ func (cp *Processor) loadCommonConfigFromFile(
return err
}

func (cp *Processor) getAccessTokenCallback(serviceKey string, secretProvider interfaces.SecretProvider, err error, configProviderInfo *ProviderInfo) (types.GetAccessTokenCallback, error) {
func (cp *Processor) getAccessTokenCallback(serviceKey string, secretProvider interfaces.SecretProviderExt, err error, configProviderInfo *ProviderInfo) (types.GetAccessTokenCallback, error) {
var accessToken string
var getAccessToken types.GetAccessTokenCallback

Expand Down Expand Up @@ -827,7 +827,7 @@ func (cp *Processor) applyWritableUpdates(serviceConfig interfaces.Configuration
case currentInsecureSecrets != nil &&
!reflect.DeepEqual(currentInsecureSecrets, previousInsecureSecrets):
lc.Info("Insecure Secrets have been updated")
secretProvider := container.SecretProviderFrom(cp.dic.Get)
secretProvider := container.SecretProviderExtFrom(cp.dic.Get)
if secretProvider != nil {
// Find the updated secret's path and perform call backs.
updatedSecrets := getSecretNamesChanged(previousInsecureSecrets, currentInsecureSecrets)
Expand Down
14 changes: 14 additions & 0 deletions bootstrap/container/secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,17 @@ func SecretProviderFrom(get di.Get) interfaces.SecretProvider {

return provider
}

// SecretProviderExtName contains the name of the interfaces.SecretProviderExt implementation in the DIC.
var SecretProviderExtName = di.TypeInstanceToName((*interfaces.SecretProvider)(nil))

// SecretProviderExtFrom helper function queries the DIC and returns the interfaces.SecretProviderExt
// implementation.
func SecretProviderExtFrom(get di.Get) interfaces.SecretProviderExt {
provider, ok := get(SecretProviderExtName).(interfaces.SecretProviderExt)
if !ok {
return nil
}

return provider
}
4 changes: 2 additions & 2 deletions bootstrap/handlers/auth_middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ import (
//
// For typical usage, it is preferred to use AutoConfigAuthenticationFunc which
// will automatically select between a real and a fake JWT validation handler.
func VaultAuthenticationHandlerFunc(secretProvider interfaces.SecretProvider, lc logger.LoggingClient) func(inner http.HandlerFunc) http.HandlerFunc {
func VaultAuthenticationHandlerFunc(secretProvider interfaces.SecretProviderExt, lc logger.LoggingClient) func(inner http.HandlerFunc) http.HandlerFunc {
return func(inner http.HandlerFunc) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
authHeader := r.Header.Get("Authorization")
Expand Down Expand Up @@ -89,7 +89,7 @@ func NilAuthenticationHandlerFunc() func(inner http.HandlerFunc) http.HandlerFun
// to disable JWT validation. This might be wanted for an EdgeX
// adopter that wanted to only validate JWT's at the proxy layer,
// or as an escape hatch for a caller that cannot authenticate.
func AutoConfigAuthenticationFunc(secretProvider interfaces.SecretProvider, lc logger.LoggingClient) func(inner http.HandlerFunc) http.HandlerFunc {
func AutoConfigAuthenticationFunc(secretProvider interfaces.SecretProviderExt, lc logger.LoggingClient) func(inner http.HandlerFunc) http.HandlerFunc {
// Golang standard library treats an error as false
disableJWTValidation, _ := strconv.ParseBool(os.Getenv("EDGEX_DISABLE_JWT_VALIDATION"))
authenticationHook := NilAuthenticationHandlerFunc()
Expand Down
2 changes: 1 addition & 1 deletion bootstrap/handlers/clients.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (cb *ClientsBootstrap) BootstrapHandler(
lc := container.LoggingClientFrom(dic.Get)
config := container.ConfigurationFrom(dic.Get)
cb.registry = container.RegistryFrom(dic.Get)
jwtSecretProvider := secret.NewJWTSecretProvider(container.SecretProviderFrom(dic.Get))
jwtSecretProvider := secret.NewJWTSecretProvider(container.SecretProviderExtFrom(dic.Get))

for serviceKey, serviceInfo := range config.GetBootstrap().Clients {
var url string
Expand Down
33 changes: 19 additions & 14 deletions bootstrap/interfaces/secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,29 +12,21 @@
* the License.
*******************************************************************************/package interfaces

import (
"time"
)
import "time"

// SecretProvider defines the contract for secret provider implementations that
// allow secrets to be retrieved/stored from/to a services Secret Store.
// allow secrets to be retrieved/stored from/to a services Secret Store and other secret related APIs.
// This interface is limited to the APIs that individual service code need.
type SecretProvider interface {
// StoreSecret stores new secrets into the service's SecretStore at the specified secretName.
StoreSecret(secretName string, secrets map[string]string) error

// GetSecret retrieves secrets from the service's SecretStore at the specified secretName.
GetSecret(secretName string, keys ...string) (map[string]string, error)

// SecretsUpdated sets the secrets last updated time to current time.
SecretsUpdated()

// SecretsLastUpdated returns the last time secrets were updated
SecretsLastUpdated() time.Time

// GetAccessToken return an access token for the specified token type and service key.
// Service key is use as the access token role which must have be previously setup.
GetAccessToken(tokenType string, serviceKey string) (string, error)

// ListSecretNames returns a list of secretNames for the current service from an insecure/secure secret store.
ListSecretNames() ([]string, error)

Expand All @@ -44,11 +36,24 @@ type SecretProvider interface {
// RegisteredSecretUpdatedCallback registers a callback for a secret.
RegisteredSecretUpdatedCallback(secretName string, callback func(path string)) error

// SecretUpdatedAtSecretName performs updates and callbacks for an updated secret or secretName.
SecretUpdatedAtSecretName(secretName string)

// DeregisterSecretUpdatedCallback removes a secret's registered callback secretName.
DeregisterSecretUpdatedCallback(secretName string)
}

// SecretProviderExt defines the extended contract for secret provider implementations that
// provide additional APIs needed only from the bootstrap code.
type SecretProviderExt interface {
SecretProvider

// SecretsUpdated sets the secrets last updated time to current time.
SecretsUpdated()

// GetAccessToken return an access token for the specified token type and service key.
// Service key is use as the access token role which must have be previously setup.
GetAccessToken(tokenType string, serviceKey string) (string, error)

// SecretUpdatedAtSecretName performs updates and callbacks for an updated secret or secretName.
SecretUpdatedAtSecretName(secretName string)

// GetMetricsToRegister returns all metric objects that needs to be registered.
GetMetricsToRegister() map[string]interface{}
Expand Down
2 changes: 1 addition & 1 deletion bootstrap/registration/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func createRegistryClient(
var accessToken string
var getAccessToken registryTypes.GetAccessTokenCallback

secretProvider := container.SecretProviderFrom(dic.Get)
secretProvider := container.SecretProviderExtFrom(dic.Get)
// secretProvider will be nil if not configured to be used. In that case, no access token required.
if secretProvider != nil {
// Define the callback function to retrieve the Access Token
Expand Down
4 changes: 2 additions & 2 deletions bootstrap/secret/jwtprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ import (
)

type jwtSecretProvider struct {
secretProvider interfaces.SecretProvider
secretProvider interfaces.SecretProviderExt
}

func NewJWTSecretProvider(secretProvider interfaces.SecretProvider) clientInterfaces.AuthenticationInjector {
func NewJWTSecretProvider(secretProvider interfaces.SecretProviderExt) clientInterfaces.AuthenticationInjector {
return &jwtSecretProvider{
secretProvider: secretProvider,
}
Expand Down
9 changes: 7 additions & 2 deletions bootstrap/secret/secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,10 @@ func NewSecretProvider(
ctx context.Context,
startupTimer startup.Timer,
dic *di.Container,
serviceKey string) (interfaces.SecretProvider, error) {
serviceKey string) (interfaces.SecretProviderExt, error) {
lc := container.LoggingClientFrom(dic.Get)

var provider interfaces.SecretProvider
var provider interfaces.SecretProviderExt

switch IsSecurityEnabled() {
case true:
Expand Down Expand Up @@ -134,9 +134,14 @@ func NewSecretProvider(
}

dic.Update(di.ServiceConstructorMap{
// Must put the SecretProvider instance in the DIC for both the standard API use by service code
// and the extended API used by boostrap code
container.SecretProviderName: func(get di.Get) interface{} {
return provider
},
container.SecretProviderExtName: func(get di.Get) interface{} {
return provider
},
})

return provider, nil
Expand Down
29 changes: 23 additions & 6 deletions bootstrap/secret/secret_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,16 @@ import (
"github.com/stretchr/testify/require"
)

const (
expectedUsername = "admin"
expectedPassword = "password"
expectedSecretName = "redisdb"
)
const expectedUsername = "admin"
const expectedPassword = "password"
const expectedSecretName = "redisdb"
const expectedInsecureJWT = "" // Empty when in non-secure mode
const expectedSecureJWT = "secureJwtToken"

// nolint: gosec
var testTokenResponse = `{"auth":{"accessor":"9OvxnrjgV0JTYMeBreak7YJ9","client_token":"s.oPJ8uuJCkTRb2RDdcNova8wg","entity_id":"","lease_duration":3600,"metadata":{"edgex-service-name":"edgex-core-data"},"orphan":true,"policies":["default","edgex-service-edgex-core-data"],"renewable":true,"token_policies":["default","edgex-service-edgex-core-data"],"token_type":"service"},"data":null,"lease_duration":0,"lease_id":"","renewable":false,"request_id":"ee749ee1-c8bf-6fa9-3ed5-644181fc25b0","warnings":null,"wrap_info":null}`
var expectedSecrets = map[string]string{UsernameKey: expectedUsername, PasswordKey: expectedPassword}
var expectedSecureJwtData = map[string]string{"token": expectedSecureJWT}

func TestNewSecretProvider(t *testing.T) {
tests := []struct {
Expand All @@ -73,6 +74,7 @@ func TestNewSecretProvider(t *testing.T) {
})

var configuration interfaces.Configuration
expectedJWT := expectedInsecureJWT

if tc.Secure == "true" {
testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
Expand All @@ -86,6 +88,12 @@ func TestNewSecretProvider(t *testing.T) {
data["data"] = expectedSecrets
response, _ := json.Marshal(data)
_, _ = w.Write(response)
case "/v1/identity/oidc/token/testServiceKey":
w.WriteHeader(http.StatusOK)
data := make(map[string]interface{})
data["data"] = expectedSecureJwtData
response, _ := json.Marshal(data)
_, _ = w.Write(response)
default:
w.WriteHeader(http.StatusNotFound)
}
Expand All @@ -103,6 +111,8 @@ func TestNewSecretProvider(t *testing.T) {
return mockTokenLoader
},
})

expectedJWT = expectedSecureJWT
} else {
configuration = TestConfig{
map[string]bootstrapConfig.InsecureSecretsInfo{
Expand All @@ -119,13 +129,20 @@ func TestNewSecretProvider(t *testing.T) {
actual, err := NewSecretProvider(configuration, envVars, context.Background(), timer, dic, "testServiceKey")
require.NoError(t, err)
require.NotNil(t, actual)

actualProvider := container.SecretProviderFrom(dic.Get)
assert.NotNil(t, actualProvider)

actualSecrets, err := actualProvider.GetSecret(expectedSecretName)
require.NoError(t, err)
assert.Equal(t, expectedUsername, actualSecrets[UsernameKey])
assert.Equal(t, expectedPassword, actualSecrets[PasswordKey])

actualProviderExt := container.SecretProviderExtFrom(dic.Get)
assert.NotNil(t, actualProviderExt)

actualJWT, err := actualProviderExt.GetSelfJWT()
require.NoError(t, err)
assert.Equal(t, expectedJWT, actualJWT)
})
}
}
Expand Down

0 comments on commit d95cec1

Please sign in to comment.