fastify / fastify-oauth2

Enable to perform login using oauth2 protocol
MIT License
254 stars 73 forks source link

Discord auth invalid state #218

Closed flav-code closed 3 months ago

flav-code commented 1 year ago

Prerequisites

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

flav-code commented 1 year ago

just this package version

mcollina commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

Uzlopak commented 1 year ago

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.

mcollina commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.

carere commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

LiezarZ commented 1 year ago

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 commented 1 year ago

@marco-ippolito PTAL

marco-ippolito commented 1 year ago

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 commented 1 year ago

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:

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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

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 commented 6 months ago

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:

flav-code commented 3 months ago

Someone ?

FlawTECH commented 3 months ago

Someone ?

Have you tried with the changes I suggested ? I see in your very first image, in the OP, that startRedirectPath and callbackUri are on different base paths.

As I explained above, and following your code, your browser will set a cookie with a base path of /discord Then, when checking the cookie, you redirect the user to /v1/auth/callback/discord (so, we're in a different path than /discord). Effectively, this means, that the cookie is inaccessible from this callback, resulting in an invalid state.

You could, for example, set startRedirectPath to /v1/auth/discord/login and callbackUri to /v1/auth/discord/callback. In this case, they both share /v1/auth/discord, so the cookie will be read.

flav-code commented 3 months ago

Oh ok, will test now

flav-code commented 3 months ago

It fixed my issue ! we can close this issue Thank you a lot @FlawTECH