Skip to content
This repository was archived by the owner on Mar 25, 2021. It is now read-only.

Commit 03901fc

Browse files
nictownsendnictownsend
committed
fix: render index for unknown requests
- always return index if a path cannot be met - security tests to ensure index is protected Contributes to: #154 Signed-off-by: Nic Townsend <[email protected]>
1 parent 8d00115 commit 03901fc

File tree

3 files changed

+88
-49
lines changed

3 files changed

+88
-49
lines changed

‎server/client/client.feature

Lines changed: 68 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -4,45 +4,75 @@ Feature: client module
44

55
Behaviours and capabilities provided by the client module
66

7-
Scenario Outline: If no <Asset> asset can be served, the client module returns 404
8-
Given a 'client_only' server configuration
9-
And There are no files to serve
10-
And Authentication is required
11-
And I run an instance of the Strimzi-UI server
12-
When I make a 'get' request to '<Asset>'
13-
Then I get the expected status code '<StatusCode>' response
7+
Scenario Outline: With auth '<Auth>' - If no <Asset> asset can be served, the client module returns 404
8+
Given a 'client_only' server configuration
9+
And There are no files to serve
10+
And '<Auth>' authentication is required
11+
And I run an instance of the Strimzi-UI server
12+
When I make a 'get' request to '<Asset>'
13+
Then I get the expected status code '<StatusCode>' response
1414

15-
Examples:
16-
| Asset | StatusCode |
17-
| /index.html | 404 |
18-
| /images/picture.svg | 404 |
19-
| /doesnotexist.html | 404 |
20-
| /someroute | 404 |
21-
| /protected.html | 404 |
22-
| / | 404 |
15+
Examples:
16+
| Asset | Auth | StatusCode |
17+
| /index.html | scram | 404 |
18+
| /images/picture.svg | scram | 404 |
19+
| /doesnotexist.html | scram | 404 |
20+
| /someroute | scram | 404 |
21+
| /protected.html | scram | 404 |
22+
| / | scram | 404 |
23+
| /index.html | oauth | 404 |
24+
| /images/picture.svg | oauth | 404 |
25+
| /doesnotexist.html | oauth | 404 |
26+
| /someroute | oauth | 404 |
27+
| /protected.html | oauth | 404 |
28+
| / | oauth | 404 |
29+
| /index.html | none | 404 |
30+
| /images/picture.svg | none | 404 |
31+
| /doesnotexist.html | none | 404 |
32+
| /someroute | none | 404 |
33+
| /protected.html | none | 404 |
34+
| / | none | 404 |
2335

24-
Scenario Outline: If assets can be served, the client module returns the appropriate <StatusCode> return code for a request of <Asset>
25-
Given a 'client_only' server configuration
26-
And There are files to serve
27-
And Authentication is required
28-
And I run an instance of the Strimzi-UI server
29-
When I make a 'get' request to '<Asset>'
30-
Then I get the expected status code '<StatusCode>' response
31-
# if the route (not file) is not matched, we redirect to index.html. Hence / and someroute response
32-
Examples:
33-
| Asset | StatusCode |
34-
| /index.html | 200 |
35-
| /images/picture.svg | 200 |
36-
| /doesnotexist.html | 404 |
37-
| /someroute | 302 |
38-
| /protected.html | 511 |
39-
| / | 200 |
36+
Scenario Outline: With auth '<Auth>' - if assets can be served, the client module returns the appropriate <StatusCode> return code for a request of <Asset>
37+
Given a 'client_only' server configuration
38+
And There are files to serve
39+
And '<Auth>' authentication is required
40+
And I run an instance of the Strimzi-UI server
41+
When I make a 'get' request to '<Asset>'
42+
Then I get the expected status code '<StatusCode>' response
43+
# if the route (not file) is not matched, we render index.html as the repsonse (200)
44+
Examples:
45+
| Asset | Auth | StatusCode |
46+
| /index.html | scram | 511 |
47+
| /images/picture.svg | scram | 200 |
48+
| /doesnotexist.html | scram | 511 |
49+
| /someroute | scram | 511 |
50+
| /protected.html | scram | 511 |
51+
| / | scram | 511 |
52+
| /index.html | oauth | 511 |
53+
| /images/picture.svg | oauth | 200 |
54+
| /doesnotexist.html | oauth | 511 |
55+
| /someroute | oauth | 511 |
56+
| /protected.html | oauth | 511 |
57+
| / | oauth | 511 |
58+
| /index.html | none | 200 |
59+
| /images/picture.svg | none | 200 |
60+
| /doesnotexist.html | none | 200 |
61+
| /someroute | none | 200 |
62+
| /protected.html | none | 200 |
63+
| / | none | 200 |
4064

4165

42-
Scenario: Critical configuration is templated into index.html so the client can bootstrap
43-
Given a 'client_only' server configuration
44-
And There are files to serve
45-
And Authentication is required
46-
And I run an instance of the Strimzi-UI server
47-
When I make a 'get' request to '/index.html'
48-
Then the file is returned as with the expected configuration included
66+
Scenario Outline: With auth '<Auth>' - Critical configuration is templated into index.html so the client can bootstrap
67+
Given a 'client_only' server configuration
68+
And There are files to serve
69+
And '<Auth>' authentication is required
70+
And I am authenticated
71+
And I run an instance of the Strimzi-UI server
72+
When I make a 'get' request to '/index.html'
73+
Then the file is returned as with the expected configuration included
74+
Examples:
75+
| Auth |
76+
| scram |
77+
| oauth |
78+
| none |

‎server/client/router.ts

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,22 +32,19 @@ export const ClientModule: UIServerModule = {
3232
// add the auth middleware to all non public files
3333
protectedFiles.forEach((file) => routerForModule.get(`${file}`, authFn));
3434

35-
// return index.html, with configuration templated in
35+
// Direct request for index, serve it (behind auth check)
3636
hasIndexFile &&
37-
routerForModule.get('/index.html', renderTemplate(indexFile));
37+
routerForModule.get('/index.html', authFn, renderTemplate(indexFile));
3838

3939
// host all files from the client dir
4040
routerForModule.get(
4141
'*',
42-
expressStaticGzip(builtClientDir, {}),
42+
expressStaticGzip(builtClientDir, { index: false }),
4343
express.static(builtClientDir, { index: false })
4444
);
4545

46-
// if no match, not a file (path contains '.'), and we have an index.html file, redirect to it (ie return index so client navigation logic kicks in). Else do nothing (404 unless another module handles it)
47-
hasIndexFile &&
48-
routerForModule.get(/^((?!\.).)+$/, (req, res) =>
49-
res.redirect(`/index.html`)
50-
);
46+
// If no match and we have an index, serve it (behind auth check)
47+
hasIndexFile && routerForModule.get('*', authFn, renderTemplate(indexFile));
5148

5249
return exit({ mountPoint: '/', routerForModule });
5350
},

‎server/test_common/commonServerSteps.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
*/
55
import request from 'supertest';
66
import { returnExpress } from 'core';
7+
import { mocked } from 'ts-jest/utils';
78
import { Given, When, And, Then } from 'jest-cucumber-fusion';
89
import {
910
serverConfigType,
@@ -21,6 +22,8 @@ import { requests } from './testGQLRequests';
2122

2223
import MockedWebSocket from 'ws';
2324
jest.mock('ws');
25+
jest.mock('../placeholderFunctionsToReplace');
26+
import { authFunction } from '../placeholderFunctionsToReplace';
2427

2528
type supertestRequestType = request.SuperTest<request.Test>;
2629

@@ -47,6 +50,8 @@ const { resetWorld, stepWhichUpdatesWorld, stepWithWorld } = worldGenerator(
4750

4851
beforeEach(() => {
4952
resetWorld();
53+
jest.resetAllMocks();
54+
mocked(authFunction).mockReturnValue((_req, _res, next) => next());
5055
});
5156

5257
Given(
@@ -60,13 +65,16 @@ Given(
6065
);
6166

6267
And(
63-
'Authentication is required',
64-
stepWhichUpdatesWorld((world) => {
68+
/^'(\S+)' authentication is required$/,
69+
stepWhichUpdatesWorld((world, auth) => {
70+
if ((auth as string) !== 'none') {
71+
mocked(authFunction).mockReturnValue((_req, res) => res.sendStatus(511));
72+
}
6573
const { configuration } = world;
6674
return {
6775
...world,
6876
configuration: merge(configuration, {
69-
authentication: { strategy: 'oauth' },
77+
authentication: { strategy: auth },
7078
}),
7179
};
7280
})
@@ -84,6 +92,10 @@ And(
8492
})
8593
);
8694

95+
And('I am authenticated', () => {
96+
mocked(authFunction).mockReturnValue((_req, _res, next) => next());
97+
});
98+
8799
And(
88100
'I enable WebSocket connections on the Strimzi-UI server',
89101
stepWhichUpdatesWorld((world) => {

0 commit comments

Comments
 (0)