Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Filebeat][httpjson] httpjson oauth2 authentication mechanism for salesforce events #29087

Merged
merged 10 commits into from
Dec 2, 2021
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
- Add support in aws-s3 input for custom script parsing of s3 notifications. {pull}28946[28946]
- Improve error handling in aws-s3 input for malformed s3 notifications. {issue}28828[28828] {pull}28946[28946]
- Add support for parsers on journald input {pull}29070[29070]
- Add support in httpjson input for oAuth2ProviderDefault of password grant_type. {pull}29087[29087]

*Heartbeat*

Expand Down
29 changes: 29 additions & 0 deletions x-pack/filebeat/docs/inputs/input-httpjson.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,19 @@ filebeat.inputs:
request.url: http://localhost
----

["source","yaml",subs="attributes"]
----
filebeat.inputs:
- type: httpjson
auth.oauth2:
client.id: 12345678901234567890abcdef
client.secret: abcdef12345678901234567890
token_url: http://localhost/oauth2/token
user: [email protected]
password: P@$$W0₹D
request.url: http://localhost
----

[[input-state]]
==== Input state

Expand Down Expand Up @@ -265,6 +278,22 @@ except if using `google` as provider. Required for providers: `default`, `azure`
The client secret used as part of the authentication flow. It is always required
except if using `google` as provider. Required for providers: `default`, `azure`.

[float]
==== `auth.oauth2.user`

The user used as part of the authentication flow. It is required for authentication
- grant type password. It is only available for provider `default`.

[float]
==== `auth.oauth2.password`

The password used as part of the authentication flow. It is required for authentication
- grant type password. It is only available for provider `default`.

NOTE: user and password are required for grant_type password. If user and
password is not used then it will automatically use the `token_url` and
`client credential` method.

[float]
==== `auth.oauth2.scopes`

Expand Down
47 changes: 39 additions & 8 deletions x-pack/filebeat/input/httpjson/config_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ import (
"github.com/elastic/beats/v7/libbeat/common"
)

// authStyleInParams sends the "client_id" and "client_secret" in the POST body as application/x-www-form-urlencoded parameters.
const authStyleInParams = 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to add a brief comment in here to know what is used for, and maybe declare it local to where it is going to be used if not expected to be reused anymore.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, Got it.
Thanks

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. added one liner describing the context.


type authConfig struct {
Basic *basicAuthConfig `config:"basic"`
OAuth2 *oAuth2Config `config:"oauth2"`
Expand Down Expand Up @@ -83,9 +86,11 @@ type oAuth2Config struct {
ClientID string `config:"client.id"`
ClientSecret string `config:"client.secret"`
EndpointParams map[string][]string `config:"endpoint_params"`
Password string `config:"password"`
Provider oAuth2Provider `config:"provider"`
Scopes []string `config:"scopes"`
TokenURL string `config:"token_url"`
User string `config:"user"`

// google specific
GoogleCredentialsFile string `config:"google.credentials_file"`
Expand All @@ -103,20 +108,43 @@ func (o *oAuth2Config) isEnabled() bool {
return o != nil && (o.Enabled == nil || *o.Enabled)
}

// clientCredentialsGrant creates http client from token_url and client credentials
func (o *oAuth2Config) clientCredentialsGrant(ctx context.Context, client *http.Client) *http.Client {
creds := clientcredentials.Config{
ClientID: o.ClientID,
ClientSecret: o.ClientSecret,
TokenURL: o.getTokenURL(),
Scopes: o.Scopes,
EndpointParams: o.getEndpointParams(),
}
return creds.Client(ctx)
}

// Client wraps the given http.Client and returns a new one that will use the oauth authentication.
func (o *oAuth2Config) client(ctx context.Context, client *http.Client) (*http.Client, error) {
ctx = context.WithValue(ctx, oauth2.HTTPClient, client)

switch o.getProvider() {
case oAuth2ProviderAzure, oAuth2ProviderDefault:
creds := clientcredentials.Config{
ClientID: o.ClientID,
ClientSecret: o.ClientSecret,
TokenURL: o.getTokenURL(),
Scopes: o.Scopes,
EndpointParams: o.getEndpointParams(),
case oAuth2ProviderDefault:
if o.User != "" || o.Password != "" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it also valid for Azure?

Do you think it's worth considering introducing another type: oauth2ProviderSalesforce?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure but they do support username-password authentication here.
If its valid then that means they will also be able to use another authentication mechanism.
I will look into it if its not ok then we can add oauth2ProviderSalesforce

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think it's worth focusing on the oauth2ProviderSalesforce provider now?

Copy link
Collaborator Author

@kush-elastic kush-elastic Nov 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we have another option.
what if we use oAuth2ProviderDefault:

	case oAuth2ProviderDefault:
		if o.User != "" || o.Password != "" {
			conf := &oauth2.Config{
				ClientID:     o.ClientID,
				ClientSecret: o.ClientSecret,
				Endpoint: oauth2.Endpoint{
					TokenURL:  o.TokenURL,
					AuthStyle: AuthStyleInParams,
				},
			}
			token, err := conf.PasswordCredentialsToken(ctx, o.User, o.Password)
			if err != nil {
				return nil, fmt.Errorf("oauth2 client: error loading credentials using user and password: %w", err)
			}
			return conf.Client(ctx, token), nil
		} else {
			creds := clientcredentials.Config{
				ClientID:       o.ClientID,
				ClientSecret:   o.ClientSecret,
				TokenURL:       o.getTokenURL(),
				Scopes:         o.Scopes,
				EndpointParams: o.getEndpointParams(),
			}
			return creds.Client(ctx), nil
		}
	case oAuth2ProviderAzure:
		creds := clientcredentials.Config{
			ClientID:       o.ClientID,
			ClientSecret:   o.ClientSecret,
			TokenURL:       o.getTokenURL(),
			Scopes:         o.Scopes,
			EndpointParams: o.getEndpointParams(),
		}
		return creds.Client(ctx), nil

What do you think if we do this?
Instead creating new provider we can use default one and user will also able use user-password method for other than salesforce.

Copy link
Contributor

@mtojek mtojek Nov 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RFC defines it (Resource Owner Password Credentials Grant), so I don't see a reason why don't apply it also here. Feel free to modify the source code.

conf := &oauth2.Config{
ClientID: o.ClientID,
ClientSecret: o.ClientSecret,
mtojek marked this conversation as resolved.
Show resolved Hide resolved
Endpoint: oauth2.Endpoint{
TokenURL: o.TokenURL,
AuthStyle: authStyleInParams,
},
}
token, err := conf.PasswordCredentialsToken(ctx, o.User, o.Password)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any ideas for covering this logic with unit tests?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as an idea, if we want to test this bit in isolation, could get extracted to its own function and validate the resulting token

if err != nil {
return nil, fmt.Errorf("oauth2 client: error loading credentials using user and password: %w", err)
}
return conf.Client(ctx, token), nil
} else {
return o.clientCredentialsGrant(ctx, client), nil
}
return creds.Client(ctx), nil
case oAuth2ProviderAzure:
return o.clientCredentialsGrant(ctx, client), nil
case oAuth2ProviderGoogle:
if o.GoogleJWTFile != "" {
cfg, err := google.JWTConfigFromJSON(o.GoogleCredentialsJSON, o.Scopes...)
Expand Down Expand Up @@ -184,6 +212,9 @@ func (o *oAuth2Config) Validate() error {
if o.TokenURL == "" || o.ClientID == "" || o.ClientSecret == "" {
return errors.New("both token_url and client credentials must be provided")
}
if (o.User != "" && o.Password == "") || (o.User == "" && o.Password != "") {
mtojek marked this conversation as resolved.
Show resolved Hide resolved
return errors.New("both user and password credentials must be provided")
}
default:
return fmt.Errorf("unknown provider %q", o.getProvider())
}
Expand Down
42 changes: 42 additions & 0 deletions x-pack/filebeat/input/httpjson/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,48 @@ func TestConfigOauth2Validation(t *testing.T) {
"auth.oauth2": map[string]interface{}{},
},
},
{
name: "if user and password is set oauth2 must use user-password authentication",
input: map[string]interface{}{
"auth.oauth2": map[string]interface{}{
"user": "a_client_user",
"password": "a_client_password",
"token_url": "localhost",
"client": map[string]interface{}{
"id": "a_client_id",
"secret": "a_client_secret",
},
},
},
},
{
name: "if user is set password credentials must be set for user-password authentication",
expectedErr: "both user and password credentials must be provided accessing 'auth.oauth2'",
input: map[string]interface{}{
"auth.oauth2": map[string]interface{}{
"user": "a_client_user",
"token_url": "localhost",
"client": map[string]interface{}{
"id": "a_client_id",
"secret": "a_client_secret",
},
},
},
},
{
name: "if password is set user credentials must be set for user-password authentication",
expectedErr: "both user and password credentials must be provided accessing 'auth.oauth2'",
input: map[string]interface{}{
"auth.oauth2": map[string]interface{}{
"password": "a_client_password",
"token_url": "localhost",
"client": map[string]interface{}{
"id": "a_client_id",
"secret": "a_client_secret",
},
},
},
},
{
name: "must fail with an unknown provider",
expectedErr: "unknown provider \"unknown\" accessing 'auth.oauth2'",
Expand Down