1
0
mirror of https://github.com/Unleash/unleash.git synced 2025-07-07 01:16:28 +02:00

fix: admin token should be passed forward from controllers (#5960)

We were sending `user.id` to the service, but if an admin token is used,
there is no `user.id.` Instead, there is
`user.internalAdminTokenUserId`. so we need to use the special method
`extractUserIdFromUser`.

This PR adds this implementation, and now the service correctly
retrieves the appropriate ID for admins.

Related to: https://github.com/Unleash/unleash/pull/5924
This commit is contained in:
Jaanus Sellin 2024-01-30 11:03:15 +02:00 committed by GitHub
parent 2643ac1356
commit 832884b4f5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 53 additions and 21 deletions

View File

@ -2,7 +2,10 @@ import { Request, Response } from 'express';
import { IUnleashConfig } from '../../types/option'; import { IUnleashConfig } from '../../types/option';
import { IUnleashServices } from '../../types'; import { IUnleashServices } from '../../types';
import Controller from '../../routes/controller'; import Controller from '../../routes/controller';
import { extractUsername } from '../../util/extract-user'; import {
extractUserIdFromUser,
extractUsername,
} from '../../util/extract-user';
import { DELETE_FEATURE, NONE, UPDATE_FEATURE } from '../../types/permissions'; import { DELETE_FEATURE, NONE, UPDATE_FEATURE } from '../../types/permissions';
import FeatureToggleService from './feature-toggle-service'; import FeatureToggleService from './feature-toggle-service';
import { IAuthRequest } from '../../routes/unleash-types'; import { IAuthRequest } from '../../routes/unleash-types';
@ -142,7 +145,7 @@ export default class ArchiveController extends Controller {
const { user } = req; const { user } = req;
const features = await this.featureService.getAllArchivedFeatures( const features = await this.featureService.getAllArchivedFeatures(
true, true,
user.id, extractUserIdFromUser(user),
); );
this.openApiService.respondWithValidation( this.openApiService.respondWithValidation(
200, 200,

View File

@ -124,7 +124,9 @@ export interface IGetFeatureParams {
} }
export type FeatureNameCheckResultWithFeaturePattern = export type FeatureNameCheckResultWithFeaturePattern =
| { state: 'valid' } | {
state: 'valid';
}
| { | {
state: 'invalid'; state: 'invalid';
invalidNames: Set<string>; invalidNames: Set<string>;
@ -1008,7 +1010,11 @@ class FeatureToggleService {
userId, userId,
archived, archived,
); );
return { ...result, dependencies, children }; return {
...result,
dependencies,
children,
};
} else { } else {
const result = const result =
await this.featureStrategiesStore.getFeatureToggleWithEnvs( await this.featureStrategiesStore.getFeatureToggleWithEnvs(
@ -1016,7 +1022,11 @@ class FeatureToggleService {
userId, userId,
archived, archived,
); );
return { ...result, dependencies, children }; return {
...result,
dependencies,
children,
};
} }
} }
@ -1227,7 +1237,10 @@ class FeatureToggleService {
); );
if (result.state === 'invalid') { if (result.state === 'invalid') {
return { ...result, featureNaming: patternData }; return {
...result,
featureNaming: patternData,
};
} }
} }
} catch (error) { } catch (error) {
@ -1341,7 +1354,11 @@ class FeatureToggleService {
const cloneDependencies = const cloneDependencies =
this.dependentFeaturesService.cloneDependencies( this.dependentFeaturesService.cloneDependencies(
{ featureName, newFeatureName, projectId }, {
featureName,
newFeatureName,
projectId,
},
userName, userName,
userId, userId,
); );
@ -1362,7 +1379,10 @@ class FeatureToggleService {
featureName: string, featureName: string,
userId: number, userId: number,
): Promise<FeatureToggle> { ): Promise<FeatureToggle> {
await this.validateFeatureBelongsToProject({ featureName, projectId }); await this.validateFeatureBelongsToProject({
featureName,
projectId,
});
this.logger.info(`${userName} updates feature toggle ${featureName}`); this.logger.info(`${userName} updates feature toggle ${featureName}`);
@ -1901,7 +1921,11 @@ class FeatureToggleService {
const defaultEnv = environments.find((e) => e.name === DEFAULT_ENV); const defaultEnv = environments.find((e) => e.name === DEFAULT_ENV);
const strategies = defaultEnv?.strategies || []; const strategies = defaultEnv?.strategies || [];
const enabled = defaultEnv?.enabled || false; const enabled = defaultEnv?.enabled || false;
return { ...legacyFeature, enabled, strategies }; return {
...legacyFeature,
enabled,
strategies,
};
} }
async changeProject( async changeProject(
@ -2271,7 +2295,9 @@ class FeatureToggleService {
): Promise<IVariant[]> { ): Promise<IVariant[]> {
await variantsArraySchema.validateAsync(newVariants); await variantsArraySchema.validateAsync(newVariants);
const fixedVariants = this.fixVariantWeights(newVariants); const fixedVariants = this.fixVariantWeights(newVariants);
const oldVariants: { [env: string]: IVariant[] } = {}; const oldVariants: {
[env: string]: IVariant[];
} = {};
for (const env of environments) { for (const env of environments) {
const featureEnv = await this.featureEnvironmentStore.get({ const featureEnv = await this.featureEnvironmentStore.get({
featureName, featureName,

View File

@ -21,6 +21,7 @@ import {
playgroundViewModel, playgroundViewModel,
} from './playground-view-model'; } from './playground-view-model';
import { IAuthRequest } from '../../routes/unleash-types'; import { IAuthRequest } from '../../routes/unleash-types';
import { extractUserIdFromUser } from '../../util';
export default class PlaygroundController extends Controller { export default class PlaygroundController extends Controller {
private openApiService: OpenApiService; private openApiService: OpenApiService;
@ -129,7 +130,7 @@ export default class PlaygroundController extends Controller {
req.body.environments, req.body.environments,
req.body.context, req.body.context,
limit, limit,
user.id, extractUserIdFromUser(user),
); );
const response: AdvancedPlaygroundResponseSchema = const response: AdvancedPlaygroundResponseSchema =

View File

@ -1,7 +1,7 @@
import { Db } from '../../db/db'; import { Db } from '../../db/db';
import { Logger, LogProvider } from '../../logger'; import { Logger, LogProvider } from '../../logger';
import { IPrivateProjectStore } from './privateProjectStoreType'; import { IPrivateProjectStore } from './privateProjectStoreType';
import { ADMIN_TOKEN_ID } from '../../types'; import { ADMIN_TOKEN_USER } from '../../types';
export type ProjectAccess = export type ProjectAccess =
| { | {
@ -29,7 +29,7 @@ class PrivateProjectStore implements IPrivateProjectStore {
destroy(): void {} destroy(): void {}
async getUserAccessibleProjects(userId: number): Promise<ProjectAccess> { async getUserAccessibleProjects(userId: number): Promise<ProjectAccess> {
if (userId === ADMIN_TOKEN_ID) { if (userId === ADMIN_TOKEN_USER.id) {
return ALL_PROJECT_ACCESS; return ALL_PROJECT_ACCESS;
} }
const isViewer = await this.db('role_user') const isViewer = await this.db('role_user')

View File

@ -39,7 +39,7 @@ import {
SegmentsSchema, SegmentsSchema,
} from '../../openapi/spec/segments-schema'; } from '../../openapi/spec/segments-schema';
import { anonymiseKeys } from '../../util'; import { anonymiseKeys, extractUserIdFromUser } from '../../util';
import { BadDataError } from '../../error'; import { BadDataError } from '../../error';
type IUpdateFeatureStrategySegmentsRequest = IAuthRequest< type IUpdateFeatureStrategySegmentsRequest = IAuthRequest<
@ -351,7 +351,7 @@ export class SegmentsController extends Controller {
const { user } = req; const { user } = req;
const strategies = await this.segmentService.getVisibleStrategies( const strategies = await this.segmentService.getVisibleStrategies(
id, id,
user.id, extractUserIdFromUser(user),
); );
const segmentStrategies = strategies.strategies.map((strategy) => ({ const segmentStrategies = strategies.strategies.map((strategy) => ({

View File

@ -2,7 +2,10 @@ import { Request, Response } from 'express';
import Controller from '../controller'; import Controller from '../controller';
import { extractUsername } from '../../util/extract-user'; import {
extractUserIdFromUser,
extractUsername,
} from '../../util/extract-user';
import { import {
CREATE_CONTEXT_FIELD, CREATE_CONTEXT_FIELD,
@ -310,7 +313,7 @@ export class ContextController extends Controller {
const contextFields = const contextFields =
await this.contextService.getStrategiesByContextField( await this.contextService.getStrategiesByContextField(
contextField, contextField,
user.id, extractUserIdFromUser(user),
); );
this.openApiService.respondWithValidation( this.openApiService.respondWithValidation(

View File

@ -15,6 +15,7 @@ import {
} from '../../openapi/util/standard-responses'; } from '../../openapi/util/standard-responses';
import { CreateApplicationSchema } from '../../openapi/spec/create-application-schema'; import { CreateApplicationSchema } from '../../openapi/spec/create-application-schema';
import { IAuthRequest } from '../unleash-types'; import { IAuthRequest } from '../unleash-types';
import { extractUserIdFromUser } from '../../util';
class MetricsController extends Controller { class MetricsController extends Controller {
private logger: Logger; private logger: Logger;
@ -158,7 +159,7 @@ class MetricsController extends Controller {
: {}; : {};
const applications = await this.clientInstanceService.getApplications( const applications = await this.clientInstanceService.getApplications(
query, query,
user.id, extractUserIdFromUser(user),
); );
res.json({ applications }); res.json({ applications });
} }

View File

@ -1,6 +1,4 @@
import { ADMIN } from './permissions'; import { ADMIN } from './permissions';
export const ADMIN_TOKEN_ID = -1;
export default class NoAuthUser { export default class NoAuthUser {
isAPI: boolean; isAPI: boolean;
@ -12,7 +10,7 @@ export default class NoAuthUser {
constructor( constructor(
username: string = 'unknown', username: string = 'unknown',
id: number = ADMIN_TOKEN_ID, id: number = -1,
permissions: string[] = [ADMIN], permissions: string[] = [ADMIN],
) { ) {
this.isAPI = true; this.isAPI = true;