Browse Source

Fix: Solve comments to pull request

pull/6075/head
Germán Martín 1 week ago
parent
commit
7e45c97a9a
  1. 29
      apps/api/src/app/auth/auth.controller.ts
  2. 10
      apps/api/src/app/auth/auth.module.ts
  3. 34
      apps/api/src/app/auth/auth.service.ts
  4. 16
      apps/api/src/app/auth/interfaces/interfaces.ts
  5. 53
      apps/api/src/app/auth/oidc-state.store.ts
  6. 32
      apps/api/src/app/auth/oidc.strategy.ts
  7. 5
      apps/client/src/app/components/user-account-settings/user-account-settings.component.ts

29
apps/api/src/app/auth/auth.controller.ts

@ -10,10 +10,12 @@ import {
import { import {
Body, Body,
ConflictException,
Controller, Controller,
Get, Get,
HttpException, HttpException,
Logger, Logger,
NotFoundException,
Param, Param,
Post, Post,
Req, Req,
@ -126,17 +128,18 @@ export class AuthController {
@Req() request: Request, @Req() request: Request,
@Res() response: Response @Res() response: Response
) { ) {
const result = request.user as OidcValidationResult; const { linkState, thirdPartyId, jwt } =
request.user as OidcValidationResult;
const rootUrl = this.configurationService.get('ROOT_URL'); const rootUrl = this.configurationService.get('ROOT_URL');
// Check if this is a link mode callback // Check if this is a link mode callback
if (result.linkState?.linkMode) { if (linkState?.linkMode) {
try { try {
// Link the OIDC account to the existing user // Link the OIDC account to the existing user
await this.authService.linkOidcToUser( await this.authService.linkOidcToUser({
result.linkState.userId, thirdPartyId,
result.thirdPartyId userId: linkState.userId
); });
// Redirect to account page with success message // Redirect to account page with success message
response.redirect( response.redirect(
@ -150,14 +153,14 @@ export class AuthController {
'AuthController' 'AuthController'
); );
// Determine error type for frontend // Determine error type for frontend based on error type
let errorCode = 'unknown'; let errorCode = 'unknown';
if (errorMessage.includes('already linked')) { if (error instanceof ConflictException) {
errorCode = 'already-linked'; errorCode = error.message.includes('token authentication')
} else if (errorMessage.includes('not found')) { ? 'invalid-provider'
: 'already-linked';
} else if (error instanceof NotFoundException) {
errorCode = 'invalid-session'; errorCode = 'invalid-session';
} else if (errorMessage.includes('token authentication')) {
errorCode = 'invalid-provider';
} }
response.redirect( response.redirect(
@ -168,8 +171,6 @@ export class AuthController {
} }
// Normal OIDC login flow // Normal OIDC login flow
const jwt: string = result.jwt;
if (jwt) { if (jwt) {
response.redirect(`${rootUrl}/${DEFAULT_LANGUAGE_CODE}/auth/${jwt}`); response.redirect(`${rootUrl}/${DEFAULT_LANGUAGE_CODE}/auth/${jwt}`);
} else { } else {

10
apps/api/src/app/auth/auth.module.ts

@ -17,6 +17,7 @@ import { AuthController } from './auth.controller';
import { AuthService } from './auth.service'; import { AuthService } from './auth.service';
import { GoogleStrategy } from './google.strategy'; import { GoogleStrategy } from './google.strategy';
import { JwtStrategy } from './jwt.strategy'; import { JwtStrategy } from './jwt.strategy';
import { OidcStateStore } from './oidc-state.store';
import { OidcStrategy } from './oidc.strategy'; import { OidcStrategy } from './oidc.strategy';
@Module({ @Module({
@ -39,11 +40,13 @@ import { OidcStrategy } from './oidc.strategy';
AuthService, AuthService,
GoogleStrategy, GoogleStrategy,
JwtStrategy, JwtStrategy,
OidcStateStore,
{ {
inject: [AuthService, ConfigurationService], inject: [AuthService, OidcStateStore, ConfigurationService],
provide: OidcStrategy, provide: OidcStrategy,
useFactory: async ( useFactory: async (
authService: AuthService, authService: AuthService,
stateStore: OidcStateStore,
configurationService: ConfigurationService configurationService: ConfigurationService
) => { ) => {
const isOidcEnabled = configurationService.get( const isOidcEnabled = configurationService.get(
@ -113,10 +116,7 @@ import { OidcStrategy } from './oidc.strategy';
clientSecret: configurationService.get('OIDC_CLIENT_SECRET') clientSecret: configurationService.get('OIDC_CLIENT_SECRET')
}; };
// Pass JWT secret for link mode validation return new OidcStrategy(authService, stateStore, options);
const jwtSecret = configurationService.get('JWT_SECRET_KEY');
return new OidcStrategy(authService, { ...options, jwtSecret });
} }
}, },
WebAuthService WebAuthService

34
apps/api/src/app/auth/auth.service.ts

@ -4,13 +4,18 @@ import { PropertyService } from '@ghostfolio/api/services/property/property.serv
import { import {
ConflictException, ConflictException,
ForbiddenException,
Injectable, Injectable,
InternalServerErrorException, InternalServerErrorException,
Logger Logger,
NotFoundException
} from '@nestjs/common'; } from '@nestjs/common';
import { JwtService } from '@nestjs/jwt'; import { JwtService } from '@nestjs/jwt';
import { ValidateOAuthLoginParams } from './interfaces/interfaces'; import {
LinkOidcToUserParams,
ValidateOAuthLoginParams
} from './interfaces/interfaces';
@Injectable() @Injectable()
export class AuthService { export class AuthService {
@ -61,7 +66,7 @@ export class AuthService {
await this.propertyService.isUserSignupEnabled(); await this.propertyService.isUserSignupEnabled();
if (!isUserSignupEnabled) { if (!isUserSignupEnabled) {
throw new Error('Sign up forbidden'); throw new ForbiddenException('Sign up forbidden');
} }
// Create new user if not found // Create new user if not found
@ -92,16 +97,17 @@ export class AuthService {
* The user must have provider ANONYMOUS (token-based auth). * The user must have provider ANONYMOUS (token-based auth).
* The thirdPartyId must not be already linked to another user. * The thirdPartyId must not be already linked to another user.
* *
* @param userId - The ID of the user to link * @param params - Parameters for linking OIDC to user
* @param thirdPartyId - The OIDC subject identifier * @param params.userId - The ID of the user to link
* @param params.thirdPartyId - The OIDC subject identifier
* @returns JWT token for the linked user * @returns JWT token for the linked user
* @throws ConflictException if thirdPartyId is already linked to another user * @throws ConflictException if thirdPartyId is already linked to another user
* @throws Error if user not found or has invalid provider * @throws Error if user not found or has invalid provider
*/ */
public async linkOidcToUser( public async linkOidcToUser({
userId: string, thirdPartyId,
thirdPartyId: string userId
): Promise<string> { }: LinkOidcToUserParams): Promise<string> {
// Check if thirdPartyId is already linked to another user // Check if thirdPartyId is already linked to another user
const [existingUser] = await this.userService.users({ const [existingUser] = await this.userService.users({
where: { thirdPartyId } where: { thirdPartyId }
@ -130,17 +136,19 @@ export class AuthService {
const user = await this.userService.user({ id: userId }); const user = await this.userService.user({ id: userId });
if (!user) { if (!user) {
throw new Error('User not found'); throw new NotFoundException('User not found');
} }
if (user.provider !== 'ANONYMOUS') { if (user.provider !== 'ANONYMOUS') {
throw new Error('Only users with token authentication can link OIDC'); throw new ConflictException(
'Only users with token authentication can link OIDC'
);
} }
// Update user with thirdPartyId and switch provider to OIDC // Update user with thirdPartyId and switch provider to OIDC
await this.userService.updateUser({ await this.userService.updateUser({
where: { id: userId }, data: { thirdPartyId, provider: 'OIDC' },
data: { thirdPartyId, provider: 'OIDC' } where: { id: userId }
}); });
return this.jwtService.sign({ id: userId }); return this.jwtService.sign({ id: userId });

16
apps/api/src/app/auth/interfaces/interfaces.ts

@ -25,6 +25,22 @@ export interface OidcProfile {
sub?: string; sub?: string;
} }
export interface LinkOidcToUserParams {
thirdPartyId: string;
userId: string;
}
export interface OidcLinkState {
linkMode: boolean;
userId: string;
}
export interface OidcValidationResult {
jwt?: string;
linkState?: OidcLinkState;
thirdPartyId: string;
}
export interface ValidateOAuthLoginParams { export interface ValidateOAuthLoginParams {
provider: Provider; provider: Provider;
thirdPartyId: string; thirdPartyId: string;

53
apps/api/src/app/auth/oidc-state.store.ts

@ -1,24 +1,20 @@
import { Logger } from '@nestjs/common'; import { Injectable, Logger } from '@nestjs/common';
import * as jwt from 'jsonwebtoken'; import { JwtService } from '@nestjs/jwt';
import ms from 'ms'; import ms from 'ms';
export interface OidcLinkState { import { OidcLinkState } from './interfaces/interfaces';
linkMode: boolean;
userId: string;
}
/** /**
* Custom state store for OIDC authentication that doesn't rely on express-session. * Custom state store for OIDC authentication that doesn't rely on express-session.
* This store manages OAuth2 state parameters in memory with automatic cleanup. * This store manages OAuth2 state parameters in memory with automatic cleanup.
* Supports link mode for linking existing token-authenticated users to OIDC. * Supports link mode for linking existing token-authenticated users to OIDC.
*/ */
@Injectable()
export class OidcStateStore { export class OidcStateStore {
private readonly STATE_EXPIRY_MS = ms('10 minutes'); private readonly STATE_EXPIRY_MS = ms('10 minutes');
private pendingLinkState?: OidcLinkState; private pendingLinkState?: OidcLinkState;
private jwtSecret?: string;
private stateMap = new Map< private stateMap = new Map<
string, string,
{ {
@ -30,11 +26,23 @@ export class OidcStateStore {
} }
>(); >();
public constructor(private readonly jwtService: JwtService) {}
/** /**
* Set the JWT secret for token validation in link mode * Get and clear pending link state (used internally by store)
*/ */
public setJwtSecret(secret: string) { public getPendingLinkState(): OidcLinkState | undefined {
this.jwtSecret = secret; const linkState = this.pendingLinkState;
this.pendingLinkState = undefined;
return linkState;
}
/**
* Set link state for an existing or upcoming state entry.
* This allows the controller to attach user information before the OIDC flow starts.
*/
public setLinkStateForNextStore(linkState: OidcLinkState) {
this.pendingLinkState = linkState;
} }
/** /**
@ -74,11 +82,9 @@ export class OidcStateStore {
token = request.headers.authorization.substring(7); token = request.headers.authorization.substring(7);
} }
if (token && this.jwtSecret) { if (token) {
try { try {
const decoded = jwt.verify(token, this.jwtSecret) as { const decoded = this.jwtService.verify<{ id: string }>(token);
id: string;
};
if (decoded?.id) { if (decoded?.id) {
linkState = { linkState = {
linkMode: true, linkMode: true,
@ -187,21 +193,4 @@ export class OidcStateStore {
Date.now().toString(36) Date.now().toString(36)
); );
} }
/**
* Set link state for an existing or upcoming state entry.
* This allows the controller to attach user information before the OIDC flow starts.
*/
public setLinkStateForNextStore(linkState: OidcLinkState) {
this.pendingLinkState = linkState;
}
/**
* Get and clear pending link state (used internally by store)
*/
public getPendingLinkState(): OidcLinkState | undefined {
const linkState = this.pendingLinkState;
this.pendingLinkState = undefined;
return linkState;
}
} }

32
apps/api/src/app/auth/oidc.strategy.ts

@ -8,43 +8,25 @@ import { AuthService } from './auth.service';
import { import {
OidcContext, OidcContext,
OidcIdToken, OidcIdToken,
OidcLinkState,
OidcParams, OidcParams,
OidcProfile OidcProfile,
OidcValidationResult
} from './interfaces/interfaces'; } from './interfaces/interfaces';
import { OidcLinkState, OidcStateStore } from './oidc-state.store'; import { OidcStateStore } from './oidc-state.store';
export interface OidcValidationResult {
jwt?: string;
linkState?: OidcLinkState;
thirdPartyId: string;
}
export interface OidcStrategyOptions extends StrategyOptions {
jwtSecret?: string;
}
@Injectable() @Injectable()
export class OidcStrategy extends PassportStrategy(Strategy, 'oidc') { export class OidcStrategy extends PassportStrategy(Strategy, 'oidc') {
private static readonly stateStore = new OidcStateStore();
public static getStateStore(): OidcStateStore {
return OidcStrategy.stateStore;
}
public constructor( public constructor(
private readonly authService: AuthService, private readonly authService: AuthService,
options: OidcStrategyOptions stateStore: OidcStateStore,
options: StrategyOptions
) { ) {
super({ super({
...options, ...options,
passReqToCallback: true, passReqToCallback: true,
store: OidcStrategy.stateStore store: stateStore
}); });
// Configure JWT secret for link mode validation
if (options.jwtSecret) {
OidcStrategy.stateStore.setJwtSecret(options.jwtSecret);
}
} }
public async validate( public async validate(

5
apps/client/src/app/components/user-account-settings/user-account-settings.component.ts

@ -121,11 +121,11 @@ export class GfUserAccountSettingsComponent implements OnDestroy, OnInit {
this.baseCurrency = baseCurrency; this.baseCurrency = baseCurrency;
this.currencies = currencies; this.currencies = currencies;
// Check global permissions for auth methods
this.hasPermissionForAuthOidc = hasPermission( this.hasPermissionForAuthOidc = hasPermission(
globalPermissions, globalPermissions,
permissions.enableAuthOidc permissions.enableAuthOidc
); );
this.hasPermissionForAuthToken = hasPermission( this.hasPermissionForAuthToken = hasPermission(
globalPermissions, globalPermissions,
permissions.enableAuthToken permissions.enableAuthToken
@ -145,6 +145,7 @@ export class GfUserAccountSettingsComponent implements OnDestroy, OnInit {
this.hasPermissionForAuthToken && this.hasPermissionForAuthToken &&
this.user.provider === 'ANONYMOUS' && this.user.provider === 'ANONYMOUS' &&
!!this.user.thirdPartyId; !!this.user.thirdPartyId;
this.canLinkOidc = this.canLinkOidc =
this.hasPermissionForAuthOidc && this.hasPermissionForAuthOidc &&
this.hasPermissionForAuthToken && this.hasPermissionForAuthToken &&
@ -217,8 +218,6 @@ export class GfUserAccountSettingsComponent implements OnDestroy, OnInit {
return 'Security Token'; return 'Security Token';
case 'GOOGLE': case 'GOOGLE':
return 'Google'; return 'Google';
case 'INTERNET_IDENTITY':
return 'Internet Identity';
case 'OIDC': case 'OIDC':
return 'OpenID Connect (OIDC)'; return 'OpenID Connect (OIDC)';
default: default:

Loading…
Cancel
Save