allow authentication via access token in addition to username/password#16
allow authentication via access token in addition to username/password#16wgorman wants to merge 1 commit into
Conversation
ghost
left a comment
ghost
left a comment
There was a problem hiding this comment.
Thanks for your pull request. Could you please add few unit tests for your change?
Please make sure the existing tests pass with your changes as well.
Please create PR for https://github.com/springml/salesforce-wave-api with your changes
| def writeMetadata(metaDataJson: String, | ||
| mode: SaveMode, | ||
| upsert: Boolean): Option[String] = { | ||
| val partnerConnection = createConnection(userName, password, login, version) |
There was a problem hiding this comment.
We need to support existing login as well. i.e with username and password
| if (username.isDefined) { | ||
| waveAPI = APIFactory.getInstance.waveAPI(username.get, password, serverUrl, version) | ||
| } else { | ||
| waveAPI = APIFactory.getInstance.waveAPIwAuthToken(authToken.get, serverUrl, version) |
There was a problem hiding this comment.
We need PR for salesforce-wave-api as well
| password = param(parameters, "SF_PASSWORD", "password") | ||
| serverUrl = parameters.getOrElse("login", "https://login.salesforce.com") | ||
| } else { | ||
| serverUrl = parameters.getOrElse("instanceUrl", "https://login.salesforce.com") |
There was a problem hiding this comment.
Why don't we use the existing property for instanceUrl?
| if (monitorJob) { | ||
| logger.info("Monitoring Job status in Salesforce wave") | ||
| if (Utils.monitorJob(writtenId.get, username, password, login, version)) { | ||
| if (Utils.monitorJob(writtenId.get, username, password, authToken, serverUrl, version)) { |
There was a problem hiding this comment.
How is the authToken expiry should be handled?
There was a problem hiding this comment.
Great question, would you like automatic logic built into the spark layer or would you expect that to take place outside of spark?
There was a problem hiding this comment.
@wgorman I would like to handle in spark layer. There might be a chance that the access token got expired while fetching millions of rows
|
Is there any hope of this PR getting merged? If you have MFA enabled for API access to salesforce, you can't get a security key. This means that the only way to access Salesforce APIs for accounts with MFA is via OAuth. Submitting a username, password, and security token simply isn't an option in that case :( Salesforce has said that they are not responsible for security breaches for any customers that don't have MFA enabled on their accounts. (makes sense). This means that more and more SF accounts are enabling MFA. I think that means that this library is useless for those accounts in it's current state. (unless I'm wrong, which I'd love to be) References: https://help.salesforce.com/s/articleView?id=sf.security_require_2fa_api.htm&type=5 https://help.salesforce.com/s/articleView?id=sf.user_security_token.htm&type=5 |
2 participants
Note, this change requires the upstream change egen/salesforce-wave-api#3