Skip to content

Commit 713fc1f

Browse files
matthewpeterkortmatthewpeterkort
committed
Addresses PR feedback
1 parent cca5950 commit 713fc1f

File tree

5 files changed

+43
-104
lines changed

5 files changed

+43
-104
lines changed

‎src/server/auth/__tests__/authHelper.test.js

+1-2
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,7 @@ setupMockDataEndpoint();
1212
describe('AuthHelper', () => {
1313
test('could create auth helper instance', async () => {
1414
const authHelper = await getAuthHelperInstance('fake-jwt');
15-
expect(authHelper.getAccessibleCreateResources()).toEqual(['internal-project-1', 'internal-project-3', 'internal-project-4', 'internal-project-5']);
16-
expect(authHelper.getAccessibleCreateResources()).not.toContain(['internal-project-2', 'internal-project-6']);
15+
expect(authHelper.getCanRefresh()).toEqual(false);
1716
expect(authHelper.getAccessibleResources()).toEqual(['internal-project-1', 'internal-project-2', 'internal-project-4', 'internal-project-5']);
1817
expect(authHelper.getAccessibleResources()).not.toContain(['internal-project-3', 'internal-project-6']);
1918
expect(authHelper.getUnaccessibleResources()).toEqual(['external-project-1', 'external-project-2']);

‎src/server/auth/arboristClient.js

+2-83
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@ class ArboristClient {
88
this.baseEndpoint = arboristEndpoint;
99
}
1010

11-
listAuthorizedResources(jwt) {
11+
listAuthMapping(jwt) {
1212
// Make request to arborist for list of resources with access
1313
const resourcesEndpoint = `${this.baseEndpoint}/auth/mapping`;
14-
log.debug('[ArboristClient] listAuthorizedResources jwt: ', jwt);
14+
log.debug('[ArboristClient] listAuthMapping jwt: ', jwt);
1515

1616
const headers = (jwt) ? { Authorization: `bearer ${jwt}` } : {};
1717
return fetch(
@@ -40,87 +40,6 @@ class ArboristClient {
4040
log.error(err);
4141
throw new CodedError(500, err);
4242
},
43-
).then(
44-
(result) => {
45-
const data = {
46-
resources: [],
47-
};
48-
Object.keys(result).forEach((key) => {
49-
// logic: you have access to a project if you have the following access:
50-
// method 'read' (or '*' - all methods) to service 'guppy' (or '*' - all services)
51-
// on the project resource.
52-
if (result[key] && result[key].some((x) => (
53-
(x.method === 'read' || x.method === '*')
54-
&& (x.service === 'guppy' || x.service === '*')
55-
))) {
56-
data.resources.push(key);
57-
}
58-
});
59-
log.debug('[ArboristClient] data: ', data);
60-
return data;
61-
},
62-
(err) => {
63-
log.error(err);
64-
throw new CodedError(500, err);
65-
},
66-
);
67-
}
68-
69-
listAuthorizedCreateResources(jwt) {
70-
// Make request to arborist for list of resources with access
71-
const resourcesEndpoint = `${this.baseEndpoint}/auth/mapping`;
72-
log.debug('[ArboristClient] listAuthorizedCreateResources jwt: ', jwt);
73-
74-
const headers = (jwt) ? { Authorization: `bearer ${jwt}` } : {};
75-
return fetch(
76-
resourcesEndpoint,
77-
{
78-
method: 'POST',
79-
headers,
80-
},
81-
).then(
82-
(response) => {
83-
if (response.status === 400) {
84-
// Retry with GET instead of POST. Older version of Arborist POST auth/mapping
85-
// didn't support token authentication.
86-
// This catch block can be removed in a little while, when it will likely not cause issues
87-
return fetch(
88-
resourcesEndpoint,
89-
{
90-
method: 'GET',
91-
headers,
92-
},
93-
).then((res) => res.json());
94-
}
95-
return response.json();
96-
},
97-
(err) => {
98-
log.error(err);
99-
throw new CodedError(500, err);
100-
},
101-
).then(
102-
(result) => {
103-
const data = {
104-
resources: [],
105-
};
106-
Object.keys(result).forEach((key) => {
107-
// logic: you have access to a project if you have the following access:
108-
// method 'create' (or '*' - all methods) to service 'guppy' (or '*' - all services)
109-
// on the project resource.
110-
if (result[key] && result[key].some((x) => (
111-
(x.method === 'create' || x.method === '*')
112-
&& (x.service === 'guppy' || x.service === '*')
113-
))) {
114-
data.resources.push(key);
115-
}
116-
});
117-
log.debug('[ArboristClient] data: ', data);
118-
return data;
119-
},
120-
(err) => {
121-
log.error(err);
122-
throw new CodedError(500, err);
123-
},
12443
);
12544
}
12645
}

‎src/server/auth/authHelper.js

+5-5
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {
55
getRequestResourceListFromFilter,
66
buildFilterWithResourceList,
77
getAccessibleResourcesFromArboristasync,
8-
getAccessibleCreateResourcesFromArboristasync
8+
canRefresh,
99
} from './utils';
1010
import config from '../config';
1111

@@ -19,8 +19,8 @@ export class AuthHelper {
1919
this._accessibleResourceList = await getAccessibleResourcesFromArboristasync(this._jwt);
2020
log.debug('[AuthHelper] accessible resources:', this._accessibleResourceList);
2121

22-
this._accessibleCreateResourceList = await getAccessibleCreateResourcesFromArboristasync(this._jwt);
23-
log.debug('[AuthHelper] accessible resources with create method:', this._accessibleCreateResourceList);
22+
this._canRefresh = await canRefresh(this._jwt);
23+
log.debug('[AuthHelper] can user refresh:', this._canRefresh);
2424

2525
const promiseList = [];
2626
config.esConfig.indices.forEach(({ index, type }) => {
@@ -43,8 +43,8 @@ export class AuthHelper {
4343
return this._accessibleResourceList;
4444
}
4545

46-
getAccessibleCreateResources(){
47-
return this._accessibleCreateResourceList
46+
getCanRefresh(){
47+
return this._canRefresh
4848
}
4949

5050
getUnaccessibleResources() {

‎src/server/auth/utils.js

+28-7
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ export const getAccessibleResourcesFromArboristasync = async (jwt) => {
1616
],
1717
};
1818
} else {
19-
data = await arboristClient.listAuthorizedResources(jwt);
19+
data = await arboristClient.listAuthMapping(jwt);
2020
}
2121

2222
log.debug('[authMiddleware] list resources: ', JSON.stringify(data, null, 4));
@@ -27,11 +27,13 @@ export const getAccessibleResourcesFromArboristasync = async (jwt) => {
2727
}
2828
throw new CodedError(data.error.code, data.error.message);
2929
}
30+
31+
data = resourcePathsWithServiceMethodCombination(data, ['guppy', '*'], ['read', '*'])
3032
const resources = data.resources ? _.uniq(data.resources) : [];
3133
return resources;
3234
};
3335

34-
export const getAccessibleCreateResourcesFromArboristasync = async (jwt) => {
36+
export const canRefresh = async (jwt) => {
3537
let data;
3638
if (config.internalLocalTest) {
3739
data = {
@@ -41,19 +43,21 @@ export const getAccessibleCreateResourcesFromArboristasync = async (jwt) => {
4143
],
4244
};
4345
} else {
44-
data = await arboristClient.listAuthorizedCreateResources(jwt);
46+
data = await arboristClient.listAuthMapping(jwt);
4547
}
4648

47-
log.debug('[authMiddleware] list create resources: ', JSON.stringify(data, null, 4));
49+
log.debug('[authMiddleware] list resources: ', JSON.stringify(data, null, 4));
4850
if (data && data.error) {
4951
// if user is not in arborist db, assume has no access to any
5052
if (data.error.code === 404) {
51-
return [];
53+
return false;
5254
}
5355
throw new CodedError(data.error.code, data.error.message);
5456
}
55-
const resources = data.resources ? _.uniq(data.resources) : [];
56-
return resources;
57+
data = resourcePathsWithServiceMethodCombination(data, ['guppy'], ['admin_access', '*'])
58+
59+
// Only guppy_admin resource path can control guppy admin access
60+
return data.resources ? data.resources.includes('/guppy_admin') : false;
5761
};
5862

5963
export const getRequestResourceListFromFilter = async (
@@ -74,3 +78,20 @@ export const buildFilterWithResourceList = (resourceList = []) => {
7478
};
7579
return filter;
7680
};
81+
82+
export const resourcePathsWithServiceMethodCombination = (userAuthMapping, services, methods = {}) => {
83+
const data = {
84+
resources: [],
85+
};
86+
Object.keys(userAuthMapping).forEach((key) => {
87+
// logic: you have access to a project if you have
88+
// access to any of the combinations made by the method and service lists
89+
if (userAuthMapping[key] && userAuthMapping[key].some((x) => (
90+
methods.includes(x.method)
91+
&& services.includes(x.service)
92+
))) {
93+
data.resources.push(key);
94+
}
95+
});
96+
return data
97+
}

‎src/server/server.js

+7-7
Original file line numberDiff line numberDiff line change
@@ -28,24 +28,24 @@ app.use(bodyParser.json({ limit: '50mb' }));
2828
const refreshRouter = async (req, res, next) => {
2929
res.setHeader('Content-Type', 'application/json; charset=utf-8');
3030
try {
31-
if (config.allowRefresh) {
31+
if (config.allowRefresh === false) {
32+
const disabledRefresh = new CodedError(404, '[Refresh] guppy _refresh functionality is not enabled');
33+
throw disabledRefresh;
34+
}
35+
else if (config.allowRefresh) {
3236
log.debug('[Refresh] ', JSON.stringify(req.body, null, 4));
3337
const jwt = headerParser.parseJWT(req);
3438
if (!jwt) {
3539
const noJwtError = new CodedError(401, '[Refresh] no JWT user token provided to _refresh function');
3640
throw noJwtError;
3741
}
3842
const authHelper = await getAuthHelperInstance(jwt);
39-
console.log("AUTH HELPER: ", authHelper)
40-
if (authHelper._accessibleCreateResourceList === undefined || authHelper._accessibleCreateResourceList.length === 0) {
41-
const noPermsUser = new CodedError(401, '[Refresh] User cannot refresh Guppy without a valid token that has read access to at least one project');
43+
if (authHelper._canRefresh === undefined || authHelper._canRefresh === false) {
44+
const noPermsUser = new CodedError(401, '[Refresh] User cannot refresh Guppy without a valid token that has admin_access method on guppy service for resource path /guppy_admin');
4245
throw noPermsUser;
4346
}
4447
await server.stop()
4548
await initializeAndStartServer();
46-
} else if (config.allowRefresh === false) {
47-
const disabledRefresh = new CodedError(404, '[Refresh] guppy _refresh functionality is not enabled');
48-
throw disabledRefresh;
4949
}
5050
res.send("[Refresh] guppy refreshed successfully")
5151
} catch (err) {

0 commit comments

Comments
 (0)