finsemble-seed icon indicating copy to clipboard operation
finsemble-seed copied to clipboard

[16445] Adding oauthResponse.html back

Open sonyl-ciq opened this issue 6 years ago • 6 comments

fix: #id 16445

Kanban link

Description of change

The OAuth documentation references the oauthResponse.html file, but it doesn't exist in the seed. File does exist in the sales demo.

  • Changes
    • Duplicated finsemble-sales-demo/src/components/authentication/oauthResponse.html to finsemble-seed/src-built-in/components/authentication/oauthResponse.html

NOTE: as using this requires a great deal of application-specific logic (what oAuth endpoints, what oAuth service, etc.) so testing will be involved. This heavily depends upon the client's implementation, so a skeleton of the process is provided below.

Description of testing

  1. I followed the authentication tutorial.

    1. For the purposes of testing, I included googleapis to handle the oAuth work for the server-side. If using Google, run npm install googleapis. Version tested was ^43.0.0. This is implementation-specific and thus not included in package.json.
    2. server/server.js can be modified to accept an incoming /authenticate route for the backchannel_endpoint. As this contains secret keys, this must be server side. I have a secret gist for this with server/server.js and manifest-local.json containing this code. Again, this is implementation-specific so code is not included in the PR.
  2. Start Finsemble

    • [ ] oAuth window displays
    • [ ] Completing the auth flow displays: Screen Shot 2019-10-02 at 3 26 36 PM This displays the contents provided in oauthResponse.html
  3. x out of the window

    • [ ] Electron quits with code 0

sonyl-ciq avatar Oct 02 '19 16:10 sonyl-ciq

@NinjaManatee OK, will clean things up. It crossed my mind to do so, so thanks for the push.

I haven't tested using SalesForce, but I did use Google oAuth. I'll also test with SalesForce.

sonyl-ciq avatar Oct 03 '19 18:10 sonyl-ciq

I discussed this change request with Terry. I have concerns over us expanding the seed content even further with things for client developers to modify and then have conflicts during upgrades over.

Terry has agreed, that in this circumstance at least, it is fine that we not include this in the seed and instead focus on fixing the documentation. The documentation should reflect what this file should look like/do and not reference a file existing in the seed. Client developers should get the file contents as a starter point and add it in an area we don't modify to try and reduce any merge conflict problems down the line as we are doing more work in the seed structure.

Garbee avatar Oct 23 '19 18:10 Garbee

@Garbee That sounds reasonable to me.

NinjaManatee avatar Oct 23 '19 20:10 NinjaManatee

@NinjaManatee would it make sense for me to take a stab at writing the doc?

sonyl-ciq avatar Oct 24 '19 00:10 sonyl-ciq

@sonyl-ciq Yes, write the initial pass for the tutorial, or extension of the current tutorial. That will make Josh's life much easier as he will just need to massage existing content. Thanks!

NinjaManatee avatar Oct 24 '19 12:10 NinjaManatee

@NinjaManatee I've manually merged this into upcoming 5.0 branches. FYI, in case you'd rather just close this and not merge into 4.x branches

thorsent avatar May 25 '20 14:05 thorsent

Old

thorsent avatar Feb 07 '24 14:02 thorsent