Syndesis: [react-ui] Cannot validate salesforce connection

[mcada]

This is a...

[ ] Feature request
[x] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  
[ ] Documentation issue or request

Description

Error in meta pod:

2019-06-04 12:15:54.639  INFO 1 --- [  XNIO-3 task-7] i.s.c.s.verifier.api.ComponentVerifier   : Verify (PARAMETERS): salesforce === ERROR
2019-06-04 12:15:54.639 ERROR 1 --- [  XNIO-3 task-7] i.s.c.s.verifier.api.ComponentVerifier   : salesforce --> 
2019-06-04 12:15:54.639 ERROR 1 --- [  XNIO-3 task-7] i.s.c.s.verifier.api.ComponentVerifier   :    ILLEGAL_PARAMETER_GROUP_COMBINATION : null
2019-06-04 12:15:54.639 ERROR 1 --- [  XNIO-3 task-7] i.s.c.s.verifier.api.ComponentVerifier   :    ILLEGAL_PARAMETER_GROUP_COMBINATION : null
2019-06-04 12:15:54.639 ERROR 1 --- [  XNIO-3 task-7] i.s.c.s.verifier.api.ComponentVerifier   :    ILLEGAL_PARAMETER_GROUP_COMBINATION : null

[riccardo-forina]

Duplicate of https://github.com/syndesisio/syndesis/issues/5482

[tplevko]

This issue is still present, even after the fix of https://github.com/syndesisio/syndesis/issues/5482
It seems to be a separate issue, it's not related to the registering new salesforce connection using oauth flow.
The server log attached:
server.log

[zregvart]

Looks like clientId and clientSecret properties are not set on the connector, @tplevko have you configured OAuth settings for Salesforce connector?

[tplevko]

The salesforce connection can be created using the OAuth and the no-OAuth way. This issue is present when user tries to use the no-Oauth way, so AFAIK user shouldn't need to configure the OAuth settings.

[zregvart]

I don't actually see the need for alternative connection configuration if a connector supports OAuth we should offer only OAuth.

I think we need to remove the option of configuring a connection without OAuth for connectors that can use OAuth. In this particular example fetching the refresh token for Salesforce is pretty involved.

What do you think about removing non-OAuth connection creation from OAuth-capable connectors @gaughan?

[gaughan]

I think we need both. If your salesforce instance doesn't have OAuth you are out of options, right?
So both are valid.

[zregvart]

We require OAuth either way, that's how we connect to Salesforce -- using OAuth access token. We have a click-a-button-to-connect way, or wrestle-with-properties-trying-to-get-the-thing-to-work way. I'm suggesting that we ditch the latter.

[zregvart]

Also of interest is #2013

[tplevko]

I tend to agree - however this is not the case only for creating salesforce connection, but for all the connections that are present in the settings page (twitter, google calendar, ...). If we would want to go this way for this release, it would imply ditching all of these connections configs.
The connection creation flow in current UI would in this case require some adjustments.

[riccardo-forina]

Sorry, I'm a bit lost. Do we still need to do something about this or not?

[mcada]

Still broken on latest image.

[riccardo-forina]

Nothing has been done to allow configuring an OAuth connector without using the Settings. My question is, should we? It's my understanding that this isn't an officially supported flow and a user is expected to use the Settings all the time.

[mcada]

That is not my call, I will update tests accordingly after the decision is made.

[zregvart]

I plan on working on this, from what I understand, the root cause for this issue is that the Salesforce connection was created by filling in the parameters of the connection and not via the OAuth flow. I plan on not allowing that OAuth capable connection to be created in this way, i.e. only through the OAuth flow.

[dhirajsb]

@zregvart I don't think disallowing non-OAuth flow connections is the solution to this problem. Let me illustrate what happens with Salesforce Non-OAuth flow:

a. User provides clientId, clientSecret, password, etc.
b. User hits validate, which actually sends an empty 'refreshToken' to backend for some reason, which fails.

User never provided a value for refreshToken in the console, so it should not be used for verification in this scenario.

Now that there is a check for valid property group combinations, the same thing will happen with any other connector that doesn't expect properties other than what the user provided for verification. Even properties that aren't OAuth related will potentially break verification, like the UI seems to be sending an empty value for refreshToken.

[dhirajsb]

Basically, since backend has constraints on what property combinations can be used for verification, the UI also needs to be aware of that and follow those rules somehow.

[dhirajsb]

Found a somewhat related blocker issue to this #5763

[zregvart]

I think in Syndesis we should offer the simplest and most streamlined user experience. I don't making user enter a number of configuration options is that, at least not compared to entering just two configuration options in the OAuth settings and clicking a button. We're not loosing any functionality by doing so.

I think that #5763 is directly related to having multiple options and not having one way of creating connections. Solving that issue with and still maintaining the ability for the client to manage OAuth connection settings is making the implementation more complex.

In general I think we should not be looking at what's possible in Camel or in the Salesforce component in Camel, we should be looking at what's the simplest possible way to enable functionality for the end users. Having a single OAuth flow, and using the refresh token is that.

[dhirajsb]

I am all for simplicity of configuration, but isn't it a severe limitation that users can only use one Salesforce org and user in Syndesis? At least with the non-Oauth login approach, they can configure separate credentials for multiple connections.
Syndesis would have to add support for creating multiple credentials in the OAuth Settings page to support the second scenario.

[tplevko]

As we are in blockers only phase - I think we need to focus on the issue described - the verification of salesforce is not working.
The changes to the connection creation workflow - this can be discussed in separate issue, and we need someone to decide what will be done for this release.

wdyt? @dhirajsb @zregvart @heiko-braun

[zregvart]

We rely on the refresh token to be defined if it's not defined then when the access token expires the integration starts failing. The same is true for username and passwords -- passwords by default expire within 90 days.

If we agree that this is an issue, i.e. that integrations fail to run after a certain amount of time; and that the refresh token is required and needs to be provided then we need to entering the refresh token, which is currently hidden:

https://github.com/syndesisio/syndesis/blob/0e818c23250be06c53901cb809ea155ba6646156/app/connector/salesforce/src/main/resources/META-INF/syndesis/connector/salesforce.json#L650

Once the refresh token field is available for the user to fill in the user needs to provide one. There's an extremely difficult way of getting the refresh token from the OAuth callback URL. It goes like this:

1. User creates a Salesforce Connected App and sets the callback URL to https://login.salesforce.com/services/oauth2/success or https://test.salesforce.com/services/oauth2/success (NOT Syndesis callback URL)
2. User triggers the OAuth flow by going to https://login.salesforce.com/services/oauth2/authorize?response_type=code&client_id= ${CLIENT_ID}&redirect_uri=https%3A%2F%2Flogin.salesforce.com%2Fservices%2Foauth2%2Fsuccess
3. User logs in
4. User fetches the refresh_token query parameter from the address bar in the browser, browser should be at https://login.salesforce.com/services/oauth2/success?...refresh_token=...
5. Now user has the client id, secret and the refresh token to create the connection manually

So with that, can we agree that the the only solution to solving this issue is to enforce the OAuth flow and not allow manual entry of connection details for OAuth based connectors?

I'd like to step back and let you ponder on this, if your decision is to enable this, and then allow for even more edge cases we don't really support and awful user experience I'm fine with that however misguided that is.

[tplevko]

I think really many valid points were raised in this discussion and I just want to note that for QE - we are OK with either way of solving this issue - we just would like the decision to be made. @gaughan

1. fix the current salesforce connection validation functionality
2. drop the whole no-oauth way of creating connections and provide only the oauth way for the users

We just need the decision for 7.4. release, as this is blocking many usecases we have covered in tests.

That said, in case of the second option - should there be a migration plan for integrations using connections created the no-oauth way in previous release deployments?

[zregvart]

That said, in case of the second option - should there be a migration plan for integrations using connections created the no-oauth way in previous release deployments?

It is my humble opinion that the number of manually created Salesforce connections is not totally unlike zero. (We only document the OAuth flow).