DoSomethingArchive / LetsDoThis-iOS

:iphone: iOS source code for DoSomething: Take Action on the News
http://app.dosomething.org
MIT License
2 stars 0 forks source link

Release 1.2.0 #1049

Closed aaronschachter closed 8 years ago

aaronschachter commented 8 years ago

Opening a pull request for our latest release branch to troubleshoot Magic Link issues:

cc @angaither @DFurnes

angaither commented 8 years ago

@aaronschachter 😧 how can I test this?

aaronschachter commented 8 years ago

@angaither To see it in action on iOS, you can test the 1.2.0 release sent via email Crashlytics last night (release notes contain instructions on how to install). I've added dummy magic_link_copy values to the first two campaigns in Actions > Animals (Bumble Bands & Dog Days of Winter)

I haven't tested in Paw but in theory this should be replicable by authenticating to Northstar on production, requesting a magic link from the auth/phoenix endpoint, and then opening that link with a ?redirect=node/[id] appended: https://github.com/DoSomething/LetsDoThis-iOS/pull/1048/commits/0d35f788f42e70ac0b3863bd62ac85d0a97f48f4

angaither commented 8 years ago

@aaronschachter yaa yaaa, seeing the same thing as you. 😧

DFurnes commented 8 years ago

Tapping on a magic link a 2nd time within the iOS app will result in a red error message

Magic links use mostly the same logic as Drupal's built-in password reset links, and so they're invalidated after a single use. We could think about lifting that restriction, but I'd be a little wary of touching too much there if we opened ourselves up to brute force attacks.

Happy to talk over possible ideas/solutions.

angaither commented 8 years ago

I think the issue is maybe with some redirect in prod?

sheyd commented 8 years ago

@angaither Redirect is going to country-code specific. Using @aaronschachter sample link above:

https://www.dosomething.org/user/magic/3245402/1471915770/[removed]?redirect=node/63

get redirected to

https://www.dosomething.org/us/user/magic/3245402/1471915770/%5Bremoved%5D

IIRC, the re-direct logic is within Phoenix? Do magic links with country code still work as valid?

DFurnes commented 8 years ago

Gahhhh. How do we avoid this issue with normal password reset links?

DFurnes commented 8 years ago

Oh, and answering my own question – we do not. It looks like we send people directly to the prefixed /us/user/reset/ URL from password reset emails. Is that a potential solution here?

Longer-term, would it make sense to make the global redirect rules a bit less aggressive (and not redirect to a prefixed URL for pages that don't have translations)?

angaither commented 8 years ago

@DFurnes hrm yar I tested this via paw by adding a /us/ to the front of the magic link and it worked. boo.

  1. should we default send the magic link with /us/?
  2. opt anything with /user/magic out of the global redirect code
  3. try to append the appropriate country code (based on the UID requested) to the magic link
DFurnes commented 8 years ago

I feel like 1 is the quickest fix (seeing as it seems like that's what we've done on the nearly-identical password reset flow). But either 2 or just fixing a potential order-of-operations issue with expiry/redirect seems like a better long term decision?