[PLUGIN-1741] SuccessFactors - OAuth2 Implementation #34

Shubhangi-cs · 2024-01-29T06:55:58Z

SuccessFactors OAuth2 Feature Implemenataion

Description

Succfactors currently uses basic auth where are user can enter username and password for auth. This PR adds a new option to let user use OAuth for auth.

What is assertion token and auth token ?

Assesertion Token
- A Base64 encoded xml that contains metadata about the auth token, signed by a private key to as proof of ownership, this is used to authenticate while generating the auth token.
Auth token
- A token generated using assertion token and OAuth backchanel flow that can be used as a bearer token to authenticate user with successfactors APIs.

UI Field

Modified SuccessFactors-batchsource.json
Modified SuccessFactors-connector.json

New Fields added

Authentication Type
- Let's user toggle between basic auth (default) or OAuth2
Assertion Token Type
- Let's the user either use their own assertion token or creat one on the fly
Token URL
Client ID
Private Key
Expire Assertion Token In (Minutes)
User ID
Company ID
Assertion Token

Docs

Modified SuccessFactors-batchsource.md
Modified SuccessFactors-connector.md

Code change

Added SuccessFactorsAccessToken.java
Modified ResourceConstants.java
Modified SuccessFactorsConnectorConfig.java
Modified SuccessFactorsSource.java
Modified SuccessFactorsPluginConfig.java
Modified SuccessFactorsService.java
Modified SuccessFactorsTransporter.java

Unit Tests

Modified SuccessFactorsConnectorTest.java
Modified SuccessFactorsSourceTest.java
Modified SuccessFactorsPluginConfigTest.java
Modified SuccessFactorsInputFormatTest.java
Modified SuccessFactorsSchemaGeneratorTest.java
Modified RuntimeFunctionalForAssociatedEntityTest.java
Modified RuntimeFunctionalTest.java
Modified SuccessFactorsTransporterTest.java
Modified SuccessFactorsUrlContainerTest.java

Sanity Tests

Tested pipeline with OAuth2 creds
Tested pipeline with proxy and OAuth2
Tested pipeline with Enter Token and Create Token Modes

itsankit-google

Please rebase the changes, resolve the conflicts and ensure build is green.

itsankit-google · 2024-04-03T07:59:22Z

src/main/java/io/cdap/plugin/successfactors/common/util/SuccessFactorsAccessToken.java

@@ -0,0 +1,391 @@
+/*
+ * Copyright © 2022 Cask Data, Inc.


please update year in the license.

Year update to 2025

itsankit-google · 2024-04-03T08:00:54Z

pom.xml

+    <dependency>
+      <groupId>org.apache.httpcomponents</groupId>
+      <artifactId>httpclient</artifactId>
+      <version>${httpclient.version}</version>
+    </dependency>
+    <dependency>
+      <groupId>org.opensaml</groupId>
+      <artifactId>xmltooling</artifactId>
+      <version>1.4.4</version>
+    </dependency>
+    <dependency>
+      <groupId>org.opensaml</groupId>
+      <artifactId>openws</artifactId>
+      <version>1.5.4</version>
+    </dependency>
+    <dependency>
+      <groupId>org.opensaml</groupId>
+      <artifactId>opensaml</artifactId>
+      <version>2.6.4</version>
+    </dependency>
+    <dependency>
+      <groupId>commons-codec</groupId>
+      <artifactId>commons-codec</artifactId>
+      <version>1.10</version>
+    </dependency>
+    <dependency>
+      <groupId>xml-security</groupId>
+      <artifactId>xmlsec</artifactId>
+      <version>1.3.0</version>
+    </dependency>
+    <dependency>
+      <groupId>commons-collections</groupId>
+      <artifactId>commons-collections</artifactId>
+      <version>3.2.2</version>
+    </dependency>
+    <dependency>
+      <groupId>commons-lang</groupId>
+      <artifactId>commons-lang</artifactId>
+      <version>2.1</version>
+    </dependency>
+    <dependency>
+      <groupId>commons-logging</groupId>
+      <artifactId>commons-logging</artifactId>
+      <version>1.1</version>
+    </dependency>
+    <dependency>
+      <groupId>org.apache.velocity</groupId>
+      <artifactId>velocity</artifactId>
+      <version>1.5</version>
+      <type>pom</type>
+    </dependency>


How are the versions of these dependencies decided?

Is the same code being used somewhere in any other existing plugin?

Please add comments for every dependency where it is used.

We only need opensaml for which we are using 2.6.4 as this is the latest version on oss.sonatype.
I have removed all the other dependencies.

itsankit-google · 2024-04-03T08:03:21Z

widgets/SuccessFactors-connector.json

+        "property": "authType",
+        "operator": "equal to",
+        "value": "basicAuth"


authType will be null in existing pipelines since it is a newly added config.

We should default to basicAuth when authType is also null.

Does "equal to" with will "null" ?
Can you provide reference to this.

you can use filter as we do for Connection widget in other plugins

Updated

{ "name": "basicAuth", "condition": { "expression": "authType == 'basicAuth' || authType == 'null'" }, "show": [ { "name": "username", "type": "property" }, { "name": "password", "type": "property" } ] }

itsankit-google · 2024-04-03T08:05:41Z

src/main/java/io/cdap/plugin/successfactors/source/config/SuccessFactorsPluginConfig.java

+                                            tokenURL, clientId, privateKey, userId, samlUsername, assertionToken,
+                                            companyId, authType, assertionTokenType, filterOption, selectOption,
+                                            expandOption, additionalQueryParameters, paginationType);


indentation looks off here.

itsankit-google · 2024-04-03T08:08:08Z

src/main/java/io/cdap/plugin/successfactors/source/SuccessFactorsSource.java

+        if (config.getConnection().getAuthType().equals(SuccessFactorsConnectorConfig.BASIC_AUTH)) {
+          failureCollector.addFailure(errMsg, null).withConfigProperty(SuccessFactorsPluginConfig.UNAME);
+          failureCollector.addFailure(errMsg, null).withConfigProperty(SuccessFactorsPluginConfig.PASSWORD);
+        } else {
+          failureCollector.addFailure(errMsg, null).withConfigProperty(SuccessFactorsConnectorConfig.COMPANY_ID);
+          failureCollector.addFailure(errMsg, null).withConfigProperty(SuccessFactorsConnectorConfig.CLIENT_ID);
+          if (config.getConnection().getAssertionTokenType().equals(SuccessFactorsConnectorConfig.ENTER_TOKEN)) {
+            failureCollector.addFailure(errMsg, null).
+              withConfigProperty(SuccessFactorsConnectorConfig.ASSERTION_TOKEN);
+          } else {
+            failureCollector.addFailure(errMsg, null).withConfigProperty(SuccessFactorsConnectorConfig.PRIVATE_KEY);
+            failureCollector.addFailure(errMsg, null).withConfigProperty(SuccessFactorsConnectorConfig.USER_ID);
+            failureCollector.addFailure(errMsg, null).withConfigProperty(SuccessFactorsConnectorConfig.TOKEN_URL);


constants should always be used first in java to avoid NPE in equals condition.

For example,

if (SuccessFactorsConnectorConfig.BASIC_AUTH.equals(config.getConnection().getAuthType())) {....}

Updated, reversed all equal checks to have constant first to avoid NPE.

itsankit-google · 2024-04-03T08:09:11Z

src/main/java/io/cdap/plugin/successfactors/source/config/SuccessFactorsPluginConfig.java

+      this.authType = authType;
+      return this;


authType will be null for existing pipelines.

It should default to BASIC_AUTH in that case.

Updated !

Default to basic auth if authType is not set for backward compatibility

this.authType = Strings.isNullOrEmpty(authType) ? SuccessFactorsConnectorConfig.BASIC_AUTH : authType;

itsankit-google

please also add unit tests for newly added configs for oauth2.

itsankit-google · 2025-01-06T09:14:02Z

src/main/java/io/cdap/plugin/successfactors/connector/SuccessFactorsConnectorConfig.java

-    if (SuccessFactorsUtil.isNullOrEmpty(getPassword()) && !containsMacro(PASSWORD)) {
-      String errMsg = ResourceConstants.ERR_MISSING_PARAM_PREFIX.getMsgForKey(SAP_SUCCESSFACTORS_PASSWORD);
-      failureCollector.addFailure(errMsg, COMMON_ACTION).withConfigProperty(PASSWORD);
+    if (BASIC_AUTH.equals(this.getAuthType())) {


nit: getAuthType()

itsankit-google · 2025-01-06T09:15:16Z

src/main/java/io/cdap/plugin/successfactors/connector/SuccessFactorsConnectorConfig.java

-      String errMsg = ResourceConstants.ERR_MISSING_PARAM_PREFIX.getMsgForKey(SAP_SUCCESSFACTORS_PASSWORD);
-      failureCollector.addFailure(errMsg, COMMON_ACTION).withConfigProperty(PASSWORD);
+    if (BASIC_AUTH.equals(this.getAuthType())) {
+      if (Strings.isNullOrEmpty(getUsername()) && !containsMacro(UNAME)) {


check containsMacro() first for all configs.

Authentication Type is not macro

The comment was all other configs which are macro to change the order of conditions, for example:

if (!containsMacro(UNAME) && Strings.isNullOrEmpty(getUsername())) .........

Updated, i misunderstood the request !

itsankit-google · 2025-01-06T09:17:16Z

src/main/java/io/cdap/plugin/successfactors/source/SuccessFactorsSource.java

@@ -46,6 +46,7 @@
 import io.cdap.plugin.successfactors.source.input.SuccessFactorsPartitionBuilder;
 import io.cdap.plugin.successfactors.source.service.SuccessFactorsService;
 import io.cdap.plugin.successfactors.source.transport.SuccessFactorsTransporter;
+


nit: remove empty line

itsankit-google · 2025-01-06T09:17:29Z

src/main/java/io/cdap/plugin/successfactors/source/SuccessFactorsSource.java

@@ -57,6 +58,7 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.stream.Collectors;
+


nit: remove empty line

src/main/java/io/cdap/plugin/successfactors/source/service/SuccessFactorsService.java

itsankit-google · 2025-01-06T09:18:51Z

src/main/java/io/cdap/plugin/successfactors/source/transport/SuccessFactorsTransporter.java

+            .handle(RetryableException.class)
+            .withBackoff(Duration.ofSeconds(initialRetryDuration),
+                    Duration.ofSeconds(maxRetryDuration), retryMultiplier)
+            .withMaxRetries(maxRetryCount)
+            .onRetry(event -> LOG.debug("Retrying SapTransportCall. Retry count: {}", event.getAttemptCount()))
+            .onSuccess(event -> LOG.debug("SapTransportCall executed successfully."))
+            .onRetriesExceeded(event -> LOG.error("Retry limit reached for SapTransportCall."))
+            .build();


revert unintended changes.

itsankit-google · 2025-01-06T15:09:04Z

src/main/java/io/cdap/plugin/successfactors/connector/SuccessFactorsConnectorConfig.java

+  public String getUserId() {
+    return userId;
+  }
+
  public String getBaseURL() {
    return baseURL;
  }

  public void validateBasicCredentials(FailureCollector failureCollector) {


This function should be renamed to validateAuthCredentials

itsankit-google · 2025-01-06T15:10:01Z

src/main/java/io/cdap/plugin/successfactors/source/SuccessFactorsSource.java

@@ -51,7 +51,6 @@
 import org.apache.hadoop.mapreduce.Job;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-


please revert unintended change

itsankit-google

minor comments otherwise LGTM. Please squash commits before merge.

Would be good to also run existing e2e tests once succesfuly before merge.

itsankit-google · 2025-01-06T15:18:02Z

src/test/java/io/cdap/plugin/successfactors/source/input/SuccessFactorsInputFormatTest.java

@@ -22,6 +22,7 @@
 import io.cdap.plugin.successfactors.source.metadata.TestSuccessFactorsUtil;
 import io.cdap.plugin.successfactors.source.service.SuccessFactorsService;
 import io.cdap.plugin.successfactors.source.transport.SuccessFactorsTransporter;
+


nit: remove empty line

Shubhangi-cs force-pushed the Oauth2_Implementation branch 2 times, most recently from ed6fb11 to 6989161 Compare February 2, 2024 06:54

itsankit-google added the build label Apr 3, 2024

itsankit-google self-requested a review April 3, 2024 07:56

itsankit-google reviewed Apr 3, 2024

View reviewed changes

psainics self-assigned this Jan 3, 2025

psainics changed the title ~~[Plugin-1741] SuccessFactors - OAuth2 Implementation~~ [PLUGIN-1741] SuccessFactors - OAuth2 Implementation Jan 3, 2025

psainics requested review from vikasrathee-cs, itsankit-google and psainics January 3, 2025 12:11

itsankit-google reviewed Jan 6, 2025

View reviewed changes

psainics requested a review from itsankit-google January 6, 2025 11:33

itsankit-google reviewed Jan 6, 2025

View reviewed changes

itsankit-google approved these changes Jan 6, 2025

View reviewed changes

itsankit-google reviewed Jan 6, 2025

View reviewed changes

psainics force-pushed the Oauth2_Implementation branch 2 times, most recently from a7772ea to f545134 Compare January 8, 2025 07:04

Add FR OAuth2.0

6e7d151

psainics force-pushed the Oauth2_Implementation branch from f545134 to 6e7d151 Compare January 8, 2025 07:05

psainics merged commit f833826 into data-integrations:develop Jan 8, 2025
3 of 5 checks passed

psainics mentioned this pull request Jan 8, 2025

[🍒][PLUGIN-1741] SuccessFactors - OAuth2 Implementation #48

Merged

[PLUGIN-1741] SuccessFactors - OAuth2 Implementation #34

[PLUGIN-1741] SuccessFactors - OAuth2 Implementation #34

Conversation

Shubhangi-cs commented Jan 29, 2024 • edited by psainics Loading

SuccessFactors OAuth2 Feature Implemenataion

Description

What is assertion token and auth token ?

UI Field

New Fields added

Docs

Code change

Unit Tests

Sanity Tests

itsankit-google 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

Choose a reason for hiding this comment

itsankit-google Jan 3, 2025 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

itsankit-google 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

itsankit-google Jan 6, 2025 • edited Loading

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

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

itsankit-google left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Shubhangi-cs commented Jan 29, 2024 •

edited by psainics

Loading

itsankit-google Jan 3, 2025 •

edited

Loading

itsankit-google Jan 6, 2025 •

edited

Loading

itsankit-google left a comment •

edited

Loading