Feature #6961

Simplify Tapestry/Open Humans account link flow

Added by Ward Vandewege over 4 years ago. Updated over 4 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Start date:
09/01/2015
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
2.0

Description

Wording changes

*Old wording*Explanatory text: "The link below will take you to the Open
Humans website where you will be able to create an account. Once you have
an account, you'll be returned to the PGP website, where you must approve
the export of huID to your Open Humans account."

Button: "Add authorization"

*New wording*Explanatory text: "The link below will take you to the Open
Humans website. You'll be able to create an account there. We will send
Open Humans your participant identifier (huABC123), enabling transfer of
your public data, once you finalize this data transfer on the Open Humans
website."

Button: "Share my data with Open Humans"

*New wording once the user has authorized Open Humans*The button is not
shown and the explanatory text becomes: "You've connected Open Humans to
your PGP account and we've shared your participant ID. If you would like to
remove this data and connection from Open Humans, you'll need to do so
within your account on <a href="
https://www.openhumans.org/member/me/connections/">Open Humans</a>."

Procedural changes
The "origin" parameter
A new querystring parameter, "origin", will be sent to Open Humans in the
initial request to /oauth2/authorize/. PGP will also accept the "origin"
querystring parameter on the PGP side and forward it through to Open Humans
via the request to /oauth2/authorize/. This parameter is also sent to
OAuth2 callback on the PGP side.

Immediate sending of huID
The huID will be sent immediately after the OAuth2 callback is called (
https://my.pgp-hms.org/auth/open-humans/callback).

Conditional redirect based on "origin" parameter
Depending on the value of the origin parameter the user will be redirected
either back to Open Humans (if the origin is "open-humans") or to the PGP
Open Humans participation page (if the origin is not present or is anything
but "open-humans"; we suggest using the string "external")

For example:

User starts on Open Humans
1. Open Humans sends user to
https://my.pgp-hms.org/open_humans/participate?origin=open-humans
2. User clicks button on PGP site
3. PGP sends user to https://www.openhumans.org/oauth2/authorize/?(OAuth2
parameters)&origin=open-humans (using the value of "origin" that the page
was loaded with)
4. If the user authorizes then the user is redirected back to the OAuth2
callback endpoint at
https://my.pgp-hms.org/auth/open-humans/callback?(OAuth2
parameters)&origin=open-humans
5. PGP then sends the huID to Open Humans and redirects the user back to
https://www.openhumans.org/member/me/research-data/

User starts on PGP
1. User clicks button on PGP site
2. PGP sends user to https://www.openhumans.org/oauth2/authorize/?(OAuth2
parameters)&origin=external (using "external" because the page was not
loaded with an origin specified)
3. If the user authorizes then the user is redirected back to the OAuth2
callback endpoint at
https://my.pgp-hms.org/auth/open-humans/callback?(OAuth2
parameters)&origin=external
4. PGP then sends the huID to Open Humans and sends the user to
https://my.pgp-hms.org/open_humans/participate where they see the "You've
connected Open Humans to your PGP account" text

Let me know if any of this needs clarification. :)

And here's the flowchart/mockup URL again if that's useful:
https://personalgenomes.mybalsamiq.com/projects/update-pgp/prototype/Updating%20PGP%20Harvard%20connection?key=30090b9af
29d405942c7403dec78fb37c4ff5b62

Thanks,

Beau


Subtasks

Task #7140: review 6961-simplify-open-humans-account-link-flowResolvedWard Vandewege


Related issues

Related to Tapestry - Feature #7462: Always redirect Open Humans user back to OHResolved10/06/2015

Related to Tapestry - Bug #7463: Origin parameter not included for Open HumansResolved10/06/2015

Related to Tapestry - Bug #7465: canceling link on OH integration redirects to https://my-dev.pgp-hms.org/auth/open-humans/callback?error=access_denied which throws a 500Resolved

Associated revisions

Revision 8143e1a7 (diff)
Added by Ward Vandewege over 4 years ago

Simplify Tapestry/Open Humans account link flow.

refs #6961

Revision 3fba0ae9 (diff)
Added by Ward Vandewege over 4 years ago

Simplify Tapestry/Open Humans account link flow.

refs #6961

Revision 13add59e (diff)
Added by Ward Vandewege over 4 years ago

Simplify Tapestry/Open Humans account link flow.

refs #6961

Revision 06a5f889 (diff)
Added by Ward Vandewege over 4 years ago

Fix typo.

refs #6961

Revision cdabc774 (diff)
Added by Ward Vandewege over 4 years ago

Finalize the simplified Open Humans integration flow.

Improvements:

  • add call to revoke access at OH when the user disconnects in Tapestry
  • add logging of all activity in the user log
  • general cleanups

refs #6961

Revision 917f6b5e (diff)
Added by Ward Vandewege over 4 years ago

Implement some review feedback.

refs #6961

Revision 53d864b0 (diff)
Added by Ward Vandewege over 4 years ago

Small fixes:

  • Update url for longupload submodule
  • Add system_timer to Gemfile (only needed on ruby 1.8, but we're still on that ancient version).

refs #6961

Revision 77ca2ea5 (diff)
Added by Ward Vandewege over 4 years ago

Simplify Tapestry/Open Humans account link flow.

refs #6961

Revision 460d7259 (diff)
Added by Ward Vandewege over 4 years ago

Fix typo.

refs #6961

Revision 99966771 (diff)
Added by Ward Vandewege over 4 years ago

Finalize the simplified Open Humans integration flow.

Improvements:

  • add call to revoke access at OH when the user disconnects in Tapestry
  • add logging of all activity in the user log
  • general cleanups

refs #6961

Revision caa77f0a (diff)
Added by Ward Vandewege over 4 years ago

Implement some review feedback.

refs #6961

Revision 5774fa52 (diff)
Added by Ward Vandewege over 4 years ago

Small fixes:

  • Update url for longupload submodule
  • Add system_timer to Gemfile (only needed on ruby 1.8, but we're still on that ancient version).

refs #6961

Revision c1d38295
Added by Ward Vandewege over 4 years ago

Merge branch 'master' into 6961-simplify-open-humans-account-link-flow

refs #6961

Revision 35b0c4ec
Added by Ward Vandewege over 4 years ago

Merge branch 'master' into 6961-simplify-open-humans-account-link-flow

refs #6961

Revision ce12e9b5 (diff)
Added by Ward Vandewege over 4 years ago

Handle expired tokens better.

refs #6961

Revision 931256c2
Added by Ward Vandewege over 4 years ago

Merge branch '6961-simplify-open-humans-account-link-flow'

closes #6961

History

#1 Updated by Ward Vandewege over 4 years ago

  • Description updated (diff)

#2 Updated by Ward Vandewege over 4 years ago

  • Subject changed from Simplify Tapestry/Open Humans link flow to Simplify Tapestry/Open Humans account link flow

#3 Updated by Ward Vandewege over 4 years ago

  • Status changed from New to In Progress

#4 Updated by Ward Vandewege over 4 years ago

  • Assigned To set to Ward Vandewege

#5 Updated by Tom Clegg over 4 years ago

In OpenHumansController#disconnect_worker, this comment sounds like a good idea but it doesn't seem to be implemented this way:

    # We deliberately return success based on the outcome of the local token revocation, only.
    # That's the only thing we can control anyway, and also what drives all logic on our end.

It looks like the only case where success ends up false is the one where both the OpenHumans remote API call and the local revocation fail.

I suppose the trouble is that [a] if the "dissociate" API calls fail, and we revoke our token, we have no way to retry the "dissociate" API; but [b] if "dissociate" fails, and we don't revoke our token, then it's impossible for a user to revoke permission as long as the "dissociate" API fails.

The token.revoke! call currently looks like it deletes the token if the token is authorized, but never calls the remote revocation URL for OAuth2 tokens. Since we more or less rely on this being the behavior, I suggest changing token.revoke! to token.destroy here.

I'm a bit confused by the testing situation: It looks like there are no test updates in this branch (I'm at cdabc77). The existing "delete" test passes, even though the corresponding route and action method have disappeared. What am I missing?

#6 Updated by Ward Vandewege over 4 years ago

Tom Clegg wrote:

In OpenHumansController#disconnect_worker, this comment sounds like a good idea but it doesn't seem to be implemented this way:

[...]

It looks like the only case where success ends up false is the one where both the OpenHumans remote API call and the local revocation fail.

I suppose the trouble is that [a] if the "dissociate" API calls fail, and we revoke our token, we have no way to retry the "dissociate" API; but [b] if "dissociate" fails, and we don't revoke our token, then it's impossible for a user to revoke permission as long as the "dissociate" API fails.

The token.revoke! call currently looks like it deletes the token if the token is authorized, but never calls the remote revocation URL for OAuth2 tokens. Since we more or less rely on this being the behavior, I suggest changing token.revoke! to token.destroy here.

I've covered all this in caa77f0a

#7 Updated by Ward Vandewege over 4 years ago

  • Status changed from In Progress to Resolved
  • % Done changed from 0 to 100

Also available in: Atom PDF