Page MenuHomePhabricator

Adjust Google Auth Adapter to use Open ID Connect
AbandonedPublic

Authored by dragoon20 on Wed, Jan 23, 9:11 AM.

Details

Reviewers
epriestley
Group Reviewers
Blessed Reviewers
Summary

Google planned to shutdown Google+ API because of
security vulnerabilities.

https://developers.google.com/+/api-shutdown

Test Plan
  • Locally, try to setup google authentication account and try login.

Diff Detail

Repository
rPHU libphutil
Branch
adjust_google_sign_in
Lint
Lint OK
Unit
Unit Tests OK
Build Status
Buildable 21639
Build 29505: arc lint + arc unit

Event Timeline

dragoon20 created this revision.Wed, Jan 23, 9:11 AM
dragoon20 requested review of this revision.Wed, Jan 23, 9:11 AM
dragoon20 updated this revision to Diff 47783.Wed, Jan 23, 9:14 AM

Update some uncommitted code

epriestley requested changes to this revision.Wed, Jan 23, 11:29 AM

We'll ship a patch for this in the next release (see T13227) but this patch doesn't look quite right.

This is helpful as a starting point, but there are some related Auth changes I want to make at the same time (e.g., T5591) anyway.

src/auth/PhutilGoogleAuthAdapter.php
67

This looks wrong ("famiily" is misspelled).

152–177

This looks wrong, since the message is adjusted but the parameters are not, so the %s conversions won't line up.

This revision now requires changes to proceed.Wed, Jan 23, 11:29 AM
dragoon20 updated this revision to Diff 47786.Wed, Jan 23, 2:09 PM

Fix typo and wrong parameter for error message

Harbormaster completed remote builds in B21639: Diff 47786.

Hi @epriestley , thanks for looking at this patch, please help elaborate if there is something wrong with this patch.

Currently I only replace the use of access token to id token which is working.

As for T5591, maybe I can help you in another patch also, I am thinking that would need to add parameter on the login screen which resides in rP.

dragoon20 abandoned this revision.Fri, Jan 25, 3:04 AM

@epriestley thanks for the other diff, it worked, closing this diff