MomenSherif / react-oauth

Google OAuth2 using the new Google Identity Services SDK for React 🚀
https://www.npmjs.com/package/@react-oauth/google
MIT License
1.13k stars 141 forks source link

The `width` property no longer work with strings #277

Open bilby91 opened 1 year ago

bilby91 commented 1 year ago

Hello,

Today, our BI tool started crashing (https://github.com/metabase/metabase) with the Google Authentication. After doing some troubleshooting we realized that the width property no longer works with a string, it expects a number.

Our guess is that the gsi/client (https://accounts.google.com/gsi/client) script changed. Still, documentation says that it should be a string => https://developers.google.com/identity/gsi/web/reference/js-reference#width

I was able to reproduce the problem locally with another project that uses this library.

@MomenSherif have you stumble upon this issue ?

darrillaga commented 1 year ago

Hi, same issue here, I've been digging in the minified JS little bit and I think I've found a difference.

The following (the code causing us the issue) is a response from https://accounts.google.com/gsi/client at 07/24/2023 20:46 UTC gsi_client.js.zip

        var mp = { left: 1, center: 2 },
            np = { rectangular: 1, square: 3, pill: 2, circle: 4 },
            op = { large: 1, medium: 2, small: 3 },
            pp = { signin: 1, signin_with: 2, signup_with: 3, continue_with: 4 },
            qp = { outline: 1, filled_blue: 2, filled_black: 3 },
            rp = { standard: 1, icon: 2 },
            sp = function (a, b, c) {
                this.s = a;
                this.h = c;
                this.g = !1;
                a = new _.Hi();
                b &&
                    (b.logo_alignment && _.M(a, 6, mp[b.logo_alignment]),
                    b.shape && _.M(a, 3, np[b.shape]),
                    b.size && _.M(a, 1, op[b.size]),
                    b.text && _.M(a, 5, pp[b.text]),
                    b.theme && _.M(a, 2, qp[b.theme]),
                    b.type && _.M(a, 7, rp[b.type]),
                    b.width && !isNaN(b.width) && _.$e(a, 4, b.width));
                this.Ha = a;
                this.startTime = performance.now();
            },

The line in question is:

b.width && !isNaN(b.width) && _.$e(a, 4, b.width));

        _.$e = function (a, b, c) {
            if (null != c && "number" !== typeof c) throw Error();
            return _.M(a, b, c);
        };

Wayback Machine (https://web.archive.org/web/20230705000416/https://accounts.google.com/gsi/client)

        var mp = { left: 1, center: 2 },
                np = { rectangular: 1, square: 3, pill: 2, circle: 4 },
                op = { large: 1, medium: 2, small: 3 },
                pp = { signin: 1, signin_with: 2, signup_with: 3, continue_with: 4 },
                qp = { outline: 1, filled_blue: 2, filled_black: 3 },
                rp = { standard: 1, icon: 2 },
                sp = function (a, b, c) {
                    this.s = a;
                    this.h = c;
                    this.g = !1;
                    a = new _.Oi();
                    b &&
                        (b.logo_alignment && _.L(a, 6, mp[b.logo_alignment]),
                        b.shape && _.L(a, 3, np[b.shape]),
                        b.size && _.L(a, 1, op[b.size]),
                        b.text && _.L(a, 5, pp[b.text]),
                        b.theme && _.L(a, 2, qp[b.theme]),
                        b.type && _.L(a, 7, rp[b.type]),
                        b.width && !isNaN(b.width) && _.L(a, 4, b.width));
                    this.Ga = a;
                    this.startTime = performance.now();
                },

Similar code here:

b.width && !isNaN(b.width) && _.L(a, 4, b.width));

But part of the validation had changed:

        _.L = function (a, b, c, d) {
                var e = a.v,
                    f = (0, _.I)(e);
                _.xd(f);
                _.Bd(e, f, b, c, d);
                return a;
            };
MomenSherif commented 1 year ago

Hello,

I tried it locally on this repo playground & it was working fine

  width="700"

I think yea as @darrillaga mentioned, the issue happens randomly based on Geolocation script changes for some reason they did the same thing before with clientId in Brazil

   credentialResponse?.clientId ?? credentialResponse?.client_id;

on way to fix it you can serve the right JS gsi code on your own until google fixes it

Screenshot 2023-07-24 at 11 59 52 PM
facostaembrace commented 1 year ago

Just add the 'px' as part of that string (that worked for my project)

width="700px"
MomenSherif commented 1 year ago

@bilby91 Try this version 0.11.1 that allowed width to be a number width={300} should work for Brazil and other regions

brandonparis commented 1 year ago

Just add the 'px' as part of that string (that worked for my project)

width="700px"

This worked for us, and TS complains about an int value because it's still defined as a string

BatuhanTopcu commented 1 year ago

If I give width prop it breaks my app entirely. If I remove the width prop it works as expected but then UI integrity is broken. In my demo the client thanks for that my app broken entirely. :)

szamanr commented 1 year ago

thank god i found this issue, we were using the GSI script directly and spent so much time trying to figure out why it suddenly started crashing...

it's ridiculous that google can get away with changing their script overnight like that. is there any channel where we can complain?

GastonRosso commented 1 year ago

Same issue here, thanks I've found this... unbelievable that the script can change like that