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.

Sign up for GitHub

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

Fix for Invalid Service Token error - Refresh Token Duration same as Access Token #152

Merged
merged 12 commits into from
Feb 2, 2025
Merged

Fix for Invalid Service Token error - Refresh Token Duration same as Access Token #152

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
1ecb7ad
fix: make refresh token expire longer than access token, and centrali…
binaryoverload Jan 29, 2025
79bc908
chore: don't use await when not needed
binaryoverload Jan 29, 2025
e3c7847
style: fix linting issues
binaryoverload Jan 29, 2025
2a00924
feat: use "enum" for token type
binaryoverload Jan 30, 2025
cad2d0e
chore: use different oauth refresh expiry for NNAS and API
binaryoverload Jan 30, 2025
169e686
feat: use "enum" for token system type
binaryoverload Jan 30, 2025
577b502
style: fix eslint issues in login
binaryoverload Jan 30, 2025
c7a230b
style: add return type to fix eslint
binaryoverload Jan 31, 2025
8f2e6fc
fix: use enum values for token property comparison
binaryoverload Jan 31, 2025
fdc122b
chore: resolve review comments
binaryoverload Feb 1, 2025
1197f71
fix: ensure check returns
binaryoverload Feb 1, 2025
0578bfd
chore: move back to number types for token, with const enum
binaryoverload Feb 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/database.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ export async function getPNIDByNNASAccessToken(token: string): Promise<HydratedP
const unpackedToken = unpackToken(decryptedToken);

// * Return if the system type isn't Wii U (NNAS) and the token type isn't "OAuth Access"
if (unpackedToken.system_type !== 1 || unpackedToken.token_type !== 1) {
if (unpackedToken.system_type !== 'WIIU' || unpackedToken.token_type !== 'OAUTH_ACCESS') {
return null;
}

Expand Down Expand Up @@ -142,7 +142,7 @@ export async function getPNIDByNNASRefreshToken(token: string): Promise<Hydrated
const unpackedToken = unpackToken(decryptedToken);

// * Return if the system type isn't Wii U (NNAS) and the token type isn't "OAuth Refresh"
if (unpackedToken.system_type !== 1 || unpackedToken.token_type !== 2) {
if (unpackedToken.system_type !== 'WIIU' || unpackedToken.token_type !== 'OAUTH_REFRESH') {
return null;
}

Expand Down Expand Up @@ -172,7 +172,7 @@ export async function getPNIDByAPIAccessToken(token: string): Promise<HydratedPN
const unpackedToken = unpackToken(decryptedToken);

// * Return if the system type isn't API (REST and gRPC) and the token type isn't "OAuth Access"
if (unpackedToken.system_type !== 3 || unpackedToken.token_type !== 1) {
if (unpackedToken.system_type !== 'API' || unpackedToken.token_type !== 'OAUTH_ACCESS') {
return null;
}

Expand Down Expand Up @@ -202,7 +202,7 @@ export async function getPNIDByAPIRefreshToken(token: string): Promise<HydratedP
const unpackedToken = unpackToken(decryptedToken);

// * Return if the system type isn't API (REST and gRPC) and the token type isn't "OAuth Refresh"
if (unpackedToken.system_type !== 3 || unpackedToken.token_type !== 2) {
if (unpackedToken.system_type !== 'API' || unpackedToken.token_type !== 'OAUTH_REFRESH') {
return null;
}

Expand Down
8 changes: 7 additions & 1 deletion src/models/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Schema, model } from 'mongoose';
import uniqueValidator from 'mongoose-unique-validator';
import { IServer, IServerMethods, ServerModel } from '@/types/mongoose/server';
import type { SystemType } from '@/types/common/token';

const ServerSchema = new Schema<IServer, ServerModel, IServerMethods>({
client_id: String,
Expand All @@ -18,4 +19,9 @@ const ServerSchema = new Schema<IServer, ServerModel, IServerMethods>({

ServerSchema.plugin(uniqueValidator, { message: '{PATH} already in use.' });

export const Server = model<IServer, ServerModel>('Server', ServerSchema);
export const Server = model<IServer, ServerModel>('Server', ServerSchema);

export const serverDeviceToSystemType: Record<number, SystemType> = {
1: 'WIIU',
2: '3DS'
};
51 changes: 17 additions & 34 deletions src/services/api/routes/v1/login.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import express from 'express';
import bcrypt from 'bcrypt';
import { getPNIDByUsername, getPNIDByAPIRefreshToken } from '@/database';
import { nintendoPasswordHash, generateToken} from '@/util';
import { config } from '@/config-manager';
import { nintendoPasswordHash, generateOAuthTokens} from '@/util';
import { HydratedPNIDDocument } from '@/types/mongoose/pnid';

const router = express.Router();
Expand Down Expand Up @@ -109,38 +108,22 @@ router.post('/', async (request: express.Request, response: express.Response): P
return;
}

const accessTokenOptions = {
system_type: 0x3, // * API
token_type: 0x1, // * OAuth Access
pid: pnid.pid,
access_level: pnid.access_level,
title_id: BigInt(0),
expire_time: BigInt(Date.now() + (3600 * 1000))
};

const refreshTokenOptions = {
system_type: 0x3, // * API
token_type: 0x2, // * OAuth Refresh
pid: pnid.pid,
access_level: pnid.access_level,
title_id: BigInt(0),
expire_time: BigInt(Date.now() + (3600 * 1000))
};

const accessTokenBuffer = await generateToken(config.aes_key, accessTokenOptions);
const refreshTokenBuffer = await generateToken(config.aes_key, refreshTokenOptions);

const accessToken = accessTokenBuffer ? accessTokenBuffer.toString('hex') : '';
const newRefreshToken = refreshTokenBuffer ? refreshTokenBuffer.toString('hex') : '';

// TODO - Handle null tokens

response.json({
access_token: accessToken,
token_type: 'Bearer',
expires_in: 3600,
refresh_token: newRefreshToken
});
try {
const tokenGeneration = generateOAuthTokens('API', pnid, { refreshExpiresIn: 14 * 24 * 60 * 60 }); // * 14 days

response.json({
access_token: tokenGeneration.accessToken,
token_type: 'Bearer',
expires_in: tokenGeneration.expiresInSecs.access,
refresh_token: tokenGeneration.refreshToken
});
} catch {
response.status(500).json({
app: 'api',
status: 500,
error: 'Internal server error'
});
}
});

export default router;
50 changes: 17 additions & 33 deletions src/services/api/routes/v1/register.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import moment from 'moment';
import hcaptcha from 'hcaptcha';
import Mii from 'mii-js';
import { doesPNIDExist, connection as databaseConnection } from '@/database';
import { nintendoPasswordHash, sendConfirmationEmail, generateToken } from '@/util';
import { nintendoPasswordHash, sendConfirmationEmail, generateOAuthTokens } from '@/util';
import { LOG_ERROR } from '@/logger';
import { PNID } from '@/models/pnid';
import { NEXAccount } from '@/models/nex-account';
Expand Down Expand Up @@ -366,38 +366,22 @@ router.post('/', async (request: express.Request, response: express.Response): P

await sendConfirmationEmail(pnid);

const accessTokenOptions = {
system_type: 0x3, // * API
token_type: 0x1, // * OAuth Access
pid: pnid.pid,
access_level: pnid.access_level,
title_id: BigInt(0),
expire_time: BigInt(Date.now() + (3600 * 1000))
};

const refreshTokenOptions = {
system_type: 0x3, // * API
token_type: 0x2, // * OAuth Refresh
pid: pnid.pid,
access_level: pnid.access_level,
title_id: BigInt(0),
expire_time: BigInt(Date.now() + (3600 * 1000))
};

const accessTokenBuffer = await generateToken(config.aes_key, accessTokenOptions);
const refreshTokenBuffer = await generateToken(config.aes_key, refreshTokenOptions);

const accessToken = accessTokenBuffer ? accessTokenBuffer.toString('hex') : '';
const refreshToken = refreshTokenBuffer ? refreshTokenBuffer.toString('hex') : '';

// TODO - Handle null tokens

response.json({
access_token: accessToken,
token_type: 'Bearer',
expires_in: 3600,
refresh_token: refreshToken
});
try {
const tokenGeneration = generateOAuthTokens('API', pnid, { refreshExpiresIn: 14 * 24 * 60 * 60 }); // * 14 days

response.json({
access_token: tokenGeneration.accessToken,
token_type: 'Bearer',
expires_in: tokenGeneration.expiresInSecs.access,
refresh_token: tokenGeneration.refreshToken
});
} catch {
response.status(500).json({
app: 'api',
status: 500,
error: 'Internal server error'
});
}
});

export default router;
78 changes: 23 additions & 55 deletions src/services/grpc/api/login.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ import { Status, ServerError } from 'nice-grpc';
import { LoginRequest, LoginResponse, DeepPartial } from '@pretendonetwork/grpc/api/login_rpc';
import bcrypt from 'bcrypt';
import { getPNIDByUsername, getPNIDByAPIRefreshToken } from '@/database';
import { nintendoPasswordHash, generateToken} from '@/util';
import { config } from '@/config-manager';
import { nintendoPasswordHash, generateOAuthTokens} from '@/util';
import type { HydratedPNIDDocument } from '@/types/mongoose/pnid';

export async function login(request: LoginRequest): Promise<DeepPartial<LoginResponse>> {
Expand All @@ -16,74 +15,43 @@ export async function login(request: LoginRequest): Promise<DeepPartial<LoginRes
throw new ServerError(Status.INVALID_ARGUMENT, 'Invalid grant type');
}

if (grantType === 'password' && !username) {
throw new ServerError(Status.INVALID_ARGUMENT, 'Invalid or missing username');
}

if (grantType === 'password' && !password) {
throw new ServerError(Status.INVALID_ARGUMENT, 'Invalid or missing password');
}

if (grantType === 'refresh_token' && !refreshToken) {
throw new ServerError(Status.INVALID_ARGUMENT, 'Invalid or missing refresh token');
}

let pnid: HydratedPNIDDocument | null;

if (grantType === 'password') {
pnid = await getPNIDByUsername(username!); // * We know username will never be null here
if (!username) throw new ServerError(Status.INVALID_ARGUMENT, 'Invalid or missing username');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style nit for the common config, we always use full blocks

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will be linted later

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what this response means. I made the comment because I was under the impression that this was not part of the common config (hence why I said "for the common config"). It's not clear to me what the resolution here is based on this response. Is it missing and will be added later? Does it exist and the linter will just be ran later? Something else?

Copy link
Member Author

@binaryoverload binaryoverload Feb 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We agreed previously that if it's not formatted by the linter, review comments won't be made on it.

The eslint PR #151 hasn't been merged yet, so this will be formatted in accordance with the linter when both are merged

I'll update common-config

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been resolved in PretendoNetwork/common-configs#2

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More like PretendoNetwork/common-configs@3399c72

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We agreed previously that if it's not formatted by the linter, review comments won't be made on it

I know you said you misunderstood on Discord, but just to clarify (and to make the clarification public): comments like this are mean't only to bring up styling examples to be added to the common config, not necessarily to fix them in the current PR (though that is also welcomed). I figured I would save making an issue and just bring it up here with a real code example. Would you prefer issues on the common config repo directly instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah in future I think making an issue on the common config repo would be better 👍

if (!password) throw new ServerError(Status.INVALID_ARGUMENT, 'Invalid or missing password');

if (!pnid) {
throw new ServerError(Status.INVALID_ARGUMENT, 'User not found');
}
pnid = await getPNIDByUsername(username); // * We know username will never be null here

if (!pnid) throw new ServerError(Status.INVALID_ARGUMENT, 'User not found');

const hashedPassword = nintendoPasswordHash(password!, pnid.pid); // * We know password will never be null here
const hashedPassword = nintendoPasswordHash(password, pnid.pid); // * We know password will never be null here

if (!bcrypt.compareSync(hashedPassword, pnid.password)) {
throw new ServerError(Status.INVALID_ARGUMENT, 'Password is incorrect');
}
} else {
pnid = await getPNIDByAPIRefreshToken(refreshToken!); // * We know refreshToken will never be null here
if (!refreshToken) throw new ServerError(Status.INVALID_ARGUMENT, 'Invalid or missing refresh token');

if (!pnid) {
throw new ServerError(Status.INVALID_ARGUMENT, 'Invalid or missing refresh token');
}
pnid = await getPNIDByAPIRefreshToken(refreshToken); // * We know refreshToken will never be null here

if (!pnid) throw new ServerError(Status.INVALID_ARGUMENT, 'Invalid or missing refresh token');
}

if (pnid.deleted) {
throw new ServerError(Status.UNAUTHENTICATED, 'Account has been deleted');
}

const accessTokenOptions = {
system_type: 0x3, // * API
token_type: 0x1, // * OAuth Access
pid: pnid.pid,
access_level: pnid.access_level,
title_id: BigInt(0),
expire_time: BigInt(Date.now() + (3600 * 1000))
};

const refreshTokenOptions = {
system_type: 0x3, // * API
token_type: 0x2, // * OAuth Refresh
pid: pnid.pid,
access_level: pnid.access_level,
title_id: BigInt(0),
expire_time: BigInt(Date.now() + (3600 * 1000))
};

const accessTokenBuffer = await generateToken(config.aes_key, accessTokenOptions);
const refreshTokenBuffer = await generateToken(config.aes_key, refreshTokenOptions);

const accessToken = accessTokenBuffer ? accessTokenBuffer.toString('hex') : '';
const newRefreshToken = refreshTokenBuffer ? refreshTokenBuffer.toString('hex') : '';

// TODO - Handle null tokens

return {
accessToken: accessToken,
tokenType: 'Bearer',
expiresIn: 3600,
refreshToken: newRefreshToken
};
try {
const tokenGeneration = generateOAuthTokens('API', pnid, { refreshExpiresIn: 14 * 24 * 60 * 60 }); // * 14 days

return {
accessToken: tokenGeneration.accessToken,
tokenType: 'Bearer',
expiresIn: tokenGeneration.expiresInSecs.access,
refreshToken: tokenGeneration.refreshToken
};
} catch {
throw new ServerError(Status.INTERNAL, 'Could not generate OAuth tokens');
}
}
46 changes: 13 additions & 33 deletions src/services/grpc/api/register.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import moment from 'moment';
import hcaptcha from 'hcaptcha';
import Mii from 'mii-js';
import { doesPNIDExist, connection as databaseConnection } from '@/database';
import { nintendoPasswordHash, sendConfirmationEmail, generateToken } from '@/util';
import { nintendoPasswordHash, sendConfirmationEmail, generateOAuthTokens } from '@/util';
import { LOG_ERROR } from '@/logger';
import { PNID } from '@/models/pnid';
import { NEXAccount } from '@/models/nex-account';
Expand Down Expand Up @@ -229,36 +229,16 @@ export async function register(request: RegisterRequest): Promise<DeepPartial<Lo

await sendConfirmationEmail(pnid);

const accessTokenOptions = {
system_type: 0x3, // * API
token_type: 0x1, // * OAuth Access
pid: pnid.pid,
access_level: pnid.access_level,
title_id: BigInt(0),
expire_time: BigInt(Date.now() + (3600 * 1000))
};

const refreshTokenOptions = {
system_type: 0x3, // * API
token_type: 0x2, // * OAuth Refresh
pid: pnid.pid,
access_level: pnid.access_level,
title_id: BigInt(0),
expire_time: BigInt(Date.now() + (3600 * 1000))
};

const accessTokenBuffer = await generateToken(config.aes_key, accessTokenOptions);
const refreshTokenBuffer = await generateToken(config.aes_key, refreshTokenOptions);

const accessToken = accessTokenBuffer ? accessTokenBuffer.toString('hex') : '';
const refreshToken = refreshTokenBuffer ? refreshTokenBuffer.toString('hex') : '';

// TODO - Handle null tokens

return {
accessToken: accessToken,
tokenType: 'Bearer',
expiresIn: 3600,
refreshToken: refreshToken
};
try {
const tokenGeneration = generateOAuthTokens('API', pnid, { refreshExpiresIn: 14 * 24 * 60 * 60 }); // * 14 days

return {
accessToken: tokenGeneration.accessToken,
tokenType: 'Bearer',
expiresIn: tokenGeneration.expiresInSecs.access,
refreshToken: tokenGeneration.refreshToken
};
} catch {
throw new ServerError(Status.INTERNAL, 'Could not generate OAuth tokens');
}
}
Loading