Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: check for only matching fingerprint on key lookups #3784

Merged
merged 3 commits into from
Aug 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions services/api-sidecar-handler/internal/server/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
)

var (
rsaPub string = base64.StdEncoding.EncodeToString([]byte("ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACAQDEZlms5XsiyWjmnnUyhpt93VgHypse9Bl8kNkmZJTiM3Ex/wZAfwogzqd2LrTEiIOWSH1HnQazR+Cc9oHCmMyNxRrLkS/MEl0yZ38Q+GDfn37h/llCIZNVoHlSgYkqD0MQrhfGL5AulDUKIle93dA6qdCUlnZZjDPiR0vEXR36xGuX7QYAhK30aD2SrrBruTtFGvj87IP/0OEOvUZe8dcU9G/pCoqrTzgKqJRpqs/s5xtkqLkTIyR/SzzplO21A+pCKNax6csDDq3snS8zfx6iM8MwVfh8nvBW9seax1zBvZjHAPSTsjzmZXm4z32/ujAn/RhIkZw3ZgRKrxzryttGnWJJ8OFyF31JTJgwWWuPdH53G15PC83ZbmEgSV3win51RZRVppN4uQUuaqZWG9wwk2a6P5aen1RLCSLpTkd2mAEk9PlgmJrf8vITkiU9pF9n68ENCoo556qSdxW2pxnjrzKVPSqmqO1Xg5K4LOX4/9N4n4qkLEOiqnzzJClhFif3O28RW86RPxERGdPT81UI0oDAcU5euQr8Emz+Hd+PY1115UIld3CIHib5PYL9Ee0bFUKiWpR/acSe1fHB64mCoHP7hjFepGsq7inkvg2651wUDKBshGltpNkMj6+aZedNc0/rKYyjl80nT8g8QECgOSRzpmYp0zli2HpFoLOiWw== local-cli"))
ed25519Key string = base64.StdEncoding.EncodeToString([]byte(`-----BEGIN OPENSSH PRIVATE KEY-----
b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAArAAAABNlY2RzYS
1zaGEyLW5pc3RwNTIxAAAACG5pc3RwNTIxAAAAhQQA/BOcH7y4PL73zvZph1bGUCvHTYHS
Expand Down Expand Up @@ -40,6 +41,13 @@ func Test_validatePublicKey(t *testing.T) {
want: `{"error":"ssh: no key found"}`,
statusCode: http.StatusInternalServerError,
},
{
name: "with public rsa",
method: http.MethodPost,
input: fmt.Sprintf("key=%s", rsaPub),
want: `{"publickey":"ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACAQDEZlms5XsiyWjmnnUyhpt93VgHypse9Bl8kNkmZJTiM3Ex/wZAfwogzqd2LrTEiIOWSH1HnQazR+Cc9oHCmMyNxRrLkS/MEl0yZ38Q+GDfn37h/llCIZNVoHlSgYkqD0MQrhfGL5AulDUKIle93dA6qdCUlnZZjDPiR0vEXR36xGuX7QYAhK30aD2SrrBruTtFGvj87IP/0OEOvUZe8dcU9G/pCoqrTzgKqJRpqs/s5xtkqLkTIyR/SzzplO21A+pCKNax6csDDq3snS8zfx6iM8MwVfh8nvBW9seax1zBvZjHAPSTsjzmZXm4z32/ujAn/RhIkZw3ZgRKrxzryttGnWJJ8OFyF31JTJgwWWuPdH53G15PC83ZbmEgSV3win51RZRVppN4uQUuaqZWG9wwk2a6P5aen1RLCSLpTkd2mAEk9PlgmJrf8vITkiU9pF9n68ENCoo556qSdxW2pxnjrzKVPSqmqO1Xg5K4LOX4/9N4n4qkLEOiqnzzJClhFif3O28RW86RPxERGdPT81UI0oDAcU5euQr8Emz+Hd+PY1115UIld3CIHib5PYL9Ee0bFUKiWpR/acSe1fHB64mCoHP7hjFepGsq7inkvg2651wUDKBshGltpNkMj6+aZedNc0/rKYyjl80nT8g8QECgOSRzpmYp0zli2HpFoLOiWw== local-cli","type":"ssh-rsa","value":"AAAAB3NzaC1yc2EAAAADAQABAAACAQDEZlms5XsiyWjmnnUyhpt93VgHypse9Bl8kNkmZJTiM3Ex/wZAfwogzqd2LrTEiIOWSH1HnQazR+Cc9oHCmMyNxRrLkS/MEl0yZ38Q+GDfn37h/llCIZNVoHlSgYkqD0MQrhfGL5AulDUKIle93dA6qdCUlnZZjDPiR0vEXR36xGuX7QYAhK30aD2SrrBruTtFGvj87IP/0OEOvUZe8dcU9G/pCoqrTzgKqJRpqs/s5xtkqLkTIyR/SzzplO21A+pCKNax6csDDq3snS8zfx6iM8MwVfh8nvBW9seax1zBvZjHAPSTsjzmZXm4z32/ujAn/RhIkZw3ZgRKrxzryttGnWJJ8OFyF31JTJgwWWuPdH53G15PC83ZbmEgSV3win51RZRVppN4uQUuaqZWG9wwk2a6P5aen1RLCSLpTkd2mAEk9PlgmJrf8vITkiU9pF9n68ENCoo556qSdxW2pxnjrzKVPSqmqO1Xg5K4LOX4/9N4n4qkLEOiqnzzJClhFif3O28RW86RPxERGdPT81UI0oDAcU5euQr8Emz+Hd+PY1115UIld3CIHib5PYL9Ee0bFUKiWpR/acSe1fHB64mCoHP7hjFepGsq7inkvg2651wUDKBshGltpNkMj6+aZedNc0/rKYyjl80nT8g8QECgOSRzpmYp0zli2HpFoLOiWw==","sha256fingerprint":"SHA256:kDhWiCrizJSFOAPdDcpVIOV3W9f2VlwtjUrDUudJgTg","md5fingerprint":"cd:1f:41:c6:8c:28:2e:5c:64:3a:c4:ce:75:f7:71:83","comment":"local-cli"}`,
statusCode: http.StatusOK,
},
{
name: "with public ed25519",
method: http.MethodPost,
Expand Down
12 changes: 12 additions & 0 deletions services/api/src/apolloServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,12 @@ const apolloServer = new ApolloServer({
// grab the users project ids and roles in the first request
groupRoleProjectIds = await User.User(modelClients).getAllProjectsIdsForUser(currentUser.id, keycloakUsersGroups);
}
if (legacyGrant) {
const { role } = legacyGrant;
if (role == 'admin') {
platformOwner = true
}
}

return {
keycloakAdminClient,
Expand Down Expand Up @@ -227,6 +233,12 @@ const apolloServer = new ApolloServer({
groupRoleProjectIds = await User.User(modelClients).getAllProjectsIdsForUser(currentUser.id, keycloakUsersGroups);
await User.User(modelClients).userLastAccessed(currentUser);
}
if (legacyGrant) {
const { role } = legacyGrant;
if (role == 'admin') {
platformOwner = true
}
}

// do a permission check to see if the user is platform admin/owner, or has permission for `viewAll` on certain resources
// this reduces the number of `viewAll` permission look ups that could potentially occur during subfield resolvers for non admin users
Expand Down
12 changes: 6 additions & 6 deletions services/api/src/resources/group/resolvers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export const getAllGroups: ResolverFn = async (
{ hasPermission, models, keycloakGrant, keycloakUsersGroups, adminScopes }
) => {
// use the admin scope check instead of `hasPermission` for speed
if (adminScopes.platformOwner && adminScopes.platformViewer) {
if (adminScopes.platformOwner || adminScopes.platformViewer) {
try {

if (name) {
Expand Down Expand Up @@ -99,7 +99,7 @@ export const getGroupRolesByUserId: ResolverFn =async (
{ hasPermission, models, keycloakGrant, keycloakUsersGroups, adminScopes }
) => {
// use the admin scope check instead of `hasPermission` for speed
if (adminScopes.platformOwner && adminScopes.platformViewer) {
if (adminScopes.platformOwner || adminScopes.platformViewer) {
try {
const queryUserGroups = await models.UserModel.getAllGroupsForUser(uid);
let groups = []
Expand Down Expand Up @@ -192,7 +192,7 @@ export const getGroupsByProjectId: ResolverFn = async (
{ hasPermission, sqlClientPool, models, keycloakGrant, keycloakUsersGroups, adminScopes }
) => {
// use the admin scope check instead of `hasPermission` for speed
if (adminScopes.platformOwner && adminScopes.platformViewer) {
if (adminScopes.platformOwner || adminScopes.platformViewer) {
try {
const projectGroups = await Helpers(sqlClientPool).selectGroupsByProjectId(models, pid)
return projectGroups;
Expand Down Expand Up @@ -260,7 +260,7 @@ export const getGroupsByUserId: ResolverFn = async (
{ hasPermission, models, keycloakGrant, keycloakUsersGroups, adminScopes }
) => {
// use the admin scope check instead of `hasPermission` for speed
if (adminScopes.platformOwner && adminScopes.platformViewer) {
if (adminScopes.platformOwner || adminScopes.platformViewer) {
try {
const queryUserGroups = await models.UserModel.getAllGroupsForUser(uid);

Expand All @@ -283,7 +283,7 @@ export const getGroupByName: ResolverFn = async (
{ models, hasPermission, keycloakGrant, keycloakUsersGroups, adminScopes }
) => {
// use the admin scope check instead of `hasPermission` for speed
if (adminScopes.platformOwner && adminScopes.platformViewer) {
if (adminScopes.platformOwner || adminScopes.platformViewer) {
try {
const group = await models.GroupModel.loadGroupByName(name);
return group;
Expand Down Expand Up @@ -748,7 +748,7 @@ export const getAllProjectsInGroup: ResolverFn = async (
} = models;

// use the admin scope check instead of `hasPermission` for speed
if (adminScopes.platformOwner && adminScopes.platformViewer) {
if (adminScopes.platformOwner || adminScopes.platformViewer) {
try {
// get group from all keycloak groups apollo context
const group = await loadGroupByIdOrName(groupInput);
Expand Down
1 change: 1 addition & 0 deletions services/api/src/routes/keys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ const keysRoute = async (
sqlClientPool,
knex('ssh_key AS sk')
.select(knex.raw("CONCAT(sk.key_type, ' ', sk.key_value) as sshKey"))
.where("key_fingerprint","=", fingerprint)
.toString(),
);
const keys = R.map(R.prop('sshKey'), rows);
Expand Down