smart-classic / smart-issues

2 stars 0 forks source link

OAuth launch flow updates #85

Open jmandel opened 11 years ago

jmandel commented 11 years ago
  1. Register callback URL in manifest ("index" for all apps, and "oauth_callback" for apps that need it)
  2. Pass 'oob' as the oauth_callback in the GET request_tokens call (and use URL registered above)
  3. Add a "frameable" boolean to app manifest
  4. In UI server, treat "frameable":true apps just as we do today but render the app's index + record_id={}&api_base={}
  5. In UI server, treat "frameable":false apps by rendering a clickable link with target=_blank
  6. Update SMART Connect API to use window.top || window.opener as the source for the jschannel
  7. Eliminate get_credentials step from SMART Connect launch flow + simplify the smart connect tokens approach!
p2 commented 11 years ago

Current status on dev_rest:

  1. This works. The question is will we only support oob or also accept callback URLs via oauth_callback param in the request? This is how it's currently implemented, the server disregards passed-in callbacks.
  2. See 1.)
  3. Given that frameable is probably the default for most apps, should we assume apps are frameable unless they have an optional flag, possibly standalone, set to TRUE?
  4. to 7. Not attacked
p2 commented 11 years ago

Current status on dev_rest:

  1. Implemented
  2. Implemented, oob is required meaning oauth_callback parameters will be ignored
  3. Added a flag standalone which is true for every app that does not run in a frame container
  4. Implemented in that standalone apps will show an overview page with a launch button as a target=blank link. Apps that define an index-URL with any other scheme than HTTP will show a warning. There is also a link to have the option to nevertheless launch the app in an iframe inside the container. Will have to be tested, I don't have any standalone apps.
  5. See 4.
  6. and 7. I am not the right person to work with SMART Connect
p2 commented 11 years ago

I've put my ae-reporting app on AppFog, it can run standalone or in the UI frame. If you just launch it you can see how the login/patient selection is handled: http://ae-reporting.aws.af.cm/

(Note the rule checking doesn't work in the online version)

jmandel commented 11 years ago

Very nice! A couple of thoughts:


Do both behaviors result from the same manifest file? Is this the intended

overall behavior?

I lost context when I had to stop and register in the middle of the 1st app

launch :-) That's probably a special case and I'm not too worried about it.

I like the table in sandbox-test.

On Wed, Feb 13, 2013 at 7:02 AM, Pascal Pfiffner notifications@github.comwrote:

I've put my ae-reporting app on AppFog, it can run standalone or in the UI frame. If you just launch it you can see how the login/patient selection is handled: http://ae-reporting.aws.af.cm/

(Note the rule checking doesn't work in the online version)

— Reply to this email directly or view it on GitHubhttps://github.com/chb/smart_project_management/issues/85#issuecomment-13497644.

p2 commented 11 years ago

Yes, it's one manifest and actually pretty simple: The index page checks if record_id is present and if it isn't, redirects to login/record selection, which returns back to the index page with record_id set. This is always set when run from the iframe. Then the OAuth dance starts (unless a token for the record/base_api combination has been stored already).

Ah, yes, if you have to register then it loses context. I had to work around this for Indivo a while back, I probably won't get to replicate this any time soon. But as you say, very special case.

If you like the records-table then good news: it has been merged into dev. :)

jmandel commented 11 years ago

Great -- and there's a separate piece in the manifest for "break out of iframe into a new window when launched from the container", which isn't set for this app?

On Wed, Feb 13, 2013 at 10:21 AM, Pascal Pfiffner notifications@github.comwrote:

Yes, it's one manifest and actually pretty simple: The index page checks if record_id is present and if it isn't, redirects to login/record selection, which returns back to the index page with record_id set. This is always set when run from the iframe. Then the OAuth dance starts (unless a token for the record/base_api combination has been stored already).

Ah, yes, if you have to register then it loses context. I had to work around this for Indivo a while back, I probably won't get to replicate this any time soon. But as you say, very special case.

If you like the records-table then good news: it has been merged into dev. :)

— Reply to this email directly or view it on GitHubhttps://github.com/chb/smart_project_management/issues/85#issuecomment-13508889.

p2 commented 11 years ago

Yes, if you set the standalone flag our container would open the app in a new tab/window. No, it's not set for this app.

nschwertner commented 11 years ago

Open issue:

  1. Update SMART Connect API to use window.top || window.opener as the source for the jschannel