Skip to content

Conversation

@marcosholgado
Copy link
Contributor

@marcosholgado marcosholgado commented Mar 24, 2021

Task/Issue URL: https://app.asana.com/0/1125189844152671/1200105784338364
Tech Design URL: One, Two, Three
CC:

Description:
This PR identifies when a Google auth flow will require third party cookies to be enabled and do so once after the sign in flow finishes to allow users to successfully sign in.

Steps to test this PR:

Implicit flow in new tab

  1. Install the app
  2. Go to pinterest.com. You can also test in gumtree.com, yelp.com or indeed.com
  3. A logcat message should say Cookies disabled for pinterest
  4. Click in Continue with Google, a new tab should open
  5. Authenticate using your Google account
  6. Once the flow finishes the tab should be closed and you should be taken back to pinterest.
  7. In the logcat you should see Cookies enabled for pinterest and you should be signed in.
  8. Refreshing the page or navigating to a different page should disable 3p cookies again and you should see Cookies disabled for xxx in the logcat.

Implicit flow same tab on Nest

  1. Go to home.nest.com
  2. A logcat message should say Cookies disabled for pinterest
  3. Click in Sign in with Google
  4. Authenticate using your Google account
  5. Once the flow finishes you should be signed in and the logcat show Cookies enabled for home.nest.com.
  6. Nest is a special case that always requires 3p cookies enabled after sign in.
  7. Whenever you refresh the page or if you navigate back to home.nest.com you should alway see in the logcat Cookies enable for home.nest.com

Basic auth flow

  1. This flow doesn't require 3p cookies enabled.
  2. Go to etsy.com. You can try other sites like imdb.com, dropbox.com, yell.com, lego.com, etc.
  3. Sign in with your Google account.
  4. You should never see at any point the message Cookies enabled for xxxx
  5. Finish the sign in flow and you should be taken back to etsy.com successfully signed in.
  6. Make sure you never saw in the logcat that cookies were enabled.

Incomplete sign in flow

  1. Install the app
  2. Go to pinterest.com. You can also test in gumtree.com, yelp.com or indeed.com
  3. A logcat message should say Cookies disabled for pinterest
  4. Click in Continue with Google, a new tab should open
  5. Check the database, pinterest should have been added to allowed_domains
  6. Kill the tab and go back to the previous one.
  7. In the logcat you should not see Cookies enabled for pinterest.
  8. The pinterest domain should have been deleted from the allowed_domains table

Fire button

  1. Install the app
  2. Go to pinterest.com. You can also test in gumtree.com, yelp.com or indeed.com
  3. A logcat message should say Cookies disabled for pinterest
  4. Click in Continue with Google, a new tab should open
  5. Check the database, pinterest should have been added to allowed_domains
  6. Press the fire button.
  7. Once the app restarts the pinterest domain should have been deleted from allowed_domains
  8. If you navigate to pinterest.com you should not see Cookies enabled for pinterest in the logcat.
  9. The only domain we always keep in the table is home.nest.com since it always requires 3p cookies enabled.

Internal references:

Software Engineering Expectations
Technical Design Template

@marcosholgado marcosholgado changed the title Fix Google auth flow Fix Google auth flow to allow users to sign in Mar 24, 2021
@marcosholgado marcosholgado marked this pull request as ready for review March 24, 2021 15:45
Copy link
Contributor

@cmonfortep cmonfortep left a comment

Choose a reason for hiding this comment

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

GJ, all minor comments or related to naming.

I've tested all scenarios and worked as described. The only scenario I was not able to test is Implicit flow same tab on Nest because I don't have an account there.

@marcosholgado marcosholgado merged commit 2384a32 into develop Mar 26, 2021
@marcosholgado marcosholgado deleted the google_auth_flow branch March 26, 2021 13:10
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.

2 participants