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

[Feedback] Authentication - @colyseus/auth #660

Open
endel opened this issue Dec 12, 2023 · 13 comments
Open

[Feedback] Authentication - @colyseus/auth #660

endel opened this issue Dec 12, 2023 · 13 comments
Milestone

Comments

@endel
Copy link
Member

endel commented Dec 12, 2023

This thread is for general feedback for the new @colyseus/auth module (#657, colyseus/colyseus.js#133, colyseus/docs#150)

@SleepingProgrammer
Copy link

TypeError: Cannot read properties of undefined (reading 'then')

Getting this from a fresh install, not sure if I missed some sort of setup stuff.

image

@endel
Copy link
Member Author

endel commented Dec 14, 2023

Hi @SleepingProgrammer, thanks for checking out!

The @colyseus/auth needs a preview version of @colyseus/core, you can force to use the preview version by adding this to your package.json:

  "overrides": {
    "@colyseus/core": "preview"
  },

(See config on webgame-template repo)

Soon the preview version will be incorporated on 0.15.x

@endel
Copy link
Member Author

endel commented Dec 27, 2023

Final release notes are out 🎉 https://github.com/colyseus/colyseus/releases/tag/0.15.15

@SleepingProgrammer
Copy link

Thanks for letting me know and happy new year!

I haven't had the time to try out your earlier suggestion, does that mean I can just force colyseus/core to use 0.15.15?

@endel
Copy link
Member Author

endel commented Jan 1, 2024

Hi @SleepingProgrammer, yes 🙌
Happy New Year ✨🍾

@JeromeGill
Copy link

https://docs.colyseus.io/authentication/module/#onfinduserbyemail-setting

What is the expected behavior if a user isn't found? are you supposed to throw an error yourself ?

I think the documentation needs to reflect this

Presumably the colysesus db package throws the error internally somewhere here https://github.com/colyseus/webgame-template/blob/main/packages/backend/src/config/auth.ts#L10

@endel
Copy link
Member Author

endel commented Jan 2, 2024

Hi @JeromeGill, thank you for the feedback.

There was a bug getting the return value from onFindUserByEmail at the login route - if null or undefined are returned, the invalid_credentials error should've been thrown. I just fixed this here.

You may customize the error by throwing a different one inside onFindUserByEmail too, updated the docs to clarify this behaviour here 🙌 https://docs.colyseus.io/authentication/module/#onfinduserbyemail-setting

@hunkydoryrepair
Copy link
Contributor

I'm a little concerned about the change to make onAuth static.
We do a lot of checks in onAuth that are specific to the room. Since each room is unique, but all share the same class, we need to know WHICH room they are in, and the static onAuth doesn't even appear to provide the options where we might determine which room is GOING to be created.

Also, we use the request passed in to get the IP address of the player, and then we need to SAVE it on the client.userData or in the options (I don't remember which). Using a static onAuth doesn't appear to provide a good way to do that. We could still get the IP address, but then associating it with the player during onJoin, is it possible?

I can see having a static onAuth being useful for optimizations, but deprecating the existing version creates some problems. We could still do a lot of it in onJoin, possibly, except we don't get the request in onJoin, so we can't get IP address.

@hunkydoryrepair
Copy link
Contributor

Just, for example what we pass to our onAuth and use from the client:

  • sessionKey from our authentication process (using Stytch)
  • The version number of the client, to determine if they need to refresh the client to get latest version.
  • A "captcha" string, so we can validate they are human.
  • A telemetry string for fingerprinting
  • A timestamp of when they validated, which is used in conjuction with our mapId (room indicator) to make sure they were validated for the given room and have not joined other rooms since being validated.
  • A room/map identifier, which is needed to select or create the room, but we also used in conjuction with the timestamp above.
  • A world number, for rooms where we allow duplication. We check validity and have some restrictions based on who the player is to access different world numbers.

We also do a check on the # of players in the room. This is so we can allow ADMIN into the rooms even if they are otherwise full for other players. This works because most of our rooms are "single instance" and we prevent creation of duplicate rooms. We turn people away if they are full, rather than spin up a new instance of the same area.

Conceivably, I think most of these things can be in onJoin, but we cannot benefit from the static onAuth unless we could access all the options passed in from the client.

@hungnvbb
Copy link

hungnvbb commented May 8, 2024

I expect static onAuth() when return data I can get it on onJoin or onCreate, currently can't get it . What purpose static onAuth () ??? .

@endel
Copy link
Member Author

endel commented May 8, 2024

Hi @hungnvbb, the return value from static onAuth() should be available in 2 places:

onJoin(client, options, auth) {
  console.log(client.auth) // here
  console.log(auth) // here
}

Let me know if that works for you

@hungnvbb
Copy link

hungnvbb commented May 9, 2024

Hi @endel This is my code .

static async onAuth(token,request) {
    if (!token) return false;
    try {
      const info = await verifyToken(token);
      if (!info) return false;
      return info;
    } catch (e) {
      logger.error(e);
      return false;
    }
  }

and in onJoin I printed auth , client.auth result is undefined .

@yjeroen
Copy link

yjeroen commented May 15, 2024

Wishlist:

  • Ability to configure password requirements
  • Ability to add 2FA Authenticator option (e.g. via email and/or an QR-code based 2fa Authenticator app)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants