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

Add authorization via Session ID #29

Merged
merged 9 commits into from
Mar 14, 2018
Merged

Conversation

mmrobins
Copy link
Collaborator

https://developer.salesforce.com/docs/atlas.en-us.api_rest.meta/api_rest/quickstart_oauth.htm
Session ID Authorization - You can use a session ID instead of an OAuth
2.0 access token if you aren’t handling someone else’s password

TODO

This works for my use case of session id auth, but still working on cleaning
all this up. Thought I'd put in a Work In Progress PR though to get any early
feedback.

Without tests for the oauth case I'm not sure if I broke that in some way. I'm
playing with figuring out a good way to test this by setting up an app oauth on
one of our test salesforce instances, and then actually writing tests with a
mocked out wrapper around the httpoison calls.

bulk api should be able to reuse the session id auth mechanism (especially
since I cargo culted most of the soap request from there), just needs a way to
set a different key for the auth header. Also, tests would be nice before I
change anyway, but we'll see.

https://developer.salesforce.com/docs/atlas.en-us.api_rest.meta/api_rest/quickstart_oauth.htm
Session ID Authorization - You can use a session ID instead of an OAuth
2.0 access token if you aren’t handling someone else’s password
]

"https://login.salesforce.com/services/Soap/u/#{starting_struct.api_version}"
|> HTTPoison.post!(envelope, headers)
Copy link
Owner

Choose a reason for hiding this comment

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

@mmrobins it looks like we have different semantics for login between the 2 implementations of the Forcex.Auth behaviour. This one will raise if it encounters a problem whereas the other will return an error tuple (I think. I might have to dig in further). Regardless, it would likely be beneficial to have login and login!. It doesn't need to be part of this PR though.

Copy link
Owner

Choose a reason for hiding this comment

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

I dug in. They're all ! functions underneath, despite outward appear. Present-Jeff is shaming Past-Jeff.

You're good to go with this PR.

server_url = extract_from_parameters(login_parameters, :serverUrl)
session_id = extract_from_parameters(login_parameters, :sessionId)
host = server_url |> URI.parse() |> Map.get(:host)
endpoint = "https://#{host}/"
Copy link
Owner

Choose a reason for hiding this comment

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

Do we ever have to worry about other ports? What do the SFDC sandbox server urls look like?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure, but based on the format of the SOAP response I would expect if there were ports they would just come back in that extracted serverUrl tag and just work. I've got tests in the new commits I'm about to push, so if anyone hits any issue with sandbox urls (something I'll probably look into shortly instead of my 30 day trial I signed up for) it should be easy to debug and fix.

Step toward making these interactions mocked and testable
Wanted at least 1 test where auth header was already set for an example
Since the stubbed out Api module does all the parsing of the JSON that's
returned the test doesn't exercise a ton, other than a url is called and
kind of documents the format of the returned data.
So I can run `mix test.watch ---stale` and tests run automatically when
I save files in my editor.
@mmrobins mmrobins changed the title WIP: Add authorization via Session ID Add authorization via Session ID Mar 13, 2018
@mmrobins
Copy link
Collaborator Author

Feeling better about these changes now that I've got some automated tests and manually tested both auth methods with a real salesforce org (signed up for a free trial for now to create the oauth stuff), and am using the library in my company's internal app on a WIP branch. Happy for any feedback on whatever before I merge though (ping @athal7)

@jeffweiss jeffweiss merged commit a857395 into jeffweiss:master Mar 14, 2018
@mmrobins mmrobins deleted the auth branch March 14, 2018 15:53
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.

3 participants