Skip to content

Commit

Permalink
fix: admin token should be passed forward from controllers (#5960)
Browse files Browse the repository at this point in the history
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: #5924
  • Loading branch information
sjaanus committed Jan 30, 2024
1 parent 2643ac1 commit 832884b
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 21 deletions.
Expand Up @@ -2,7 +2,10 @@ import { Request, Response } from 'express';
import { IUnleashConfig } from '../../types/option';
import { IUnleashServices } from '../../types';
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 FeatureToggleService from './feature-toggle-service';
import { IAuthRequest } from '../../routes/unleash-types';
Expand Down Expand Up @@ -142,7 +145,7 @@ export default class ArchiveController extends Controller {
const { user } = req;
const features = await this.featureService.getAllArchivedFeatures(
true,
user.id,
extractUserIdFromUser(user),
);
this.openApiService.respondWithValidation(
200,
Expand Down
42 changes: 34 additions & 8 deletions src/lib/features/feature-toggle/feature-toggle-service.ts
Expand Up @@ -124,7 +124,9 @@ export interface IGetFeatureParams {
}

export type FeatureNameCheckResultWithFeaturePattern =
| { state: 'valid' }
| {
state: 'valid';
}
| {
state: 'invalid';
invalidNames: Set<string>;
Expand Down Expand Up @@ -1008,15 +1010,23 @@ class FeatureToggleService {
userId,
archived,
);
return { ...result, dependencies, children };
return {
...result,
dependencies,
children,
};
} else {
const result =
await this.featureStrategiesStore.getFeatureToggleWithEnvs(
featureName,
userId,
archived,
);
return { ...result, dependencies, children };
return {
...result,
dependencies,
children,
};
}
}

Expand Down Expand Up @@ -1227,7 +1237,10 @@ class FeatureToggleService {
);

if (result.state === 'invalid') {
return { ...result, featureNaming: patternData };
return {
...result,
featureNaming: patternData,
};
}
}
} catch (error) {
Expand Down Expand Up @@ -1341,7 +1354,11 @@ class FeatureToggleService {

const cloneDependencies =
this.dependentFeaturesService.cloneDependencies(
{ featureName, newFeatureName, projectId },
{
featureName,
newFeatureName,
projectId,
},
userName,
userId,
);
Expand All @@ -1362,7 +1379,10 @@ class FeatureToggleService {
featureName: string,
userId: number,
): Promise<FeatureToggle> {
await this.validateFeatureBelongsToProject({ featureName, projectId });
await this.validateFeatureBelongsToProject({
featureName,
projectId,
});

this.logger.info(`${userName} updates feature toggle ${featureName}`);

Expand Down Expand Up @@ -1901,7 +1921,11 @@ class FeatureToggleService {
const defaultEnv = environments.find((e) => e.name === DEFAULT_ENV);
const strategies = defaultEnv?.strategies || [];
const enabled = defaultEnv?.enabled || false;
return { ...legacyFeature, enabled, strategies };
return {
...legacyFeature,
enabled,
strategies,
};
}

async changeProject(
Expand Down Expand Up @@ -2271,7 +2295,9 @@ class FeatureToggleService {
): Promise<IVariant[]> {
await variantsArraySchema.validateAsync(newVariants);
const fixedVariants = this.fixVariantWeights(newVariants);
const oldVariants: { [env: string]: IVariant[] } = {};
const oldVariants: {
[env: string]: IVariant[];
} = {};
for (const env of environments) {
const featureEnv = await this.featureEnvironmentStore.get({
featureName,
Expand Down
3 changes: 2 additions & 1 deletion src/lib/features/playground/playground.ts
Expand Up @@ -21,6 +21,7 @@ import {
playgroundViewModel,
} from './playground-view-model';
import { IAuthRequest } from '../../routes/unleash-types';
import { extractUserIdFromUser } from '../../util';

export default class PlaygroundController extends Controller {
private openApiService: OpenApiService;
Expand Down Expand Up @@ -129,7 +130,7 @@ export default class PlaygroundController extends Controller {
req.body.environments,
req.body.context,
limit,
user.id,
extractUserIdFromUser(user),
);

const response: AdvancedPlaygroundResponseSchema =
Expand Down
4 changes: 2 additions & 2 deletions src/lib/features/private-project/privateProjectStore.ts
@@ -1,7 +1,7 @@
import { Db } from '../../db/db';
import { Logger, LogProvider } from '../../logger';
import { IPrivateProjectStore } from './privateProjectStoreType';
import { ADMIN_TOKEN_ID } from '../../types';
import { ADMIN_TOKEN_USER } from '../../types';

export type ProjectAccess =
| {
Expand Down Expand Up @@ -29,7 +29,7 @@ class PrivateProjectStore implements IPrivateProjectStore {
destroy(): void {}

async getUserAccessibleProjects(userId: number): Promise<ProjectAccess> {
if (userId === ADMIN_TOKEN_ID) {
if (userId === ADMIN_TOKEN_USER.id) {
return ALL_PROJECT_ACCESS;
}
const isViewer = await this.db('role_user')
Expand Down
4 changes: 2 additions & 2 deletions src/lib/features/segment/segment-controller.ts
Expand Up @@ -39,7 +39,7 @@ import {
SegmentsSchema,
} from '../../openapi/spec/segments-schema';

import { anonymiseKeys } from '../../util';
import { anonymiseKeys, extractUserIdFromUser } from '../../util';
import { BadDataError } from '../../error';

type IUpdateFeatureStrategySegmentsRequest = IAuthRequest<
Expand Down Expand Up @@ -351,7 +351,7 @@ export class SegmentsController extends Controller {
const { user } = req;
const strategies = await this.segmentService.getVisibleStrategies(
id,
user.id,
extractUserIdFromUser(user),
);

const segmentStrategies = strategies.strategies.map((strategy) => ({
Expand Down
7 changes: 5 additions & 2 deletions src/lib/routes/admin-api/context.ts
Expand Up @@ -2,7 +2,10 @@ import { Request, Response } from 'express';

import Controller from '../controller';

import { extractUsername } from '../../util/extract-user';
import {
extractUserIdFromUser,
extractUsername,
} from '../../util/extract-user';

import {
CREATE_CONTEXT_FIELD,
Expand Down Expand Up @@ -310,7 +313,7 @@ export class ContextController extends Controller {
const contextFields =
await this.contextService.getStrategiesByContextField(
contextField,
user.id,
extractUserIdFromUser(user),
);

this.openApiService.respondWithValidation(
Expand Down
3 changes: 2 additions & 1 deletion src/lib/routes/admin-api/metrics.ts
Expand Up @@ -15,6 +15,7 @@ import {
} from '../../openapi/util/standard-responses';
import { CreateApplicationSchema } from '../../openapi/spec/create-application-schema';
import { IAuthRequest } from '../unleash-types';
import { extractUserIdFromUser } from '../../util';

class MetricsController extends Controller {
private logger: Logger;
Expand Down Expand Up @@ -158,7 +159,7 @@ class MetricsController extends Controller {
: {};
const applications = await this.clientInstanceService.getApplications(
query,
user.id,
extractUserIdFromUser(user),
);
res.json({ applications });
}
Expand Down
4 changes: 1 addition & 3 deletions src/lib/types/no-auth-user.ts
@@ -1,6 +1,4 @@
import { ADMIN } from './permissions';

export const ADMIN_TOKEN_ID = -1;
export default class NoAuthUser {
isAPI: boolean;

Expand All @@ -12,7 +10,7 @@ export default class NoAuthUser {

constructor(
username: string = 'unknown',
id: number = ADMIN_TOKEN_ID,
id: number = -1,
permissions: string[] = [ADMIN],
) {
this.isAPI = true;
Expand Down

0 comments on commit 832884b

Please sign in to comment.