Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
adriansr committed Mar 3, 2020
1 parent 629fbec commit 6079341
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 24 deletions.
2 changes: 1 addition & 1 deletion x-pack/filebeat/input/o365audit/auth/cert.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func loadConfigCerts(cfg tlscommon.CertificateConfig) (cert *x509.Certificate, k
if err != nil {
return nil, nil, errors.Wrapf(err, "error loading X509 certificate from '%s'", cfg.Certificate)
}
if tlsCert == nil || len(tlsCert.Certificate) < 1 {
if tlsCert == nil || len(tlsCert.Certificate) == 0 {
return nil, nil, fmt.Errorf("no certificates loaded from '%s'", cfg.Certificate)
}
cert, err = x509.ParseCertificate(tlsCert.Certificate[0])
Expand Down
2 changes: 2 additions & 0 deletions x-pack/filebeat/input/o365audit/auth/secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"github.com/pkg/errors"
)

// NewProviderFromClientSecret returns a token provider that uses a secret
// for authentication.
func NewProviderFromClientSecret(endpoint, resource, applicationID, tenantID, secret string) (p TokenProvider, err error) {
oauth, err := adal.NewOAuthConfig(endpoint, tenantID)
if err != nil {
Expand Down
33 changes: 13 additions & 20 deletions x-pack/filebeat/input/o365audit/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,14 @@ type Config struct {

// TenantID (aka. Directory ID) is a list of tenants for which to fetch
// the audit logs. This can be a string or a list of strings.
TenantID interface{} `config:"tenant_id,replace" validate:"required"`
TenantID stringList `config:"tenant_id,replace" validate:"required"`

// Content-Type is a list of content-types to fetch.
// This can be a string or a list of strings.
ContentType interface{} `config:"content_type,replace"`
ContentType stringList `config:"content_type,replace"`

// API contains settings to adapt to changes on the API.
API APIConfig `config:"api"`

tenants []string
contentTypes []string
}

// APIConfig contains advanced settings that are only supposed to be changed
Expand Down Expand Up @@ -149,36 +146,32 @@ func (c *Config) Validate() (err error) {
return errors.Wrap(err, "invalid certificate config")
}
}
if c.tenants, err = asStringList(c.TenantID); err != nil {
return errors.Wrap(err, "error validating tenant_id")
}
if c.contentTypes, err = asStringList(c.ContentType); err != nil {
return errors.Wrap(err, "error validating content_type")
}
return nil
}

// A helper to allow defining a field either as a string or a list of strings.
func asStringList(value interface{}) (list []string, err error) {
type stringList []string

// Unpack populates the stringList with either a single string value or an array.
func (s *stringList) Unpack(value interface{}) error {
switch v := value.(type) {
case string:
list = []string{v}
*s = []string{v}
case []string:
list = v
*s = v
case []interface{}:
list = make([]string, len(v))
*s = make([]string, len(v))
for idx, ival := range v {
str, ok := ival.(string)
if !ok {
return nil, fmt.Errorf("string value required. Found %v (type %T) at position %d",
return fmt.Errorf("string value required. Found %v (type %T) at position %d",
ival, ival, idx+1)
}
list[idx] = str
(*s)[idx] = str
}
default:
return nil, fmt.Errorf("array of strings required. Found %v (type %T)", value, value)
return fmt.Errorf("array of strings required. Found %v (type %T)", value, value)
}
return list, nil
return nil
}

// NewTokenProvider returns an auth.TokenProvider for the given tenantID.
Expand Down
6 changes: 3 additions & 3 deletions x-pack/filebeat/input/o365audit/input.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,17 +108,17 @@ func newInput(
}()

pollers := make(map[stream]*poll.Poller)
for _, tenantID := range config.tenants {
for _, tenantID := range config.TenantID {
// MaxRequestsPerMinute limitation is per tenant.
delay := time.Duration(len(config.contentTypes)) * time.Minute / time.Duration(config.API.MaxRequestsPerMinute)
delay := time.Duration(len(config.ContentType)) * time.Minute / time.Duration(config.API.MaxRequestsPerMinute)
auth, err := config.NewTokenProvider(tenantID)
if err != nil {
return nil, err
}
if _, err = auth.Token(); err != nil {
return nil, errors.Wrapf(err, "unable to acquire authentication token for tenant:%s", tenantID)
}
for _, contentType := range config.contentTypes {
for _, contentType := range config.ContentType {
key := stream{
tenantID: tenantID,
contentType: contentType,
Expand Down

0 comments on commit 6079341

Please sign in to comment.