mirror of
https://github.com/Unleash/unleash.git
synced 2025-05-26 01:17:00 +02:00
feat: allow api token middleware to fetch from db (#6344)
## About the changes When edge is configured to automatically generate tokens, it requires the token to be present in all unleash instances. It's behind a flag which enables us to turn it on on a case by case scenario. The risk of this implementation is that we'd be adding load to the database in the middleware that evaluates tokens (which are present in mostly all our API calls. We only query when the token is missing but because the /client and /frontend endpoints which will be the affected ones are high throughput, we want to be extra careful to avoid DDoSing ourselves ## Alternatives: One alternative would be that we merge the two endpoints into one. Currently, Edge does the following: If the token is not valid, it tries to create a token using a service account token and /api/admin/create-token endpoint. Then it uses the token generated (which is returned from the prior endpoint) to query /api/frontend. What if we could call /api/frontend with the same service account we use to create the token? It may sound risky but if the same application holding the service account token with permission to create a token, can call /api/frontend via the generated token, shouldn't it be able to call the endpoint directly? The purpose of the token is authentication and authorization. With the two tokens we are authenticating the same app with 2 different authorization scopes, but because it's the same app we are authenticating, can't we just use one token and assume that the app has both scopes? If the service account already has permissions to create a token and then use that token for further actions, allowing it to directly call /api/frontend does not necessarily introduce new security risks. The only risk is allowing the app to generate new tokens. Which leads to the third alternative: should we just remove this option from edge?
This commit is contained in:
parent
b738a2a1bc
commit
70499dc1d4
@ -132,6 +132,7 @@ exports[`should create default config 1`] = `
|
|||||||
"newStrategyConfigurationFeedback": false,
|
"newStrategyConfigurationFeedback": false,
|
||||||
"personalAccessTokensKillSwitch": false,
|
"personalAccessTokensKillSwitch": false,
|
||||||
"proPlanAutoCharge": false,
|
"proPlanAutoCharge": false,
|
||||||
|
"queryMissingTokens": false,
|
||||||
"responseTimeWithAppNameKillSwitch": false,
|
"responseTimeWithAppNameKillSwitch": false,
|
||||||
"scimApi": false,
|
"scimApi": false,
|
||||||
"sdkReporting": false,
|
"sdkReporting": false,
|
||||||
|
@ -2,6 +2,7 @@ import { ApiTokenType } from '../types/models/api-token';
|
|||||||
import { IUnleashConfig } from '../types/option';
|
import { IUnleashConfig } from '../types/option';
|
||||||
import { IApiRequest, IAuthRequest } from '../routes/unleash-types';
|
import { IApiRequest, IAuthRequest } from '../routes/unleash-types';
|
||||||
import { IUnleashServices } from '../server-impl';
|
import { IUnleashServices } from '../server-impl';
|
||||||
|
import { IFlagContext } from '../types';
|
||||||
|
|
||||||
const isClientApi = ({ path }) => {
|
const isClientApi = ({ path }) => {
|
||||||
return path && path.indexOf('/api/client') > -1;
|
return path && path.indexOf('/api/client') > -1;
|
||||||
@ -26,6 +27,20 @@ const isProxyApi = ({ path }) => {
|
|||||||
);
|
);
|
||||||
};
|
};
|
||||||
|
|
||||||
|
const contextFrom = (
|
||||||
|
req: IAuthRequest<any, any, any, any> | IApiRequest<any, any, any, any>,
|
||||||
|
): IFlagContext | undefined => {
|
||||||
|
// this is what we'd get from edge:
|
||||||
|
// req_path: '/api/client/features',
|
||||||
|
// req_user_agent: 'unleash-edge-16.0.4'
|
||||||
|
return {
|
||||||
|
reqPath: req.path,
|
||||||
|
reqUserAgent: req.get ? req.get('User-Agent') ?? '' : '',
|
||||||
|
reqAppName:
|
||||||
|
req.headers?.['unleash-appname'] ?? req.query?.appName ?? '',
|
||||||
|
};
|
||||||
|
};
|
||||||
|
|
||||||
export const TOKEN_TYPE_ERROR_MESSAGE =
|
export const TOKEN_TYPE_ERROR_MESSAGE =
|
||||||
'invalid token: expected a different token type for this endpoint';
|
'invalid token: expected a different token type for this endpoint';
|
||||||
|
|
||||||
@ -46,7 +61,7 @@ const apiAccessMiddleware = (
|
|||||||
return (req, res, next) => next();
|
return (req, res, next) => next();
|
||||||
}
|
}
|
||||||
|
|
||||||
return (req: IAuthRequest | IApiRequest, res, next) => {
|
return async (req: IAuthRequest | IApiRequest, res, next) => {
|
||||||
if (req.user) {
|
if (req.user) {
|
||||||
return next();
|
return next();
|
||||||
}
|
}
|
||||||
@ -55,7 +70,10 @@ const apiAccessMiddleware = (
|
|||||||
const apiToken = req.header('authorization');
|
const apiToken = req.header('authorization');
|
||||||
if (!apiToken?.startsWith('user:')) {
|
if (!apiToken?.startsWith('user:')) {
|
||||||
const apiUser = apiToken
|
const apiUser = apiToken
|
||||||
? apiTokenService.getUserForToken(apiToken)
|
? await apiTokenService.getUserForToken(
|
||||||
|
apiToken,
|
||||||
|
contextFrom(req),
|
||||||
|
)
|
||||||
: undefined;
|
: undefined;
|
||||||
const { CLIENT, FRONTEND } = ApiTokenType;
|
const { CLIENT, FRONTEND } = ApiTokenType;
|
||||||
|
|
||||||
|
@ -1,6 +1,6 @@
|
|||||||
import { ApiTokenService } from './api-token-service';
|
import { ApiTokenService } from './api-token-service';
|
||||||
import { createTestConfig } from '../../test/config/test-config';
|
import { createTestConfig } from '../../test/config/test-config';
|
||||||
import { IUnleashConfig, IUser } from '../server-impl';
|
import { IUnleashConfig, IUnleashOptions, IUser } from '../server-impl';
|
||||||
import { ApiTokenType, IApiTokenCreate } from '../types/models/api-token';
|
import { ApiTokenType, IApiTokenCreate } from '../types/models/api-token';
|
||||||
import FakeApiTokenStore from '../../test/fixtures/fake-api-token-store';
|
import FakeApiTokenStore from '../../test/fixtures/fake-api-token-store';
|
||||||
import FakeEnvironmentStore from '../features/project-environments/fake-environment-store';
|
import FakeEnvironmentStore from '../features/project-environments/fake-environment-store';
|
||||||
@ -95,7 +95,7 @@ test("Shouldn't return frontend token when secret is undefined", async () => {
|
|||||||
await apiTokenService.createApiTokenWithProjects(token);
|
await apiTokenService.createApiTokenWithProjects(token);
|
||||||
await apiTokenService.fetchActiveTokens();
|
await apiTokenService.fetchActiveTokens();
|
||||||
|
|
||||||
expect(apiTokenService.getUserForToken('')).toEqual(undefined);
|
expect(await apiTokenService.getUserForToken('')).toEqual(undefined);
|
||||||
});
|
});
|
||||||
|
|
||||||
test('Api token operations should all have events attached', async () => {
|
test('Api token operations should all have events attached', async () => {
|
||||||
@ -185,8 +185,62 @@ test('getUserForToken should get a user with admin token user id and token name'
|
|||||||
ADMIN_TOKEN_USER as IUser,
|
ADMIN_TOKEN_USER as IUser,
|
||||||
);
|
);
|
||||||
|
|
||||||
const user = tokenService.getUserForToken(token.secret);
|
const user = await tokenService.getUserForToken(token.secret);
|
||||||
expect(user).toBeDefined();
|
expect(user).toBeDefined();
|
||||||
expect(user!.username).toBe(token.tokenName);
|
expect(user!.username).toBe(token.tokenName);
|
||||||
expect(user!.internalAdminTokenUserId).toBe(ADMIN_TOKEN_USER.id);
|
expect(user!.internalAdminTokenUserId).toBe(ADMIN_TOKEN_USER.id);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('When token is added by another instance', () => {
|
||||||
|
const setup = (options?: IUnleashOptions) => {
|
||||||
|
const token: IApiTokenCreate = {
|
||||||
|
environment: 'default',
|
||||||
|
projects: ['*'],
|
||||||
|
secret: '*:*:some-random-string',
|
||||||
|
type: ApiTokenType.CLIENT,
|
||||||
|
tokenName: 'new-token-by-another-instance',
|
||||||
|
expiresAt: undefined,
|
||||||
|
};
|
||||||
|
|
||||||
|
const config: IUnleashConfig = createTestConfig(options);
|
||||||
|
const apiTokenStore = new FakeApiTokenStore();
|
||||||
|
const environmentStore = new FakeEnvironmentStore();
|
||||||
|
|
||||||
|
const apiTokenService = new ApiTokenService(
|
||||||
|
{ apiTokenStore, environmentStore },
|
||||||
|
config,
|
||||||
|
createFakeEventsService(config),
|
||||||
|
);
|
||||||
|
return {
|
||||||
|
apiTokenService,
|
||||||
|
apiTokenStore,
|
||||||
|
token,
|
||||||
|
};
|
||||||
|
};
|
||||||
|
test('should not return the token when query db flag is disabled', async () => {
|
||||||
|
const { apiTokenService, apiTokenStore, token } = setup();
|
||||||
|
|
||||||
|
// simulate this token being inserted by another instance (apiTokenService does not know about it)
|
||||||
|
apiTokenStore.insert(token);
|
||||||
|
|
||||||
|
const found = await apiTokenService.getUserForToken(token.secret);
|
||||||
|
expect(found).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
test('should return the token when query db flag is enabled', async () => {
|
||||||
|
const { apiTokenService, apiTokenStore, token } = setup({
|
||||||
|
experimental: {
|
||||||
|
flags: {
|
||||||
|
queryMissingTokens: true,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
// simulate this token being inserted by another instance (apiTokenService does not know about it)
|
||||||
|
apiTokenStore.insert(token);
|
||||||
|
|
||||||
|
const found = await apiTokenService.getUserForToken(token.secret);
|
||||||
|
expect(found).toBeDefined();
|
||||||
|
expect(found?.username).toBe(token.tokenName);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
@ -24,6 +24,7 @@ import {
|
|||||||
ApiTokenCreatedEvent,
|
ApiTokenCreatedEvent,
|
||||||
ApiTokenDeletedEvent,
|
ApiTokenDeletedEvent,
|
||||||
ApiTokenUpdatedEvent,
|
ApiTokenUpdatedEvent,
|
||||||
|
IFlagContext,
|
||||||
IFlagResolver,
|
IFlagResolver,
|
||||||
IUser,
|
IUser,
|
||||||
SYSTEM_USER,
|
SYSTEM_USER,
|
||||||
@ -160,7 +161,10 @@ export class ApiTokenService {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
public getUserForToken(secret: string): IApiUser | undefined {
|
public async getUserForToken(
|
||||||
|
secret: string,
|
||||||
|
flagContext?: IFlagContext, // temporarily added, expected from the middleware
|
||||||
|
): Promise<IApiUser | undefined> {
|
||||||
if (!secret) {
|
if (!secret) {
|
||||||
return undefined;
|
return undefined;
|
||||||
}
|
}
|
||||||
@ -181,6 +185,16 @@ export class ApiTokenService {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (
|
||||||
|
!token &&
|
||||||
|
this.flagResolver.isEnabled('queryMissingTokens', flagContext)
|
||||||
|
) {
|
||||||
|
token = await this.store.get(secret);
|
||||||
|
if (token) {
|
||||||
|
this.activeTokens.push(token);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
if (token) {
|
if (token) {
|
||||||
this.lastSeenSecrets.add(token.secret);
|
this.lastSeenSecrets.add(token.secret);
|
||||||
const apiUser: IApiUser = new ApiUser({
|
const apiUser: IApiUser = new ApiUser({
|
||||||
|
@ -47,6 +47,7 @@ export type IFlagKey =
|
|||||||
| 'inMemoryScheduledChangeRequests'
|
| 'inMemoryScheduledChangeRequests'
|
||||||
| 'collectTrafficDataUsage'
|
| 'collectTrafficDataUsage'
|
||||||
| 'useMemoizedActiveTokens'
|
| 'useMemoizedActiveTokens'
|
||||||
|
| 'queryMissingTokens'
|
||||||
| 'userAccessUIEnabled'
|
| 'userAccessUIEnabled'
|
||||||
| 'disableUpdateMaxRevisionId'
|
| 'disableUpdateMaxRevisionId'
|
||||||
| 'disablePublishUnannouncedEvents'
|
| 'disablePublishUnannouncedEvents'
|
||||||
@ -248,6 +249,10 @@ const flags: IFlags = {
|
|||||||
process.env.UNLEASH_EXPERIMENTAL_DISABLE_SCHEDULED_CACHES,
|
process.env.UNLEASH_EXPERIMENTAL_DISABLE_SCHEDULED_CACHES,
|
||||||
false,
|
false,
|
||||||
),
|
),
|
||||||
|
queryMissingTokens: parseEnvVarBoolean(
|
||||||
|
process.env.UNLEASH_EXPERIMENTAL_QUERY_MISSING_TOKENS,
|
||||||
|
false,
|
||||||
|
),
|
||||||
scimApi: parseEnvVarBoolean(
|
scimApi: parseEnvVarBoolean(
|
||||||
process.env.UNLEASH_EXPERIMENTAL_SCIM_API,
|
process.env.UNLEASH_EXPERIMENTAL_SCIM_API,
|
||||||
false,
|
false,
|
||||||
|
@ -200,8 +200,8 @@ test('should return user with multiple projects', async () => {
|
|||||||
environment: DEFAULT_ENV,
|
environment: DEFAULT_ENV,
|
||||||
});
|
});
|
||||||
|
|
||||||
const multiProjectUser = apiTokenService.getUserForToken(secret1);
|
const multiProjectUser = await apiTokenService.getUserForToken(secret1);
|
||||||
const singleProjectUser = apiTokenService.getUserForToken(secret2);
|
const singleProjectUser = await apiTokenService.getUserForToken(secret2);
|
||||||
|
|
||||||
expect(multiProjectUser!.projects).toStrictEqual([
|
expect(multiProjectUser!.projects).toStrictEqual([
|
||||||
'test-project',
|
'test-project',
|
||||||
|
@ -42,7 +42,7 @@ The client needs to point to the correct API endpoint. The front-end API is avai
|
|||||||
|
|
||||||
### API token
|
### API token
|
||||||
|
|
||||||
You can create appropriate token, with type `FRONTEND` on `<YOUR_UNLEASH_URL>/admin/api/create-token` page or with a request to `/api/admin/api-tokens`. See our guide on [how to create API tokens](../how-to/how-to-create-api-tokens.mdx) for more details.
|
You can create appropriate token, with type `FRONTEND` on `<YOUR_UNLEASH_URL>/api/admin/create-token` page or with a request to `/api/admin/api-tokens`. See our guide on [how to create API tokens](../how-to/how-to-create-api-tokens.mdx) for more details.
|
||||||
|
|
||||||
### Refresh interval for tokens
|
### Refresh interval for tokens
|
||||||
|
|
||||||
|
Loading…
Reference in New Issue
Block a user