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

kush-elastic · 2021-11-23T07:12:48Z

Enhancement

What does this PR do?

Current httpjson doesn’t have grant_type: password.
This PR contains the codebase for the authentication mechanism for httpjson input which is a part of the Salesforce connector.
Authentication Details:
Added grant_type: password in existing method with configurations.
if auth.oauth2.user and auth.oauth2.password parameter are mentioned in oauth2 configuration, code will automatically uses grant_type as password and create client accordingly.

Why is it important?

Current httpjson only support one grant_type.
Some APIs need multiple kinds of grant_type.
salesforce connector also need grant_type password for authentication.

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have made corresponding change to the default configuration files
I have added tests that prove my fix is effective or that my feature works
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Related issues

Use cases

Configuration Example:

filebeat.inputs:
- type: httpjson
  auth.oauth2:
    client.id: 12345678901234567890abcdef
    client.secret: abcdef12345678901234567890
    token_url: http://localhost/oauth2/token
    user: user@domain.tld
    password: P@$$W0₹D
  request.url: http://localhost

Screenshots

Logs

mergify · 2021-11-23T07:13:22Z

This pull request does not have a backport label. Could you fix it @kush-elastic? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-v./d./d./d is the label to automatically backport to the 7./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

elasticmachine · 2021-11-23T07:18:04Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2021-12-01T14:10:42.244+0000
Duration: 99 min 8 sec
Commit: d4d21c1

Test stats 🧪

Test	Results
Failed	0
Passed	2413
Skipped	152
Total	2565

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

mtojek

We need a changelog entry for this PR.

mtojek · 2021-11-23T09:27:59Z

x-pack/filebeat/input/httpjson/config_auth.go

@@ -82,6 +82,8 @@ type oAuth2Config struct {
 	// common oauth fields
 	ClientID       string              `config:"client.id"`
 	ClientSecret   string              `config:"client.secret"`
+	User           string              `config:"user"`


Could you please put these properties in alpha-num order?

Sure. I will update it.

mtojek · 2021-11-23T09:30:14Z

x-pack/filebeat/input/httpjson/config_auth.go

-			TokenURL:       o.getTokenURL(),
-			Scopes:         o.Scopes,
-			EndpointParams: o.getEndpointParams(),
+		if o.User != "" || o.Password != "" {


Is it also valid for Azure?

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

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

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

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.

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.

x-pack/filebeat/input/httpjson/config_auth.go

mtojek · 2021-11-23T09:36:11Z

x-pack/filebeat/input/httpjson/config_auth.go

+					AuthStyle: 1,
+				},
+			}
+			token, err := conf.PasswordCredentialsToken(ctx, o.User, o.Password)


Any ideas for covering this logic with unit tests?

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

mergify · 2021-11-23T09:36:47Z

This pull request does not have a backport label. Could you fix it @kush-elastic? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-v./d./d./d is the label to automatically backport to the 7./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

mergify · 2021-11-23T09:37:05Z

This pull request does not have a backport label. Could you fix it @kush-elastic? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-v./d./d./d is the label to automatically backport to the 7./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

botelastic · 2021-11-23T09:37:37Z

This pull request doesn't have a Team:<team> label.

mergify · 2021-11-23T09:37:39Z

This pull request does not have a backport label. Could you fix it @kush-elastic? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-v./d./d./d is the label to automatically backport to the 7./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

mtojek · 2021-11-23T09:38:37Z

/test

mtojek · 2021-11-24T08:37:00Z

x-pack/filebeat/input/httpjson/config_auth.go

-			TokenURL:       o.getTokenURL(),
-			Scopes:         o.Scopes,
-			EndpointParams: o.getEndpointParams(),
+		if o.User != "" || o.Password != "" {


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

x-pack/filebeat/input/httpjson/config_auth.go

elasticmachine · 2021-11-29T10:52:34Z

Pinging @elastic/siem (Team:SIEM)

elasticmachine · 2021-11-29T10:52:35Z

Pinging @elastic/integrations (Team:Integrations)

marc-gr

Thanks for this! Is looking good so far aside of a couple nits. Also would be nice to add the related docs in https://github.com/elastic/beats/blob/master/x-pack/filebeat/docs/inputs/input-httpjson.asciidoc for the new options and any consideration needed when using them.

marc-gr · 2021-11-29T14:33:13Z

x-pack/filebeat/input/httpjson/config_auth.go

@@ -22,6 +22,8 @@ import (
 	"github.com/elastic/beats/v7/libbeat/common"
 )

+const authStyleInParams = 1


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.

Sure, Got it.
Thanks

Done. added one liner describing the context.

marc-gr · 2021-11-29T14:35:31Z

x-pack/filebeat/input/httpjson/config_auth.go

+					AuthStyle: 1,
+				},
+			}
+			token, err := conf.PasswordCredentialsToken(ctx, o.User, o.Password)


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

mtojek

Please update also the docs: https://github.com/elastic/beats/blob/c77bf19b66a95c0bb669c06b4ffb8e7ba773a22d/x-pack/filebeat/docs/inputs/input-httpjson.asciidoc

... and I think this would be in a good shape to merge.

cla-checker-service · 2021-11-30T14:17:50Z

💚 CLA has been signed

sunny-elastic · 2021-11-30T14:51:58Z

Please update also the docs: https://github.com/elastic/beats/blob/c77bf19b66a95c0bb669c06b4ffb8e7ba773a22d/x-pack/filebeat/docs/inputs/input-httpjson.asciidoc

... and I think this would be in a good shape to merge.

Updated the doc input-httpjson.asciidoc as per new extended auth mechanism.

mtojek · 2021-11-30T14:52:13Z

x-pack/filebeat/docs/inputs/input-httpjson.asciidoc

@@ -82,6 +82,8 @@ filebeat.inputs:
    client.id: 12345678901234567890abcdef
    client.secret: abcdef12345678901234567890
    token_url: http://localhost/oauth2/token
+    user: abc.xyz@mail.com


Mail.com is a product https://www.mail.com/int/ :) Let's not suggest anything. How about user@domain.tld?

sure :) its updated.

mtojek · 2021-11-30T14:58:47Z

x-pack/filebeat/docs/inputs/input-httpjson.asciidoc

+==== `auth.oauth2.user`
+
+The user used as part of the authentication flow. It is required for authentication 
+- grant type password. It is always required except if using `google` as provider. 


It doesn't look to be correct. The user option goes in pair with password for the default provider only. @kush-elastic would you mind proofreading docs?

mergify · 2021-11-30T19:16:32Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b salesforce_httpjson_input upstream/salesforce_httpjson_input
git merge upstream/master
git push upstream salesforce_httpjson_input

…ion method

…user-passowrd method

mtojek · 2021-12-01T08:35:16Z

Wow, 234 files changed. I think you should consider rebasing against the master branch to see only changes relevant to this PR.

mtojek · 2021-12-01T14:10:02Z

/test

mtojek

LGTM. Let's merge this one if CI is happy.

…esforce events (#29087) * Add grant-type: passoword in httpjson oauth2 * refactor code and add new properties tests in config_test.go * Add grant_type: password in oAuth2ProviderDefault * Update CHANGELOG.next.asciidoc * update input-httpjson.asciidoc with new extended httpjson authentication method * add comment for authstyleparam variable * update user dummy value in the doc * Update input-httpjson.asciidoc - provider should be default only for user-passowrd method * refactor code and add new properties tests in config_test.go * Add grant_type: password in oAuth2ProviderDefault Co-authored-by: Sunny Chaudhari <sunny.chaudhari@elastic.co> (cherry picked from commit 008182a)

…esforce events (#29087) (#29246) * Add grant-type: passoword in httpjson oauth2 * refactor code and add new properties tests in config_test.go * Add grant_type: password in oAuth2ProviderDefault * Update CHANGELOG.next.asciidoc * update input-httpjson.asciidoc with new extended httpjson authentication method * add comment for authstyleparam variable * update user dummy value in the doc * Update input-httpjson.asciidoc - provider should be default only for user-passowrd method * refactor code and add new properties tests in config_test.go * Add grant_type: password in oAuth2ProviderDefault Co-authored-by: Sunny Chaudhari <sunny.chaudhari@elastic.co> (cherry picked from commit 008182a) Co-authored-by: Kush Rana <89848966+kush-elastic@users.noreply.github.com> Co-authored-by: Nicolas Ruflin <spam@ruflin.com>

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Nov 23, 2021

mergify bot added the backport-skip Skip notification from the automated backport with mergify label Nov 23, 2021

kush-elastic force-pushed the salesforce_httpjson_input branch from dcde2af to cc3cc83 Compare November 23, 2021 07:17

kush-elastic changed the title ~~[Filebeat][httpjson] Add grant-type: passoword in httpjson oauth2~~ [Filebeat][httpjson] httpjson oauth2 authentication mechanism for salesforce events Nov 23, 2021

mtojek reviewed Nov 23, 2021

View reviewed changes

mtojek removed the backport-skip Skip notification from the automated backport with mergify label Nov 23, 2021

mergify bot added the backport-skip Skip notification from the automated backport with mergify label Nov 23, 2021

mtojek added Team:Integrations Label for the Integrations team and removed backport-skip Skip notification from the automated backport with mergify labels Nov 23, 2021

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Nov 23, 2021

mergify bot added the backport-skip Skip notification from the automated backport with mergify label Nov 23, 2021

mtojek added needs_team Indicates that the issue/PR needs a Team:* label Team:SIEM and removed backport-skip Skip notification from the automated backport with mergify labels Nov 23, 2021

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Nov 23, 2021

mergify bot added the backport-skip Skip notification from the automated backport with mergify label Nov 23, 2021

mtojek requested a review from marc-gr November 23, 2021 09:37

mtojek added the backport-v8.0.0 Automated backport with mergify label Nov 23, 2021

mergify bot removed the backport-skip Skip notification from the automated backport with mergify label Nov 23, 2021

kush-elastic force-pushed the salesforce_httpjson_input branch from 2e54c4a to 8e10984 Compare November 24, 2021 08:06

mtojek reviewed Nov 24, 2021

View reviewed changes

kush-elastic closed this Nov 26, 2021

kush-elastic deleted the salesforce_httpjson_input branch November 26, 2021 06:35

marc-gr reviewed Nov 29, 2021

View reviewed changes

mtojek reviewed Nov 30, 2021

View reviewed changes

kush-elastic and others added 8 commits December 1, 2021 10:33

Add grant-type: passoword in httpjson oauth2

e2aab83

refactor code and add new properties tests in config_test.go

d38852d

Add grant_type: password in oAuth2ProviderDefault

7bbacd8

Update CHANGELOG.next.asciidoc

abfae0b

update input-httpjson.asciidoc with new extended httpjson authenticat…

d30ac17

…ion method

add comment for authstyleparam variable

0c6f0ae

update user dummy value in the doc

9bbf09d

Update input-httpjson.asciidoc - provider should be default only for …

1df0b01

…user-passowrd method

kush-elastic requested review from a team as code owners December 1, 2021 05:06

botelastic bot added the Team:Automation Label for the Observability productivity team label Dec 1, 2021

kush-elastic added 2 commits December 1, 2021 16:37

refactor code and add new properties tests in config_test.go

54735fb

Add grant_type: password in oAuth2ProviderDefault

d4d21c1

kush-elastic force-pushed the salesforce_httpjson_input branch from 4983333 to d4d21c1 Compare December 1, 2021 11:15

cachedout removed the request for review from a team December 1, 2021 11:55

mtojek approved these changes Dec 1, 2021

View reviewed changes

mtojek merged commit 008182a into elastic:master Dec 2, 2021

mergify bot mentioned this pull request Dec 2, 2021

[8.0](backport #29087) [Filebeat][httpjson] httpjson oauth2 authentication mechanism for salesforce events #29246

Merged

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

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

Conversation

kush-elastic commented Nov 23, 2021 • edited Loading

What does this PR do?

Why is it important?

Checklist

Related issues

Use cases

Screenshots

Logs

mergify bot commented Nov 23, 2021

elasticmachine commented Nov 23, 2021 • edited by jenkins-beats-ci bot Loading

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

mtojek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kush-elastic Nov 25, 2021 • edited Loading

Choose a reason for hiding this comment

mtojek Nov 25, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mergify bot commented Nov 23, 2021

mergify bot commented Nov 23, 2021

botelastic bot commented Nov 23, 2021

mergify bot commented Nov 23, 2021

mtojek commented Nov 23, 2021

Choose a reason for hiding this comment

elasticmachine commented Nov 29, 2021

elasticmachine commented Nov 29, 2021

marc-gr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mtojek left a comment

Choose a reason for hiding this comment

cla-checker-service bot commented Nov 30, 2021 • edited Loading

sunny-elastic commented Nov 30, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mtojek Nov 30, 2021 • edited Loading

Choose a reason for hiding this comment

mergify bot commented Nov 30, 2021

mtojek commented Dec 1, 2021

mtojek commented Dec 1, 2021

mtojek left a comment

Choose a reason for hiding this comment

kush-elastic commented Nov 23, 2021 •

edited

Loading

elasticmachine commented Nov 23, 2021 •

edited by jenkins-beats-ci bot

Loading

kush-elastic Nov 25, 2021 •

edited

Loading

mtojek Nov 25, 2021 •

edited

Loading

cla-checker-service bot commented Nov 30, 2021 •

edited

Loading

mtojek Nov 30, 2021 •

edited

Loading