Skip to content

Commit bab3d79

Browse files
authored
add permission check for post endpoint (#597)
* add permission check for post endpoint * update linter ruleset to allow access_token
1 parent 39d154c commit bab3d79

File tree

5 files changed

+85
-65
lines changed

5 files changed

+85
-65
lines changed

.eslintrc.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ module.exports = {
99
'import/no-dynamic-require': 'off',
1010
'global-require': 'off',
1111
'react/jsx-curly-brace-presence': 'off',
12-
'react/no-unused-prop-types': 'off'
12+
'react/no-unused-prop-types': 'off',
13+
'camelcase': ['error', { 'allow': ['access_token', 'auth_token'] }]
1314
}
1415
};

server.js

Lines changed: 10 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ const url = require('url');
44
const fs = require('fs');
55
const asciidoctor = require('asciidoctor.js');
66
const Mustache = require('mustache');
7-
const { fetchOpenshiftUser } = require('./server_middleware');
7+
const { requireRoles } = require('./server_middleware');
88
const giteaClient = require('./gitea_client');
99
const gitClient = require('./git_client');
1010
const bodyParser = require('body-parser');
@@ -70,6 +70,12 @@ const WALKTHROUGH_LOCATION_DEFAULT = {
7070
header: null
7171
};
7272

73+
const backendRequiredRoles = [
74+
'system:cluster-admins',
75+
'system:dedicated-admins',
76+
'dedicated-admins'
77+
];
78+
7379
const walkthroughs = [];
7480
let server;
7581

@@ -83,6 +89,7 @@ app.get('/metrics', (req, res) => {
8389
res.end(Prometheus.register.metrics());
8490
});
8591

92+
8693
// Get all user defined walkthrough repositories
8794
app.get('/user_walkthroughs', (req, res) =>
8895
getUserWalkthroughs()
@@ -101,7 +108,8 @@ app.get('/user_walkthroughs', (req, res) =>
101108
);
102109

103110
// Insert new user defined walkthrough repositories
104-
app.post('/user_walkthroughs', (req, res) => {
111+
// This requires cluster- or dedicated admin permissions
112+
app.post('/user_walkthroughs', requireRoles(backendRequiredRoles), (req, res) => {
105113
const { data } = req.body;
106114
return setUserWalkthroughs(data)
107115
.then(({ value }) => res.json(value))
@@ -111,39 +119,6 @@ app.post('/user_walkthroughs', (req, res) => {
111119
});
112120
});
113121

114-
// Init custom walkthroughs dependencies
115-
app.post('/initThread', fetchOpenshiftUser, (req, res) => {
116-
if (!req.body || !req.body.dependencies) {
117-
console.warn('Dependencies not provided in request body. Skipping thread initialization.');
118-
res.sendStatus(200);
119-
return;
120-
}
121-
const {
122-
dependencies: { repos },
123-
openshiftUser
124-
} = req.body;
125-
126-
// Return success in mock mode without actually creating any repositories
127-
if (!process.env.OPENSHIFT_HOST) {
128-
console.warn('OPENSHIFT_HOST not set. Skipping thread initialization.');
129-
res.sendStatus(200);
130-
return;
131-
}
132-
133-
if (!repos || repos.length === 0) {
134-
res.sendStatus(200);
135-
return;
136-
}
137-
138-
// eslint-disable-next-line consistent-return
139-
return Promise.all(repos.map(repo => giteaClient.createRepoForUser(openshiftUser, repo)))
140-
.then(() => res.sendStatus(200))
141-
.catch(err => {
142-
console.error(`Error creating repositories: ${err}`);
143-
return res.status(500).json({ error: err.message });
144-
});
145-
});
146-
147122
// Dynamic configuration for openshift API calls
148123
app.get('/config.js', (req, res) => {
149124
if (!process.env.OPENSHIFT_HOST) {

server_middleware.js

Lines changed: 62 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -7,36 +7,73 @@ const insecureAgent = new https.Agent({
77
rejectUnauthorized: false
88
});
99

10-
exports.fetchOpenshiftUser = (req, res, next) => {
11-
// Not much we can do if the openshift host is not set. This probably
12-
// means we are running in a mock environment
13-
if (!OPENSHIFT_HOST) {
14-
return next();
10+
11+
function getApiHost() {
12+
if (process.env.OPENSHIFT_VERSION === '4') {
13+
return `https://${process.env.OPENSHIFT_API}/apis/user.openshift.io/v1/users/~`;
14+
}
15+
return `https://${OPENSHIFT_HOST}/oapi/v1/users/~`;
16+
}
17+
18+
function checkRoles(memberGroups, requestedGroups) {
19+
// If no groups were requested the request is always authorized
20+
if (!requestedGroups) {
21+
return true;
1522
}
1623

17-
const token = req.get('X-Forwarded-Access-Token');
18-
if (!token) {
19-
return res.sendStatus(403);
24+
// If groups were requested but the user is not a member of any group the request is
25+
// always unauthorized
26+
if (!memberGroups) {
27+
return false;
2028
}
2129

22-
return axios({
23-
httpsAgent: insecureAgent,
24-
url: `https://${OPENSHIFT_HOST}/oapi/v1/users/~`,
25-
headers: {
26-
authorization: `Bearer ${token}`,
27-
Accept: 'application/json'
30+
// Require that at least one of the requested groups are part of the members group array
31+
return requestedGroups.map(group => {
32+
console.log(`checking against ${group}`);
33+
return memberGroups.indexOf(group) >= 0;
34+
}).reduce((acc, current) => {
35+
return acc || current;
36+
}, false);
37+
}
38+
39+
exports.requireRoles = config => {
40+
return function(req, res, next) {
41+
// Not much we can do if the openshift host is not set. This probably
42+
// means we are running in a mock environment
43+
if (!OPENSHIFT_HOST) {
44+
return next();
2845
}
29-
})
30-
.then(response => {
31-
const {
32-
kind,
33-
metadata: { name }
34-
} = response.data;
35-
if (kind === 'User') {
36-
req.body.openshiftUser = name;
37-
return next();
46+
47+
const token = req.get('X-Forwarded-Access-Token');
48+
if (!token) {
49+
return res.sendStatus(401);
50+
}
51+
52+
return axios({
53+
httpsAgent: insecureAgent,
54+
url: getApiHost(),
55+
headers: {
56+
authorization: `Bearer ${token}`
3857
}
39-
return res.sendStatus(403);
4058
})
41-
.catch(err => next(err));
59+
.then(response => {
60+
const {
61+
kind,
62+
groups,
63+
metadata: { name }
64+
} = response.data;
65+
if (kind === 'User') {
66+
if (checkRoles(groups, config)) {
67+
console.error(`Access granted to user ${name}`);
68+
return next();
69+
}
70+
console.error(`Access denied to user ${name}`);
71+
}
72+
return res.sendStatus(403);
73+
})
74+
.catch(err => {
75+
console.error(err);
76+
return res.sendStatus(401);
77+
});
78+
};
4279
};

src/pages/settings/settings.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import { connect, reduxActions } from '../../redux';
3737
import Breadcrumb from '../../components/breadcrumb/breadcrumb';
3838
import { setUserWalkthroughs, getUserWalkthroughs } from '../../services/walkthroughServices';
3939
import { getCurrentRhmiConfig, updateRhmiConfig } from '../../services/rhmiConfigServices';
40+
import { getUser } from '../../services/openshiftServices';
4041

4142
const moment = require('moment');
4243

@@ -145,8 +146,11 @@ class SettingsPage extends React.Component {
145146
saveSolutionPatternSettings = (e, value) => {
146147
e.preventDefault();
147148
const { history } = this.props;
148-
setUserWalkthroughs(value);
149-
history.push(`/`);
149+
getUser().then(({ access_token }) => {
150+
setUserWalkthroughs(value, access_token).then(() => {
151+
history.push(`/`);
152+
});
153+
});
150154
};
151155

152156
saveMockBackupSettings = (e, value) => {

src/services/walkthroughServices.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -373,13 +373,16 @@ const getUserWalkthroughs = () =>
373373
/**
374374
* Saves the user-defined GitHub repositories from the UI to the database.
375375
*/
376-
const setUserWalkthroughs = (data = {}) =>
376+
const setUserWalkthroughs = (data = {}, token) =>
377377
axios(
378378
serviceConfig(
379379
{
380380
method: 'post',
381381
url: `/user_walkthroughs`,
382-
data: { data }
382+
data: { data },
383+
headers: {
384+
'X-Forwarded-Access-Token': token
385+
}
383386
},
384387
false
385388
)

0 commit comments

Comments
 (0)