PGS v1 to v2 In Game account migration sample #106
Conversation
Summary of ChangesHello @wickedcube, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a crucial sample implementation designed to guide developers through the process of migrating user authentication from Google Play Games Services v1 to v2. The core innovation lies in its ability to maintain a player's identity across this transition by consistently utilizing the OpenID Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable sample for migrating from Google Play Games Services v1 to v2, complete with a Node.js backend and a Unity client. The implementation is well-documented and effectively demonstrates the core migration path using the OpenID sub claim for identity preservation.
My review identifies several critical and high-severity issues that should be addressed. These include a security vulnerability in the Unity project settings that allows insecure HTTP requests in production, a critical bug in the Node.js server that will cause it to crash on supported Node versions, and potential memory leaks in the C# client from undisposed UnityWebRequest objects. I've also provided suggestions to improve code maintainability, configuration, and robustness.
| const debugResponse = await fetch(debugUrl); | ||
| const debugData = await debugResponse.json(); |
There was a problem hiding this comment.
The README.md specifies Node.js v14+, but the native fetch API was only stabilized in v18. Using fetch here will cause the server to crash on Node.js versions 14, 16, and 17.
Since axios is already a dependency, you should use it for HTTP requests to ensure compatibility.
- Add
const axios = require('axios');at the top of the file. - Replace this
fetchcall and the one on line 222 withaxios.
| const debugResponse = await fetch(debugUrl); | |
| const debugData = await debugResponse.json(); | |
| const debugResponse = await axios.get(debugUrl); | |
| const debugData = debugResponse.data; |
| platformRequiresReadableAssets: 0 | ||
| virtualTexturingSupportEnabled: 0 | ||
| insecureHttpOption: 0 | ||
| insecureHttpOption: 2 |
There was a problem hiding this comment.
Setting insecureHttpOption to 2 (AlwaysAllowed) allows the application to make insecure HTTP requests in all builds, including production releases. This is a significant security risk. As noted in the README.md, this should be set to Allowed in development builds, which corresponds to a value of 1.
insecureHttpOption: 1
| "axios": "^1.13.1", | ||
| "dotenv": "^17.2.3", | ||
| "express": "^5.1.0", | ||
| "google-auth-library": "^10.4.1", | ||
| "jsonwebtoken": "^9.0.2" |
There was a problem hiding this comment.
Several of the package versions listed in dependencies do not correspond to public releases on npm (e.g., axios, dotenv, express, google-auth-library). This will likely cause npm install to fail unless you are using a private registry. Please verify these versions and update them to valid public versions if applicable.
Example with current public versions:
"dependencies": {
"axios": "^1.7.2",
"dotenv": "^16.4.5",
"express": "^4.19.2",
"google-auth-library": "^9.11.0",
"jsonwebtoken": "^9.0.2"
}| "axios": "^1.13.1", | |
| "dotenv": "^17.2.3", | |
| "express": "^5.1.0", | |
| "google-auth-library": "^10.4.1", | |
| "jsonwebtoken": "^9.0.2" | |
| "axios": "^1.7.2", | |
| "dotenv": "^16.4.5", | |
| "express": "^4.19.2", | |
| "google-auth-library": "^9.11.0", | |
| "jsonwebtoken": "^9.0.2" |
| statusText.text = "Initializing Google Sign-In..."; | ||
| GoogleSignIn.Configuration = new GoogleSignInConfiguration | ||
| { | ||
| WebClientId = "", |
There was a problem hiding this comment.
The WebClientId is hardcoded as an empty string, which will cause the PGS v2 authentication flow to fail. For easier configuration in a sample project, you should expose this as a public field in the Unity Inspector instead of requiring developers to edit the script directly.
First, add a public field to the class (e.g., near serverUrl):
public string serverUrl;
public string webClientId;Then, use this field in the configuration.
WebClientId = webClientId,| string jsonPayload = JsonUtility.ToJson(requestData); | ||
| byte[] bodyRaw = Encoding.UTF8.GetBytes(jsonPayload); | ||
|
|
||
| UnityWebRequest request = new UnityWebRequest(verify_and_link_google, "POST"); |
There was a problem hiding this comment.
UnityWebRequest implements IDisposable. Failing to dispose of it can lead to memory leaks and performance issues. You should wrap UnityWebRequest instances in a using block to ensure they are disposed of correctly, even if an error occurs. This applies to all UnityWebRequest usages in this file (in ExchangeAuthcodeAndLink, PostScore, and VerifyAndLinkFacebookAccount).
Example:
using (UnityWebRequest request = new UnityWebRequest(verify_and_link_google, "POST"))
{
// ... setup and send request
yield return request.SendWebRequest();
// ... handle response
}| "main": "index.js", | ||
| "scripts": { | ||
| "test": "echo \"Error: no test specified\" && exit 1" | ||
| }, |
There was a problem hiding this comment.
The main entry points to index.js, but the server file is server.js. This should be corrected to avoid issues with tools that rely on this field. Additionally, adding a start script is a common convention that makes running the server easier (npm start).
| "main": "index.js", | |
| "scripts": { | |
| "test": "echo \"Error: no test specified\" && exit 1" | |
| }, | |
| "main": "server.js", | |
| "scripts": { | |
| "start": "node server.js", | |
| "test": "echo \"Error: no test specified\" && exit 1" | |
| }, |
| 1. **Install Dependencies**: | ||
|
|
||
| ```bash | ||
| npm install express google-auth-library jsonwebtoken dotenv |
There was a problem hiding this comment.
The npm install command is missing the axios package. This package is a dependency in package.json and is required for the Facebook authentication flow to work correctly across all supported Node.js versions (once fetch is replaced).
| npm install express google-auth-library jsonwebtoken dotenv | |
| npm install express google-auth-library jsonwebtoken dotenv axios |
| let inGameAccountID; | ||
|
|
||
| if (userDatabase.has(dbKey)) { | ||
| // User already exists, retrieve their ID | ||
| inGameAccountID = userDatabase.get(dbKey); | ||
| console.log(`(PGS v1) Existing user. In-Game ID: ${inGameAccountID}`); | ||
| } else { | ||
| // New user, create a new in-game ID and store it | ||
| inGameAccountID = `ingame-${nextInGameAccountId++}`; | ||
|
|
||
| userDatabase.set(dbKey, inGameAccountID); | ||
| inGameDatabase.set(inGameAccountID, 0); | ||
|
|
||
| console.log(`(PGS v1) New user. Created In-Game ID: ${inGameAccountID}`); | ||
| } |
There was a problem hiding this comment.
The logic for finding or creating a user is duplicated across the /verify_and_link_google, /exchange_authcode_and_link, and /verify_and_link_facebook endpoints. This makes the code harder to maintain.
Consider refactoring this logic into a single helper function to reduce code duplication and make future changes easier.
Example:
function findOrCreateUser(dbKey, provider) {
let inGameAccountID;
if (userDatabase.has(dbKey)) {
inGameAccountID = userDatabase.get(dbKey);
console.log(`(${provider}) Existing user. In-Game ID: ${inGameAccountID}`);
} else {
inGameAccountID = `ingame-${nextInGameAccountId++}`;
userDatabase.set(dbKey, inGameAccountID);
inGameDatabase.set(inGameAccountID, 0);
console.log(`(${provider}) New user. Created In-Game ID: ${inGameAccountID}`);
}
return inGameAccountID;
}| { | ||
| #if PGS_V1 | ||
| public string idToken; | ||
| public string playerID; |
There was a problem hiding this comment.
The playerID field in the GoogleAuthRequest class is defined but never assigned a value or used on the server side for the PGS v1 flow. It should be removed to improve code clarity.
| var currNum = int.Parse(incText.text); | ||
| currNum++; | ||
| incText.text = currNum.ToString(); | ||
|
|
||
| StartCoroutine(PostScore()); |
There was a problem hiding this comment.
Using int.Parse can throw a FormatException if the text in incText is not a valid integer. This could happen if the UI element is somehow modified to be non-numeric. Using int.TryParse is a more robust way to handle this conversion.
if (int.TryParse(incText.text, out var currNum))
{
currNum++;
incText.text = currNum.ToString();
StartCoroutine(PostScore());
}
else
{
Debug.LogError($"Failed to parse '{incText.text}' as an integer.");
}
natetrost
left a comment
natetrost
left a comment
There was a problem hiding this comment.
Please add the apache license header to the new source files where applicable, then looks good to merge.
2 participants
Adds a sample implementation demonstrating how to migrate user authentication from PGS v1 to v2 while preserving identity via the OpenID
subclaim.Includes:
server.js): Unifies legacy v1 ID Token verification and new v2 Auth Code exchange into a single user identity.AuthManager.cs): Uses preprocessor symbols (PGS_V1,PGS_V2) to toggle between the different SDK implementations.README.md): Full guide on dependency cleanup, Client ID configuration, and testing the upgrade path.