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

OAuth2 Implementation #15

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Conversation

Shubhangi-cs
Copy link
Collaborator

@Shubhangi-cs Shubhangi-cs commented Jan 12, 2024

Implement Oauth 2.0 support in SAP Successfactors Batch Source Plugin. SAP SuccessFactors supports OAuth 2.0 to authenticate OData API and SFAPI users. Compared with HTTP Basic Auth, OAuth 2.0 is considered to be more secure in that it doesn't require users to provide their passwords during authentication. With OAuth 2.0, you can also use a third-party identity provider (IDP) for user management and provisioning.

docs/SuccessFactors-batchsource.md Outdated Show resolved Hide resolved
docs/SuccessFactors-batchsource.md Outdated Show resolved Hide resolved
pom.xml Outdated
@@ -20,7 +20,7 @@

<groupId>io.cdap.plugin.successfactors</groupId>
<artifactId>successfactors-plugins</artifactId>
<version>1.2.0-SNAPSHOT</version>
<version>1.2.1-SNAPSHOT</version>

Choose a reason for hiding this comment

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

Lets bump it to v1.3.0-SNAPSHOT as we are adding new feature support.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

usually this change is committed in PRs, reverted back

pom.xml Outdated
<dependency>
<groupId>org.apache.httpcomponents</groupId>
<artifactId>httpclient</artifactId>
<version>4.5.13</version> <!-- Use the latest version available -->

Choose a reason for hiding this comment

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

make use of properties and remove the comment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

Choose a reason for hiding this comment

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

Use the properties for other dependencies also plus please remove the comments i.e.

  • <!-- Use the latest version available -->
  • <!-- https://mvnrepository.com/artifact/commons-codec/commons-codec -->

pom.xml Show resolved Hide resolved
@Shubhangi-cs Shubhangi-cs force-pushed the Oauth2_Implementation branch from 3863117 to a8b0b98 Compare January 16, 2024 13:43
@@ -19,8 +19,21 @@ annotating metadata, etc.
**Use Connection:** Whether to use a connection. If a connection is used, you do not need to provide the credentials.
**Connection:** Name of the connection to use. Entity Names information will be provided by the connection.
You also can use the macro function ${conn(connection-name)}.
**Authentication Type:** Type of authentication used to submit request. OAuth2, Basic Authentication types are available.

Choose a reason for hiding this comment

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

Rewrite it to: Authentication type used to submit request. Supported types are Basic & OAuth 2.0. Default is Basic Authentication.

pom.xml Outdated
<dependency>
<groupId>org.apache.httpcomponents</groupId>
<artifactId>httpclient</artifactId>
<version>4.5.13</version> <!-- Use the latest version available -->

Choose a reason for hiding this comment

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

Use the properties for other dependencies also plus please remove the comments i.e.

  • <!-- Use the latest version available -->
  • <!-- https://mvnrepository.com/artifact/commons-codec/commons-codec -->

public static final String TEST = "TEST";
private static final String COMMON_ACTION = ResourceConstants.ERR_MISSING_PARAM_OR_MACRO_ACTION.getMsgForKey();
private static final String SAP_SUCCESSFACTORS_USERNAME = "SAP SuccessFactors Username";
private static final String SAP_SUCCESSFACTORS_PASSWORD = "SAP SuccessFactors Password";
private static final String SAP_SUCCESSFACTORS_BASE_URL = "SAP SuccessFactors Base URL";
private static final Logger LOG = LoggerFactory.getLogger(SuccessFactorsConnectorConfig.class);

@Name(PROPERTY_AUTH_TYPE)
@Description("Type of authentication used to submit request. \n" +

Choose a reason for hiding this comment

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

nit: \n can be removed.

public static final String TEST = "TEST";
private static final String COMMON_ACTION = ResourceConstants.ERR_MISSING_PARAM_OR_MACRO_ACTION.getMsgForKey();
private static final String SAP_SUCCESSFACTORS_USERNAME = "SAP SuccessFactors Username";
private static final String SAP_SUCCESSFACTORS_PASSWORD = "SAP SuccessFactors Password";
private static final String SAP_SUCCESSFACTORS_BASE_URL = "SAP SuccessFactors Base URL";
private static final Logger LOG = LoggerFactory.getLogger(SuccessFactorsConnectorConfig.class);

@Name(PROPERTY_AUTH_TYPE)
@Description("Type of authentication used to submit request. \n" +
"OAuth2, Basic Authentication types are available.")

Choose a reason for hiding this comment

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

Lets be consistent with OAuth 2.0 in the descriptions.

}

if (authType.equals(BASIC_AUTH)) {

Choose a reason for hiding this comment

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

nit: remove the extra line.

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 (authType.equals(OAUTH2)) {

Choose a reason for hiding this comment

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

formating..

Request req = buildRequest(endpoint, mediaType);

Request req = null;
if (config.getAuthType().equals("basicAuth")) {

Choose a reason for hiding this comment

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

use the constant declared in the SuccessFactorsConnectorConfig

Choose a reason for hiding this comment

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

These updates are also required in SuccessFactors-connector.md

}

return assertion;
} catch (Exception e) {

Choose a reason for hiding this comment

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

avoid catching generic exceptions.

return accessToken;
}
}

Choose a reason for hiding this comment

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

nit: just keep 1 new line and remove any extra lines.

},
{
"id": "oAuth2",
"label": "OAuth2"

Choose a reason for hiding this comment

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

OAuth 2.0

},
{
"id": "oAuth2",
"label": "OAuth2"

Choose a reason for hiding this comment

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

OAuth 2.0

@Shubhangi-cs Shubhangi-cs force-pushed the Oauth2_Implementation branch 2 times, most recently from 37869a0 to 4083f5d Compare January 29, 2024 09:59
@Shubhangi-cs Shubhangi-cs force-pushed the Oauth2_Implementation branch from ed6fb11 to 6989161 Compare February 2, 2024 06:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants