Skip to content

Commit d2a17c9

Browse files
committed
Merge remote-tracking branch 'finos/main' into 946-associate-commits-by-email-rebase
2 parents 6044dec + c7d94a9 commit d2a17c9

File tree

17 files changed

+318
-69
lines changed

17 files changed

+318
-69
lines changed

package-lock.json

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@finos/git-proxy",
3-
"version": "1.19.0",
3+
"version": "1.19.1",
44
"description": "Deploy custom push protections and policies on top of Git.",
55
"scripts": {
66
"cli": "node ./packages/git-proxy-cli/index.js",

src/db/file/pushes.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import fs from 'fs';
22
import _ from 'lodash';
33
import Datastore from '@seald-io/nedb';
44
import { Action } from '../../proxy/actions/Action';
5-
import { toClass } from '../helper';
5+
import { toClass, trimTrailingDotGit } from '../helper';
66
import * as repo from './repo';
77
import { PushQuery } from '../types';
88

@@ -138,7 +138,7 @@ export const canUserCancelPush = async (id: string, user: string) => {
138138
return;
139139
}
140140

141-
const repoName = pushDetail.repoName.replace('.git', '');
141+
const repoName = trimTrailingDotGit(pushDetail.repoName);
142142
const isAllowed = await repo.isUserPushAllowed(repoName, user);
143143

144144
if (isAllowed) {
@@ -156,7 +156,8 @@ export const canUserApproveRejectPush = async (id: string, user: string) => {
156156
resolve(false);
157157
return;
158158
}
159-
const repoName = action.repoName.replace('.git', '');
159+
160+
const repoName = trimTrailingDotGit(action.repoName);
160161
const isAllowed = await repo.canUserApproveRejectPushRepo(repoName, user);
161162

162163
resolve(isAllowed);

src/db/helper.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,21 @@ export const toClass = function (obj: any, proto: any) {
33
obj.__proto__ = proto;
44
return obj;
55
};
6+
7+
export const trimTrailingDotGit = (str: string): string => {
8+
const target = '.git';
9+
if (str.endsWith(target)) {
10+
// extract string from 0 to the end minus the length of target
11+
return str.slice(0, -target.length);
12+
}
13+
return str;
14+
};
15+
16+
export const trimPrefixRefsHeads = (str: string): string => {
17+
const target = 'refs/heads/';
18+
if (str.startsWith(target)) {
19+
// extract string from the end of the target to the end of str
20+
return str.slice(target.length);
21+
}
22+
return str;
23+
};

src/db/mongo/pushes.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { connect, findDocuments, findOneDocument } from './helper';
22
import { Action } from '../../proxy/actions';
3-
import { toClass } from '../helper';
3+
import { toClass, trimTrailingDotGit } from '../helper';
44
import * as repo from './repo';
55
import { Push, PushQuery } from '../types';
66

@@ -108,7 +108,7 @@ export const canUserApproveRejectPush = async (id: string, user: string) => {
108108
return;
109109
}
110110

111-
const repoName = action.repoName.replace('.git', '');
111+
const repoName = trimTrailingDotGit(action.repoName);
112112
const isAllowed = await repo.canUserApproveRejectPushRepo(repoName, user);
113113

114114
resolve(isAllowed);
@@ -123,7 +123,7 @@ export const canUserCancelPush = async (id: string, user: string) => {
123123
return;
124124
}
125125

126-
const repoName = pushDetail.repoName.replace('.git', '');
126+
const repoName = trimTrailingDotGit(pushDetail.repoName);
127127
const isAllowed = await repo.isUserPushAllowed(repoName, user);
128128

129129
if (isAllowed) {

src/proxy/processors/push-action/checkRepoInAuthorisedList.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { Action, Step } from '../../actions';
22
import { getRepos } from '../../../db';
33
import { Repo } from '../../../db/types';
4+
import { trimTrailingDotGit } from '../../../db/helper';
45

56
// Execute if the repo is approved
67
const exec = async (
@@ -14,8 +15,8 @@ const exec = async (
1415
console.log(list);
1516

1617
const found = list.find((x: Repo) => {
17-
const targetName = action.repo.replace('.git', '').toLowerCase();
18-
const allowedName = `${x.project}/${x.name}`.replace('.git', '').toLowerCase();
18+
const targetName = trimTrailingDotGit(action.repo.toLowerCase());
19+
const allowedName = trimTrailingDotGit(`${x.project}/${x.name}`.toLowerCase());
1920
console.log(`${targetName} = ${allowedName}`);
2021
return targetName === allowedName;
2122
});

src/proxy/processors/push-action/checkUserPushPermission.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,20 @@
11
import { Action, Step } from '../../actions';
22
import { getUsers, isUserPushAllowed } from '../../../db';
3+
import { trimTrailingDotGit } from '../../../db/helper';
34

45
// Execute if the repo is approved
56
const exec = async (req: any, action: Action): Promise<Action> => {
67
const step = new Step('checkUserPushPermission');
78

8-
const repoName = action.repo.split('/')[1].replace('.git', '');
9+
const repoSplit = trimTrailingDotGit(action.repo.toLowerCase()).split('/');
10+
// we expect there to be exactly one / separating org/repoName
11+
if (repoSplit.length != 2) {
12+
step.setError('Server-side issue extracting repoName');
13+
action.addStep(step);
14+
return action;
15+
}
16+
// pull the 2nd value of the split for repoName
17+
const repoName = repoSplit[1];
918
let isUserAllowed = false;
1019

1120
// n.b. action.user will be set to whatever the user had set in their user.name config in their git client.

src/service/routes/auth.js

Lines changed: 29 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ const passportLocal = require('../passport/local');
66
const passportAD = require('../passport/activeDirectory');
77
const authStrategies = require('../passport').authStrategies;
88
const db = require('../../db');
9+
const { toPublicUser } = require('./publicApi');
910
const { GIT_PROXY_UI_HOST: uiHost = 'http://localhost', GIT_PROXY_UI_PORT: uiPort = 3000 } =
1011
process.env;
1112

@@ -46,6 +47,25 @@ const getLoginStrategy = () => {
4647
return enabledAppropriateLoginStrategies[0].type.toLowerCase();
4748
};
4849

50+
const loginSuccessHandler = () => async (req, res) => {
51+
try {
52+
const currentUser = { ...req.user };
53+
delete currentUser.password;
54+
console.log(
55+
`serivce.routes.auth.login: user logged in, username=${
56+
currentUser.username
57+
} profile=${JSON.stringify(currentUser)}`,
58+
);
59+
res.send({
60+
message: 'success',
61+
user: toPublicUser(currentUser),
62+
});
63+
} catch (e) {
64+
console.log(`service.routes.auth.login: Error logging user in ${JSON.stringify(e)}`);
65+
res.status(500).send('Failed to login').end();
66+
}
67+
};
68+
4969
// TODO: provide separate auth endpoints for each auth strategy or chain compatibile auth strategies
5070
// TODO: if providing separate auth methods, inform the frontend so it has relevant UI elements and appropriate client-side behavior
5171
router.post(
@@ -59,25 +79,7 @@ router.post(
5979
console.log('going to auth with', authType);
6080
return passport.authenticate(authType)(req, res, next);
6181
},
62-
async (req, res) => {
63-
try {
64-
const currentUser = { ...req.user };
65-
delete currentUser.password;
66-
console.log(
67-
`serivce.routes.auth.login: user logged in, username=${
68-
currentUser.username
69-
} profile=${JSON.stringify(currentUser)}`,
70-
);
71-
res.send({
72-
message: 'success',
73-
user: currentUser,
74-
});
75-
} catch (e) {
76-
console.log(`service.routes.auth.login: Error logging user in ${JSON.stringify(e)}`);
77-
res.status(500).send('Failed to login').end();
78-
return;
79-
}
80-
},
82+
loginSuccessHandler(),
8183
);
8284

8385
router.get('/oidc', passport.authenticate(authStrategies['openidconnect'].type));
@@ -114,8 +116,7 @@ router.post('/logout', (req, res, next) => {
114116
router.get('/profile', async (req, res) => {
115117
if (req.user) {
116118
const userVal = await db.findUser(req.user.username);
117-
delete userVal.password;
118-
res.send(userVal);
119+
res.send(toPublicUser(userVal));
119120
} else {
120121
res.status(401).end();
121122
}
@@ -160,14 +161,14 @@ router.post('/gitAccount', async (req, res) => {
160161

161162
router.get('/me', async (req, res) => {
162163
if (req.user) {
163-
const user = JSON.parse(JSON.stringify(req.user));
164-
if (user && user.password) delete user.password;
165-
const login = user.username;
166-
const userVal = await db.findUser(login);
167-
if (userVal && userVal.password) delete userVal.password;
168-
res.send(userVal);
164+
const userVal = await db.findUser(req.user.username);
165+
res.send(toPublicUser(userVal));
169166
} else {
170167
res.status(401).end();
171168
}
172169
});
173-
module.exports = router;
170+
171+
module.exports = {
172+
router,
173+
loginSuccessHandler
174+
};

src/service/routes/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ const jwtAuthHandler = require('../passport/jwtAuthHandler');
1010
const router = new express.Router();
1111

1212
router.use('/api', home);
13-
router.use('/api/auth', auth);
13+
router.use('/api/auth', auth.router);
1414
router.use('/api/v1/healthcheck', healthcheck);
1515
router.use('/api/v1/push', jwtAuthHandler(), push);
1616
router.use('/api/v1/repo', jwtAuthHandler(), repo);

src/service/routes/publicApi.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
export const toPublicUser = (user) => {
2+
return {
3+
username: user.username || '',
4+
displayName: user.displayName || '',
5+
email: user.email || '',
6+
title: user.title || '',
7+
gitAccount: user.gitAccount || '',
8+
admin: user.admin || false,
9+
}
10+
}

0 commit comments

Comments
 (0)