OAuth2 support in salesforce

[mtuchi]

Summary

Add support for OAuth2 in salesforce adaptor

Details

Improved the createConnection function to handle both accessToken connection and Basic Auth connection. I have created two functions [createBasicAuthConnection and createAccessTokenConnection] that gets called in createConnection depending on configuration-schema. I have also created another configuration-schema for OAuth which requires other_params.instance_url and access_token.

Also in configuration-schema I have removed the securityToken from required property since it's an optional value

Issues #410 #423

Review Checklist

Before merging, the reviewer should check the following items:

• Does the PR do what it claims to do?
• Are there any unit tests? Should there be?
• Is there a changeset associated with this PR? Should there be? Note that
  dev only changes don't need a changeset.

[taylordowns2000]

@mtuchi , is securityToken not required anymore?

[mtuchi]

@taylordowns2000 according to jsforce, securityToken is optional - https://jsforce.github.io/document/#username-and-password-login

[taylordowns2000]

ah wow. @aleksa-krolls , can you confirm that we don't need users to provide their security token when creating a new Salesforce credential? (is this an admin setting in a client's salesforce system? something like, "require security token for API access"?)

[mtuchi]

I think most salesforce instance are configured to require securityToken but if a client have a salesforce instance that does not require securityToken then the job should pass.

The change i introduced also helps with credential creation on lighting which make the Security Token input optional

[josephjclark]

Do we log anywhere when the connection is successful?

I would much prefer to say:

Connected to salesforce as ${user}

Or

Failed to connected to salesforce as ${user}

Ie, print the result, not the process.

When we say "Attempting to..." it sort of shows a lack of confidence on our part, and when there's no log to follow it up it's gonna look pretty flaky.

For something routine, like logging in, we should be super confident in our logging. And when something goes wrong we should be super clear.

[mtuchi]

I agree on with the Connected to salesforce as ${username} but on Failure i don't think we should log Failed to connect as ${username} because this suggest that the username is wrong while it could be a number of reasons.

I would suggest we leave the error message that comes from salesforce which looks like this

INVALID_LOGIN: Invalid username, password, security token; or user locked out.

cc @josephjclark @taylordowns2000

[taylordowns2000]

@mtuchi , we added the username there (if i'm remembering this correctly) because it's incredibly helpful for the openfn end-user (the salesforce admin) to know which SF user they have to debug! users do get locked out, tokens become invalid, etc., etc.

printing the username is great for the salesforce admin. i don't think it will make them think that the username is wrong, i think it will help them figure out whether the password, token, or user status in salesforce is wrong.

[josephjclark]

I don't think that error implies that username is the fault. I agree with taylor that it's a useful bit of information to report.

I also agree that the original salesforce message is fine.

We can do both!

console.error(`Failed to connected to salesforce as ${user}`)
console.error(e.message)  // or whatever

Actually, if we we throw the error, I think the runtime should report it (and if it doesn't, that's something I need to fix!)

console.error(`Failed to connected to salesforce as ${user}`)
throw e

[mtuchi]

Okay sounds good 👍🏽 , I will use this 👇🏽

console.error(`Failed to connected to salesforce as ${user}`)
throw e

[taylordowns2000]

hrmmm, i think we're not supposed to use console.error, right? isn't it a different stream and won't it cause timing/printout issues?

[josephjclark]

@taylordowns2000 I was going to say: console.errorshould be fine - we redirect it to stdout anyway.

... but then I remembered that this doesn't use our logger, so yes it will go to a different stream and cause timing problems.

I don't think that's the fault of the adaptor though. Let me just dig into something here.

[josephjclark]

Do we not need to do any sort of login here?

The connection is synchronous, how do we know we're actually connected?

[mtuchi]

No login there, It's a different strategy based on auth type,
So for basic-auth you will setup the connection then do connection.login(connectionOptions). And for accessToken auth, getConnection(state, connOpts) will have the connection with session type oauth

For Basic Auth - https://jsforce.github.io/document/#username-and-password-login
For OAuth - https://jsforce.github.io/document/#access-token

[josephjclark]

Approved subject to something in console.error that I just want to look into.

[josephjclark]

I don't have a quick solution to the console.error thing. For the moment I've just renamed it to console.log.