Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade Google Auth #26

Closed

Conversation

dextermb
Copy link

Resolves #25

Upgrades from using gapi.auth2 to google.accounts.id (https://developers.google.com/identity/gsi/web).

Screenshot 2022-07-22 at 16 33 00

This change, however, is breaking as the outputs from the events are different.

  • auth-success now returns an object with user properties:
    • name
    • email
    • picture
  • auth-failure now returns a basic string
  • init-error is no longer used

@dextermb
Copy link
Author

Once people are happy with the implementation then I will look at updating the documentation

Copy link
Member

@antony antony left a comment

Choose a reason for hiding this comment

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

Looks good, a few improvements for readability and a question

package.json Show resolved Hide resolved
src/lib/google-auth/GoogleAuth.svelte Outdated Show resolved Hide resolved
src/lib/google-auth/GoogleAuth.svelte Outdated Show resolved Hide resolved
@antony
Copy link
Member

antony commented Jul 23, 2022

Note that we don't validate the jwt signature, so it might be better to get rid of the new dependency and just bade64 decode the unencrypted part of the jwt

https://stackoverflow.com/questions/38552003/how-to-decode-jwt-token-in-javascript-without-using-a-library

@dextermb
Copy link
Author

Note that we don't validate the jwt signature, so it might be better to get rid of the new dependency and just bade64 decode the unencrypted part of the jwt

https://stackoverflow.com/questions/38552003/how-to-decode-jwt-token-in-javascript-without-using-a-library

Certainly if you're happy with having the suggested browser function in the codebase. I'll update this

@dextermb
Copy link
Author

Certainly if you're happy with having the suggested browser function in the codebase. I'll update this

Removed jwt-decode dependency and added the answer-marked implementation from stackoverflow ✅

@dextermb dextermb requested a review from antony July 25, 2022 09:23
@antony
Copy link
Member

antony commented Jul 27, 2022

lets pnpm link this to the app to try it first before we merge and release.

@dextermb
Copy link
Author

dextermb commented Aug 1, 2022

lets pnpm link this to the app to try it first before we merge and release.

👋🏻 Hey, just getting back to this today. I've done a decent amount of testing and reading through our internal use-cases.

@antony with minor changes to our login screen I've got this working as we would expect. I've exposed the whole token through the auth-success callback.

It's worth noting the GSI does not like running on non-https domains, other than localhost, so this will throw some complications for us and potentially other development workflows.

@dextermb
Copy link
Author

I've also noticed that button is now being respected on our login/sign up screens- e.g. not width: 100% by default which is correct. Potentially something else has changed in our platform but to match the same styles as previously seen I updated the styles:

  :global(.social-auth-button button) {
    width: 100%;
    font-family: 'Cabin', sans-serif;
    border-radius: 5px;
  }

But I don't think this is anything to do with this pull request as looking at the code I have not changed anything relating to styles.

@TheOnlyWayUp
Copy link

Thanks for the PR! I installed from this commit and importing failed, but copying over the GoogleAuth.svelte file and using it like any other component seems to work

:)

@antony
Copy link
Member

antony commented Oct 17, 2022

lets also consider this #14 when we merge

@dextermb dextermb closed this Oct 4, 2023
@dextermb dextermb deleted the feature/upgrade-google-auth branch October 4, 2023 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gapi.auth2.init deprecated
3 participants