Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
1400d9b
feat: support direct querying of AD group membership via LDAP
kriswest Apr 10, 2025
f552624
test: don't clean-up test-repo as cypress tests rely on it
kriswest Apr 11, 2025
189ec0c
Merge branch 'main' into 909-ldap-user-group-confirmation
kriswest Apr 16, 2025
04e9292
Merge branch 'main' into 909-ldap-user-group-confirmation
kriswest May 7, 2025
f7e6d75
docs: regenerate reference doc
kriswest May 7, 2025
5c048f4
Merge remote-tracking branch 'upstream/909-ldap-user-group-confirmati…
kriswest May 7, 2025
3f18c26
Merge branch 'main' into 909-ldap-user-group-confirmation
kriswest May 7, 2025
043c259
chore: remove change to checkCommitMessages
kriswest May 7, 2025
86e74ec
Update src/proxy/processors/push-action/checkCommitMessages.ts
kriswest May 8, 2025
84b4534
Merge branch 'main' into 909-ldap-user-group-confirmation
kriswest May 9, 2025
e815bfc
Update src/service/passport/ldaphelper.js
kriswest May 13, 2025
b7b965d
Merge branch 'main' into 909-ldap-user-group-confirmation
kriswest May 22, 2025
5114e99
chore: regenerate config ref doc from schema
kriswest May 22, 2025
36f9ee6
Merge remote-tracking branch 'upstream/909-ldap-user-group-confirmati…
kriswest May 22, 2025
1703530
Merge branch 'main' into 909-ldap-user-group-confirmation
kriswest Jun 3, 2025
65c508d
fix: resolve conflict with 'enable-multiple-auth-methods'
kriswest Jun 3, 2025
6231db0
chore: regenerate schema doc
kriswest Jun 3, 2025
04ab644
Merge branch 'main' into 909-ldap-user-group-confirmation
kriswest Jun 4, 2025
e5fe90d
fix: adjust config schema to account for changes to auth config
kriswest Jun 4, 2025
70af4ee
fix: adjust config schema for changes to auth config from main
kriswest Jun 4, 2025
ec0acbd
Merge branch '909-ldap-user-group-confirmation' of https://natwest.gi…
kriswest Jun 4, 2025
95363db
fix: lowercase openidconnect in config schema to match code
kriswest Jun 4, 2025
ed6a4ff
docs: add auth config descriptions to config schema and regenerate re…
kriswest Jun 4, 2025
adfdb96
Merge branch 'main' into 909-ldap-user-group-confirmation
JamieSlome Jun 5, 2025
709441d
Merge branch 'main' into 909-ldap-user-group-confirmation
kriswest Jun 10, 2025
6a3146d
Merge branch 'main' into 909-ldap-user-group-confirmation
JamieSlome Jun 16, 2025
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
125 changes: 118 additions & 7 deletions config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,32 @@
"sessionMaxAgeHours": { "type": "number" },
"api": {
"description": "Third party APIs",
"type": "object"
"type": "object",
"properties": {
"ls": {
"type": "object",
"description": "Configuration used in conjunction with ActiveDirectory auth, which relates to a REST API used to check user group membership, as opposed to direct querying via LDAP.<br />If this configuration is set direct querying of group membership via LDAP will be disabled.",
"properties": {
"userInADGroup": {
"type": "string",
"description": "URL template for a GET request that confirms a user's membership of a specific group. Should respond with a non-empty 200 status if the user is a member of the group, an empty response or non-200 status indicates that the user is not a group member. If set, this URL will be queried and direct queries via LDAP will be disabled. The template should contain the following string placeholders, which will be replaced to produce the final URL:<ul><li>\"&lt;domain&gt;\": AD domain,</li><li>\"&lt;name&gt;\": The group name to check membership of.</li><li>\"&lt;id&gt;\": The username to check group membership for.</li></ul>",
"examples": [
"https://somedomain.com/some/path/checkUserGroups?domain=<domain>&name=<name>&id=<id>"
]
}
}
},
"github": {
"type": "object",
"properties": {
"baseUrl": {
"type": "string",
"format": "uri",
"examples": ["https://api.github.com"]
}
}
}
}
},
"commitConfig": {
"description": "Enforce rules and patterns on commits including e-mail and message",
Expand Down Expand Up @@ -169,12 +194,98 @@
},
"authentication": {
"type": "object",
"properties": {
"type": { "type": "string" },
"enabled": { "type": "boolean" },
"options": { "type": "object" }
},
"required": ["type", "enabled"]
"description": "Configuration for an authentication source",
"oneOf": [
{
"title": "Local Auth Config",
"description": "Configuration for the use of the local database as the authentication source.",
"properties": {
"type": { "type": "string", "const": "local" },
"enabled": { "type": "boolean" }
},
"required": ["type", "enabled"]
},
{
"title": "Active Directory Auth Config",
"description": "Configuration for Active Directory authentication.",
"properties": {
"type": { "type": "string", "const": "ActiveDirectory" },
"enabled": { "type": "boolean" },
"adminGroup": {
"type": "string",
"description": "Group that indicates that a user is an admin"
},
"userGroup": {
"type": "string",
"description": "Group that indicates that a user should be able to login to the Git Proxy UI and can work as a reviewer"
},
"domain": { "type": "string", "description": "Active Directory domain" },
"adConfig": {
"type": "object",
"description": "Additional Active Directory configuration supporting LDAP connection which can be used to confirm group membership. For the full set of available options see the activedirectory 2 NPM module docs at https://www.npmjs.com/package/activedirectory2#activedirectoryoptions <br /><br />Please note that if the Third Party APIs config `api.ls.userInADGroup` is set then the REST API it represents is used in preference to direct querying of group memebership via LDAP.",
"properties": {
"url": {
"type": "string",
"description": "Active Directory server to connect to, e.g. `ldap://ad.example.com`."
},
"baseDN": {
"type": "string",
"description": "The root DN from which all searches will be performed, e.g. `dc=example,dc=com`."
},
"username": {
"type": "string",
"description": "An account name capable of performing the operations desired."
},
"password": {
"type": "string",
"description": "Password for the given `username`."
}
},
"required": ["url", "baseDN", "username", "password"]
}
},
"required": ["type", "enabled", "adminGroup", "userGroup", "domain"]
},
{
"title": "Open ID Connect Auth Config",
"description": "Configuration for Open ID Connect authentication.",
"properties": {
"type": { "type": "string", "const": "openidconnect" },
"enabled": { "type": "boolean" },
"oidcConfig": {
"type": "object",
"description": "Additional OIDC configuration.",
"properties": {
"issuer": { "type": "string" },
"clientID": { "type": "string" },
"clientSecret": { "type": "string" },
"callbackURL": { "type": "string" },
"scope": { "type": "string" }
},
"required": ["issuer", "clientID", "clientSecret", "callbackURL", "scope"]
}
},
"required": ["type", "enabled", "oidcConfig"]
},
{
"title": "JWT Auth Config",
"description": "Configuration for JWT authentication.",
"properties": {
"type": { "type": "string", "const": "jwt" },
"enabled": { "type": "boolean" },
"jwtConfig": {
"type": "object",
"description": "Additional JWT configuration.",
"properties": {
"clientID": { "type": "string" },
"authorityURL": { "type": "string" }
},
"required": ["clientID", "authorityURL"]
}
},
"required": ["type", "enabled", "jwtConfig"]
}
]
},
"routeAuthRule": {
"type": "object",
Expand Down
4 changes: 3 additions & 1 deletion proxy.config.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,9 @@
"adConfig": {
"url": "",
"baseDN": "",
"searchBase": ""
"searchBase": "",
"username": "",
"password": ""
}
},
{
Expand Down
2 changes: 1 addition & 1 deletion src/config/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@
}
}

throw Error('No database cofigured!');
throw Error('No database configured!');

Check warning on line 90 in src/config/index.ts

View check run for this annotation

Codecov / codecov/patch

src/config/index.ts#L90

Added line #L90 was not covered by tests
};

/**
Expand Down
32 changes: 25 additions & 7 deletions src/service/passport/activeDirectory.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,35 @@
profile.id = profile.username;
req.user = profile;

// First check to see if the user is in the usergroups
const isUser = await ldaphelper.isUserInAdGroup(profile.username, domain, userGroup);

if (!isUser) {
const message = `User it not a member of ${userGroup}`;
console.log(

Check warning on line 34 in src/service/passport/activeDirectory.js

View check run for this annotation

Codecov / codecov/patch

src/service/passport/activeDirectory.js#L34

Added line #L34 was not covered by tests
`passport.activeDirectory: resolved login ${
profile._json.userPrincipalName
}, profile=${JSON.stringify(profile)}`,
);
// First check to see if the user is in the AD user group
try {
const isUser = await ldaphelper.isUserInAdGroup(req, profile, ad, domain, userGroup);
if (!isUser) {
const message = `User it not a member of ${userGroup}`;
return done(message, null);

Check warning on line 44 in src/service/passport/activeDirectory.js

View check run for this annotation

Codecov / codecov/patch

src/service/passport/activeDirectory.js#L40-L44

Added lines #L40 - L44 were not covered by tests
}
} catch (e) {
const message = `An error occurred while checking if the user is a member of the user group: ${JSON.stringify(e)}`;

Check warning on line 47 in src/service/passport/activeDirectory.js

View check run for this annotation

Codecov / codecov/patch

src/service/passport/activeDirectory.js#L47

Added line #L47 was not covered by tests
return done(message, null);
}

// Now check if the user is an admin
const isAdmin = await ldaphelper.isUserInAdGroup(profile.username, domain, adminGroup);
let isAdmin = false;
try {
isAdmin = await ldaphelper.isUserInAdGroup(req, profile, ad, domain, adminGroup);

Check warning on line 54 in src/service/passport/activeDirectory.js

View check run for this annotation

Codecov / codecov/patch

src/service/passport/activeDirectory.js#L52-L54

Added lines #L52 - L54 were not covered by tests

} catch (e) {
const message = `An error occurred while checking if the user is a member of the admin group: ${JSON.stringify(e)}`;
console.error(message, e); // don't return an error for this case as you may still be a user

Check warning on line 58 in src/service/passport/activeDirectory.js

View check run for this annotation

Codecov / codecov/patch

src/service/passport/activeDirectory.js#L57-L58

Added lines #L57 - L58 were not covered by tests
}

profile.admin = isAdmin;
console.log(`passport.activeDirectory: ${profile.username} admin=${isAdmin}`);

Check warning on line 62 in src/service/passport/activeDirectory.js

View check run for this annotation

Codecov / codecov/patch

src/service/passport/activeDirectory.js#L62

Added line #L62 was not covered by tests

const user = {
username: profile.username,
Expand Down Expand Up @@ -70,6 +87,7 @@
passport.deserializeUser(function (user, done) {
done(null, user);
});
passport.type = "ActiveDirectory";

Check warning on line 90 in src/service/passport/activeDirectory.js

View check run for this annotation

Codecov / codecov/patch

src/service/passport/activeDirectory.js#L90

Added line #L90 was not covered by tests

return passport;
};
Expand Down
40 changes: 32 additions & 8 deletions src/service/passport/ldaphelper.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,42 @@
const axios = require('axios');
const thirdpartyApiConfig = require('../../config').getAPIs();
const client = axios.create({
responseType: 'json',
headers: {
'content-type': 'application/json',
},
});
const axios = require('axios');

const isUserInAdGroup = (req, profile, ad, domain, name) => {
// determine, via config, if we're using HTTP or AD directly
if (thirdpartyApiConfig?.ls?.userInADGroup) {
return isUserInAdGroupViaHttp(profile.username, domain, name);

Check warning on line 7 in src/service/passport/ldaphelper.js

View check run for this annotation

Codecov / codecov/patch

src/service/passport/ldaphelper.js#L6-L7

Added lines #L6 - L7 were not covered by tests
} else {
return isUserInAdGroupViaAD(req, profile, ad, domain, name);

Check warning on line 9 in src/service/passport/ldaphelper.js

View check run for this annotation

Codecov / codecov/patch

src/service/passport/ldaphelper.js#L9

Added line #L9 was not covered by tests
}
};

const isUserInAdGroup = (id, domain, name) => {
const isUserInAdGroupViaAD = (req, profile, ad, domain, name) => {
return new Promise((resolve, reject) => {
ad.isUserMemberOf(profile.username, name, function (err, isMember) {
if (err) {
const msg = 'ERROR isUserMemberOf: ' + JSON.stringify(err);
reject(msg);

Check warning on line 18 in src/service/passport/ldaphelper.js

View check run for this annotation

Codecov / codecov/patch

src/service/passport/ldaphelper.js#L14-L18

Added lines #L14 - L18 were not covered by tests
} else {
console.log(profile.username + ' isMemberOf ' + name + ': ' + isMember);
resolve(isMember);

Check warning on line 21 in src/service/passport/ldaphelper.js

View check run for this annotation

Codecov / codecov/patch

src/service/passport/ldaphelper.js#L20-L21

Added lines #L20 - L21 were not covered by tests
}
});
});
};

const isUserInAdGroupViaHttp = (id, domain, name) => {
const url = String(thirdpartyApiConfig.ls.userInADGroup)
.replace('<domain>', domain)
.replace('<name>', name)
.replace('<id>', id);

const client = axios.create({

Check warning on line 33 in src/service/passport/ldaphelper.js

View check run for this annotation

Codecov / codecov/patch

src/service/passport/ldaphelper.js#L33

Added line #L33 was not covered by tests
responseType: 'json',
headers: {
'content-type': 'application/json',
},
});

console.log(`checking if user is in group ${url}`);
return client
.get(url)
Expand Down
Loading
Loading