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.

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

Feat/allow admin login using demo auth #6808

Merged
merged 5 commits into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion frontend/src/component/user/DemoAuth/DemoAuth.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ const DemoAuth: VFC<IDemoAuthProps> = ({ authDetails, redirect }) => {
id='email'
data-testid={LOGIN_EMAIL_ID}
required
type='email'
type={email === 'admin' ? 'text' : 'email'}
/>

<Button
Expand Down
1 change: 1 addition & 0 deletions src/lib/__snapshots__/create-config.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ exports[`should create default config 1`] = `
"authentication": {
"createAdminUser": true,
"customAuthHandler": [Function],
"demoAllowAdminLogin": false,
"enableApiToken": true,
"initApiTokens": [],
"initialAdminUser": {
Expand Down
5 changes: 5 additions & 0 deletions src/lib/create-config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,11 @@ test('should handle cases where no env var specified for tokens', async () => {
expect(config.authentication.initApiTokens).toHaveLength(1);
});

test('should default demo admin login to false', async () => {
const config = createConfig({});
expect(config.authentication.demoAllowAdminLogin).toBeFalsy();
});

test('should load environment overrides from env var', async () => {
process.env.ENABLED_ENVIRONMENTS = 'default,production';

Expand Down
4 changes: 4 additions & 0 deletions src/lib/create-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,10 @@ const defaultVersionOption: IVersionOption = {
};

const defaultAuthentication: IAuthOption = {
demoAllowAdminLogin: parseEnvVarBoolean(
process.env.AUTH_DEMO_ALLOW_ADMIN_LOGIN,
false,
),
enableApiToken: parseEnvVarBoolean(process.env.AUTH_ENABLE_API_TOKEN, true),
type: authTypeFromString(process.env.AUTH_TYPE),
customAuthHandler: defaultCustomAuthDenyAll,
Expand Down
73 changes: 73 additions & 0 deletions src/lib/middleware/demo-authentication.e2e.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import dbInit from '../../test/e2e/helpers/database-init';
import { IAuthType } from '../server-impl';
import { setupAppWithCustomAuth } from '../../test/e2e/helpers/test-helper';
import type { ITestDb } from '../../test/e2e/helpers/database-init';
import type { IUnleashStores } from '../types';

let db: ITestDb;
let stores: IUnleashStores;

beforeAll(async () => {
db = await dbInit('demo_auth_serial');
stores = db.stores;
});

afterAll(async () => {
await db?.destroy();
});

const getApp = (adminLoginEnabled: boolean) =>
setupAppWithCustomAuth(stores, () => {}, {
authentication: {
demoAllowAdminLogin: adminLoginEnabled,
type: IAuthType.DEMO,
createAdminUser: true,
},
});

test('the demoAllowAdminLogin flag should not affect regular user login/creation', async () => {
const app = await getApp(true);
return app.request
.post(`/auth/demo/login`)
.send({ email: '[email protected]' })
.expect(200)
.expect((res) => {
expect(res.body.email).toBe('[email protected]');
expect(res.body.id).not.toBe(1);
});
});

test('if the demoAllowAdminLogin flag is disabled, using `admin` should have the same result as any other invalid email', async () => {
const app = await getApp(false);

const nonAdminUsername = 'not-an-email';
const adminUsername = 'admin';

const nonAdminUser = await app.request
.post(`/auth/demo/login`)
.send({ email: nonAdminUsername });

const adminUser = await app.request
.post(`/auth/demo/login`)
.send({ email: adminUsername });

expect(nonAdminUser.status).toBe(adminUser.status);

for (const user of [nonAdminUser, adminUser]) {
expect(user.body).toMatchObject({
error: expect.stringMatching(/^Could not sign in with /),
});
}
});

test('should allow you to login as admin if the demoAllowAdminLogin flag enabled', async () => {
const app = await getApp(true);
return app.request
.post(`/auth/demo/login`)
.send({ email: 'admin' })
.expect(200)
.expect((res) => {
expect(res.body.id).toBe(1);
expect(res.body.username).toBe('admin');
});
});
23 changes: 14 additions & 9 deletions src/lib/middleware/demo-authentication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import type { IUnleashConfig } from '../types/option';
import ApiUser from '../types/api-user';
import { ApiTokenType } from '../types/models/api-token';
import type { IAuthRequest } from '../server-impl';
import type { IAuthRequest, IUser } from '../server-impl';
import type { IApiRequest } from '../routes/unleash-types';
import { encrypt } from '../util';

Expand All @@ -17,27 +17,32 @@
flagResolver,
}: Pick<IUnleashConfig, 'authentication' | 'flagResolver'>,
): void {
app.post(`${basePath}/auth/demo/login`, async (req: IAuthRequest, res) => {
let { email } = req.body;
email = flagResolver.isEnabled('encryptEmails', { email })
? encrypt(email)
: email;
let user: IUser;

Comment on lines +22 to +23

Choose a reason for hiding this comment

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

❌ New issue: Complex Method
demoAuthentication has a cyclomatic complexity of 12, threshold = 9

Suppress

try {
const user = await userService.loginUserWithoutPassword(
email,
true,
);
if (authentication.demoAllowAdminLogin && email === 'admin') {
user = await userService.loginDemoAuthDefaultAdmin();
} else {
email = flagResolver.isEnabled('encryptEmails', { email })
? encrypt(email)
: email;

user = await userService.loginUserWithoutPassword(email, true);
}

req.session.user = user;
return res.status(200).json(user);
} catch (e) {
res.status(400)
.json({ error: `Could not sign in with ${email}` })
.end();
}
});

Check failure

Code scanning / CodeQL

Missing rate limiting High

This route handler performs
authorization
, but is not rate-limited.
This route handler performs
authorization
, but is not rate-limited.

app.use(`${basePath}/api/admin/`, (req: IAuthRequest, res, next) => {
if (req.session.user?.email) {
if (req.session.user?.email || req.session.user?.username === 'admin') {
req.user = req.session.user;
}
next();
Expand Down
6 changes: 6 additions & 0 deletions src/lib/services/user-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,12 @@ class UserService {
return user;
}

async loginDemoAuthDefaultAdmin(): Promise<IUser> {
const user = await this.store.getByQuery({ id: 1 });
await this.store.successfullyLogin(user);
return user;
}

async changePassword(userId: number, password: string): Promise<void> {
this.validatePassword(password);
const passwordHash = await bcrypt.hash(password, saltRounds);
Expand Down
1 change: 1 addition & 0 deletions src/lib/types/option.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ export type CustomAuthHandler = (
) => void;

export interface IAuthOption {
demoAllowAdminLogin?: boolean;
enableApiToken: boolean;
type: IAuthType;
customAuthHandler?: CustomAuthHandler;
Expand Down