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

Discord auth invalid state #218

Open
2 tasks done
flav-code opened this issue Aug 5, 2023 · 31 comments
Open
2 tasks done

Discord auth invalid state #218

flav-code opened this issue Aug 5, 2023 · 31 comments
Labels
bug Something isn't working

Comments

@flav-code
Copy link

flav-code commented Aug 5, 2023

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the issue has not already been raised

Issue

hello !
As of today, my login system with discord no longer works, I don't think I've touched anything.

image

on my calback url
image
image

the cookie doesnt exist :/
image

@mcollina
Copy link
Member

mcollina commented Aug 5, 2023

If you didn't touch anything and now it doesn't work anymore it might be a Discord issue?

Let us know if you find any clue on why that cookie is not transmitted any longer.

@flav-code
Copy link
Author

flav-code commented Aug 5, 2023

I think, the cookie is not set
image
image

but discord does return code and state

and where should this cookie go ?
image

@flav-code
Copy link
Author

this is where it breaks down since the state is stored in a cookie
https://github.com/fastify/fastify-oauth2/releases/tag/v7.2.0

@Uzlopak
Copy link
Contributor

Uzlopak commented Aug 5, 2023

I dont know man... generateStateFunction kind of indicates that a function is returned but you expect it to be a string?

You pass request to generateStateFunction?

@flav-code
Copy link
Author

flav-code commented Aug 5, 2023

my code is at the top of the conversation

my problem is with the way the state is stored now, on my side the cookie is not set so the state is invalid.

@mcollina
Copy link
Member

mcollina commented Aug 6, 2023

Given that you have not touched anything, how did stop working? Did anything else change?

@flav-code
Copy link
Author

just this package version

@mcollina
Copy link
Member

mcollina commented Aug 6, 2023

just this package version

This essentially contradicts your original post. You actually changed something. It's hard to help if the repoorts are not clear.

What version were you using before?

@flav-code
Copy link
Author

as there was ^ in front of the package version, well that's where it took the version above and that's why npm i had to update it

@LiezarZ
Copy link

LiezarZ commented Aug 6, 2023

hi @mcollina,

reading the whole thread as third party, as I have problems with fastify-oauth2 as well.
Trying to make it works for google I am getting the same message as @flav-code "invalid state".

You write that his post contradicts with his original post which is not correct.
He wrotes that he has not changed anything in his implementation. The only thing that has changed is the package version.
He also describes that it is no longer working since 7.2.

I think there is something not working correctly since 7.2.
#216 gave first indications that there seems to be something wrong in checkStateFunction .

As far as I can see is that "invalid state" occurs when "getAccessTokenFromAuthorizationCodeFlow" is called.
The request object is passed in this method. In "getAccessTokenFromAuthorizationCodeFlowCallbacked" its assigned as follows:

const code = request.query.code

Maybe the state does need to come from the cookie aswell know?
e.g. google seems to set it there -> request.cookies['g_state'];
But request.query.code is empty and therefore the state invalid.

It would be great if someone with a deeper knowledge of this project and code could have a deeper look.
Hope my message could give some more information

Your help is really appreciate.

@flav-code
Copy link
Author

thank you for understanding what i said. and glad to hear i'm not the only one.

@Uzlopak
Copy link
Contributor

Uzlopak commented Aug 6, 2023

where google sets it in his cookies is unimportant. When you get a redirect to your callback site, the code has to be in the query. Thats why it takes it from the request.query.code. Also oauth2-redirect-state is our cookie.

If you can create me a repository,which i can clone and test, and provide me a link where it shows how to get the creds for discord, I could debug it.

@Uzlopak Uzlopak changed the title Discord auth inalid state Discord auth invalid state Aug 6, 2023
@mcollina
Copy link
Member

mcollina commented Aug 6, 2023

Just to to recap what the bug report is.

Your Discord integration used to work as expected in v7.1.1. Then you updated to v7.2.0 and it stopped working.

Can you include some code that shows how you are integrating this library?

The way the bug report was originally written seemed that Discord changed their APIs somehow and the integration stopped working. Please read carefully this article from StackOverflow: https://stackoverflow.com/help/minimal-reproducible-example.

@flav-code
Copy link
Author

flav-code commented Aug 6, 2023

i didn't update it's npm that did it because the version in my package was ^7.1.1 but as there was ^ npm took the liberty of installing 7.2.0 when using npm i

Code where I configure how to use the fastify-oauth2 package

const discordConfig = oauthPlugin.DISCORD_CONFIGURATION;

fastify.register(oauthPlugin, {
    name: "Discord",
    credentials: {
        client: {
            id: config.auth.discord.id,
            secret: config.auth.discord.secret,
        },
        auth: discordConfig,
    },
    startRedirectPath: "/discord/login",
    callbackUri: `${config.api.baseUrl}/v1/auth/callback/discord`,
    scope: config.auth.discord.scopes, // guilds identify
    prompt: config.auth.discord.prompt, // consent
});

@Uzlopak
Copy link
Contributor

Uzlopak commented Aug 6, 2023

Your code seems correct.

I will add the "bug" label to this issue.

Anytime you create a mvce and provide a link for setting it up, you can ping me up and I will look into it.

@Uzlopak Uzlopak added the bug Something isn't working label Aug 6, 2023
@carere
Copy link

carere commented Aug 15, 2023

@mcollina @Uzlopak What the temporary solution for this ? I think that going back to 7.1 is not an option since there is some CSRF security issues.

@Uzlopak
Copy link
Contributor

Uzlopak commented Aug 15, 2023

There is no temporary solution for this, because nobody is willing to provide a mvce (repository to clone) with information how to set up an discord oauth login. If people report screenshots and code snippets, than I wont sit at my PC and type it from the image, nor will I puzzle with the code snippet till I get a reproducable example.

I am very well aware of oauth2 and openid protocol, and I am developing myself an openid provider. But I dont use this plugin. So dont expect anything, from my side.

@mcollina
Copy link
Member

I'm confident in the current implementation, so I would need some help from you to have an easy-to-reproduce way for this (including the client side code).

Given the CSRF issue and this bug, I guess your implementation was relying on the fact that state was badly implemented in this library (leading to the security issue in question).

@LiezarZ
Copy link

LiezarZ commented Aug 16, 2023

Although I'm not the thread creator, since I've noticed similar issues with Google, I could build a simple example with login page and matching route and make it available at glitch.com for remixing or upload the repository here. Just let me know if this is suitable and if so, which variant is preferred.

@Uzlopak
Copy link
Contributor

Uzlopak commented Aug 16, 2023

I just need something to debug easily. So repo here would be nice.

@LiezarZ
Copy link

LiezarZ commented Aug 16, 2023

So I have set up a simple repo on github now:
fastify-oauth2-debug

Its a one pager with a google login button. I also set up a .env file.
The result will be printed below. So when successful the token should be shown, otherwise "Error: Invalid State".

Maybe you need to change urls in src/pages/index.hbs and server.js. But I am not sure if this works, because the project url and callback uri are also set in the google developer console itself.
Also glitch comes with node 16.x, very old I now and soon no security fixes anymore, so just keep in mind.
Maybe update in package file.

For completeness here is the original on glitch:

Please let me know in case of problems or if you need anything further.

@mcollina
Copy link
Member

@marco-ippolito PTAL

@marco-ippolito
Copy link
Contributor

marco-ippolito commented Aug 17, 2023

from what I can see from your snippet:

/*     
Google OAuth2 Callback  
*/
    fastify.route({
      method: ["POST"],
      url: "/login/google/callback",
      logLevel: "warn",
      handler: async (request, reply) => {
        let paramsGoogle = {
          result: null,
        }; // gets passed to template
        console.log("code", request.query); // EMPTY
        try {
          const { token } =
            await fastify.googleOAuth2.getAccessTokenFromAuthorizationCodeFlow(
              request
            );
          paramsGoogle.result = token.access_token; // gets printed on front page

          return reply.status(200).view("/src/pages/index.hbs", paramsGoogle);
        } catch (error) {
          paramsGoogle.result = error; // gets printed on front page
          return reply.status(500).view("/src/pages/index.hbs", paramsGoogle);
        }
      },
    });
  });

the request at /login/google/callback does not contain query params, it's empty, that's why it goes into invalid state.
The state is never set because you put startRedirectPath as "/login/google", so the application expects that your authorization flow starts from that path.
This /login/google url generates the callback url with proper query params that you need to pass to google oauth in order to verify it later.
In your application the callbackUrl is hardcoded data-login_uri="http://localhost:64952/login/google/callback".
You should retrieve the callbackUrl from generateAuthorizationUri if not using startRedirectPath.
I think this is not a bug but misconfiguration.

@LiezarZ
Copy link

LiezarZ commented Aug 17, 2023

hi @marco-ippolito,

thanks for your feedback!
The importance of startRedirectPath was not clear to me.
I would have set it to / where the login button is, but fastify complains about duplicate routes then.
Deleting / was no option, because the template index.hbs is set there.

I reworked my code and did the following:

  • decommented startRedirectPath
  • using generateAuthorizationUri instead of getAccessTokenFromAuthorizationCodeFlow in my callback route /login/google/callback
  • parsing the url

Now that I am able to see the state in state.query.state I can check further how to handle the login in my app.

But to be honest, reading the project readme, examples and additional how-to resources on the web for this project, I thought it would be easier. Perhaps the documentation just needs to be expanded and made a bit more precise.
@flav-code @carere As you have similar problems, what is your opinion to that? Is your code working aswell, if you make the same changes like I did?

  .register(oauthPlugin, {
    name: "googleOAuth2",
    scope: ["profile", "email"],
    credentials: {
      client: {
        id: process.env.GOOGLE_OAUTH_CLIENT_ID,
        secret: process.env.GOOGLE_OAUTH_CLIENT_SECRET,
      },
      auth: oauthPlugin.GOOGLE_CONFIGURATION, // plugin-provided configurations for Google OAuth
    },

//    startRedirectPath: "/login/google", // This is automatically registered as a GET route in your app
    callbackUri:
      `https://${process.env.PROJECT_DOMAIN}.glitch.me` +
      "/login/google/callback", // relative path that serves HTML or an absolute URL
    callbackUriParams: {
      // custom query param that will be passed to callbackUri
      access_type: "offline", // will tell Google to send a refreshToken too
    },
  })
  .after(() => {
/*
Standard Site
*/
   fastify.route({
      method: ["GET", "HEAD"],
      url: "/",
      logLevel: "warn",
      handler: async (request, reply) => {
        let params; // gets passed to template
        return reply.
        view("/src/pages/index.hbs", params);
      },
    });
/*     
Google OAuth2 Callback  
*/  
  fastify.route({
    method: ["POST"],
    url: "/login/google/callback",
    logLevel: "warn",
    handler: async (request, reply) => {
      let paramsGoogle = {result: null,};  // gets passed to template
      let url = require('url'); 
      
      try {
        
        const authorizationEndpoint = await fastify.googleOAuth2.generateAuthorizationUri(request, reply); 
        let state = url.parse(authorizationEndpoint, true);
      
        paramsGoogle.result = state.query.state; // gets printed on front page

        return reply
          .status(200)
          .view("/src/pages/index.hbs", paramsGoogle);
        
      } catch (error) {
        paramsGoogle.result = error;  // gets printed on front page
        return reply
          .status(500)
          .view("/src/pages/index.hbs", paramsGoogle);
      }
    },
  });
});

@carere
Copy link

carere commented Aug 17, 2023

@LiezarZ I don't have the "state" problem anymore, don't know why, it just work. When I don't know why, I usually say that it's the cache 😅

@marco-ippolito
Copy link
Contributor

@LiezarZ I'm not sure that's the right approach.
I'd say the general auth flow is:

  1. generate authorizationUri with generateAuthorizationUri or from the startRedirectPath endpoint (this step adds state query param and cookies that will be check in the callback uri to ensure no csrf happened).
  2. redirect on the authorizationUri (authorization page, google login etc), when login is successful it will redirect to callbackUri with the state query param set in step 1.
  3. land on callbackUri and check the state and cookies are present.

@LiezarZ
Copy link

LiezarZ commented Aug 17, 2023

Thanks @marco-ippolito but I unfortunately does not understand that.
Is there anywhere an example for your described procedure I can have a look at?

@carere
Copy link

carere commented Aug 17, 2023

@LiezarZ For info, here how I implemented my auth flow in fastify, with fastify-oauth and JWT creation to return to the user through query string:

import { FastifyInstance } from "fastify";
import fastifyOauth2 from "@fastify/oauth2";
import { IdTokenClaims } from "../../types";
import { ShinzoError } from "../helpers";

export default async (fastify: FastifyInstance) => {
  const container = fastify.container;

  fastify.register(fastifyOauth2, {
    name: "google",
    credentials: {
      auth: fastifyOauth2.GOOGLE_CONFIGURATION,
      client: {
        id: process.env.GOOGLE_CLIENT_ID,
        secret: process.env.GOOGLE_CLIENT_SECRET,
      },
    },
    scope: ["profile", "email"],
    startRedirectPath: "/google",
    callbackUri: `${process.env.BASE_URL}/auth/google/callback`,
  });

  fastify.route({
    method: "GET",
    url: "/google/callback",
    handler: async (req, res) => {
      const { token } = await fastify.google.getAccessTokenFromAuthorizationCodeFlow(req);
      const claims = container.tokenService.decode<IdTokenClaims>(token.id_token as string);

      if (claims.isError()) throw new ShinzoError(claims.value);
      if (!claims.value.email)  throw new ShinzoError({ kind: "Unauthorized", message: "Forbidden" });

      const user = await container.userRepository.save({
        email: claims.value.email,
        avatar: claims.value.picture,
        firstName: claims.value.given_name,
        lastName: claims.value.family_name,
      });

      if (user.isError()) throw new ShinzoError(user.value);

      const jwt = await container.tokenService.sign({ id: user.value });

      if (jwt.isError()) throw new ShinzoError(jwt.value);

      return res.redirect(`${process.env.WEB_URL}?token=${jwt.value}`);
    },
  });
};

I'm using the @swan-io/boxed library to get Result / Option classes (Functional Programming), that why you can see (.value,isError(), etc).
I'm available if someone got any questions. Tag me 😄

@LiezarZ
Copy link

LiezarZ commented Aug 17, 2023

@carere many thanks for posting your code and your help! But unfortunately I still cant get it to work. :(
I noticed, that your callback route is GET where mine is POST instead. Do you use the data-callback attribut in your google button which is placed on the html page and does it work for you? I am using data-login_uri which makes a POST request, as data-callback didnt work for me. Maybe thats an important difference aswell?

@marco-ippolito I also tried your hint and using startRedirectPath: "/", again, as / is the start point for login (because the login button is placed there, so I guess it should be correct). But the request.query I receive is still empty and therefore getAccessTokenFromAuthorizationCodeFlow brings an invalid state again.

Out of curiosity I set up another test project this evening and used fastify-passport for google oauth. There my setup works. I get an filled requested with cookies, user data etc. and I would assume that fastify-passport is more complex to configure. Therefore I still would prefer to use fastify-oauth2 .

Many thanks again for all your help and have a great evening!

@JohnAllenTech
Copy link

Looks like the request.cookies['oauth2-redirect-state'] isnt getting populated by Google when it hits the callback. Just ran into this issue. I am only getting a POC together so dont have much time to debug further but I just overwrote the default checkState and generateState to get around it for the time being.

checkStateFunction: (request, callback) => { callback(); }, generateStateFunction: () => 123,

@FlawTECH
Copy link

FlawTECH commented Apr 29, 2024

Looks like the request.cookies['oauth2-redirect-state'] isnt getting populated by Google

Google is not the one responsible for the oauth2-redirect-state cookie. This module is the one creating and reading this cookie (to check state before / after oauth2 interaction).

I was also scratching my head as to why this cookie wasn't set, even though it was present in the request headers when visiting the startRedirectPath in my browser, but the answer is quite simple (at least in mine and OP's case). It is set, just not where you expect it to be.

Let me explain. By default, when you don't set a path when registering a cookie, as per RFC6265, the user agent will use the "directory" of the request-uri's path component as the default value. This essentially means that if your callbackUri is not in the same path as your startRedirectPath, the cookie won't be visible after the redirect is made.

You have 2 options:

  • have callbackUri share the same base path as startRedirectPath (so if startRedirectPath is /oauth2/google/login, your callbackUri could be /oauth2/google/callback but not /oauth2/callback)
  • set cookie.path in the module options to something more generic like /

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

8 participants