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

SCIM users endpoint #1199

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
10 changes: 9 additions & 1 deletion app/gen-server/lib/homedb/HomeDBManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,10 @@ export class HomeDBManager extends EventEmitter {
return this._usersManager.getUser(userId, options);
}

public async getUsers() {
return this._usersManager.getUsers();
}

public async getFullUser(userId: number) {
return this._usersManager.getFullUser(userId);
}
Expand Down Expand Up @@ -560,6 +564,10 @@ export class HomeDBManager extends EventEmitter {
return this._usersManager.deleteUser(scope, userIdToDelete, name);
}

public async overrideUser(userId: number, props: UserProfile) {
return this._usersManager.overrideUser(userId, props);
}

/**
* Returns a QueryResult for the given organization. The orgKey
* can be a string (the domain from url) or the id of an org. If it is
Expand Down Expand Up @@ -3597,7 +3605,7 @@ export class HomeDBManager extends EventEmitter {
// Get the user objects which map to non-null values in the userDelta.
const userIds = Object.keys(userDelta).filter(userId => userDelta[userId])
.map(userIdStr => parseInt(userIdStr, 10));
const users = await this._usersManager.getUsers(userIds, manager);
const users = await this._usersManager.getUsersByIds(userIds, manager);

// Add unaffected users to the delta so that we have a record of where they are.
groups.forEach(grp => {
Expand Down
34 changes: 32 additions & 2 deletions app/gen-server/lib/homedb/UsersManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ export class UsersManager {
// Set the user's name if our provider knows it. Otherwise use their username
// from email, for lack of something better. If we don't have a profile at this
// time, then leave the name blank in the hopes of learning it when the user logs in.
user.name = (profile && (profile.name || email.split('@')[0])) || '';
user.name = (profile && this._getNameOrDeduceFromEmail(profile.name, email)) || '';
needUpdate = true;
}
if (!user.picture && profile && profile.picture) {
Expand Down Expand Up @@ -582,6 +582,32 @@ export class UsersManager {
.filter(fullProfile => fullProfile);
}

/**
* Update users with passed property. Optional user properties that are missing will be reset to their default value.
*/
public async overrideUser(userId: number, props: UserProfile): Promise<User> {
return await this._connection.transaction(async manager => {
const user = await this.getUser(userId, {includePrefs: true});
if (!user) { throw new ApiError("unable to find user to update", 404); }
const login = user.logins[0];
user.name = this._getNameOrDeduceFromEmail(props.name, props.email);
user.picture = props.picture || '';
user.options = {...(user.options || {}), locale: props.locale ?? undefined};
if (props.email) {
login.email = normalizeEmail(props.email);
login.displayEmail = props.email;
}
await manager.save([user, login]);

return (await this.getUser(userId))!;
});
}
fflorent marked this conversation as resolved.
Show resolved Hide resolved

public async getUsers() {
return await User.find({relations: ["logins"]});
}


/**
* ==================================
*
Expand Down Expand Up @@ -688,7 +714,7 @@ export class UsersManager {
/**
* Returns a Promise for an array of User entites for the given userIds.
*/
public async getUsers(userIds: number[], optManager?: EntityManager): Promise<User[]> {
public async getUsersByIds(userIds: number[], optManager?: EntityManager): Promise<User[]> {
if (userIds.length === 0) {
return [];
}
Expand Down Expand Up @@ -772,6 +798,10 @@ export class UsersManager {
return id;
}

private _getNameOrDeduceFromEmail(name: string, email: string) {
return name || email.split('@')[0];
}

// This deals with the problem posed by receiving a PermissionDelta specifying a
// role for both alice@x and Alice@x. We do not distinguish between such emails.
// If there are multiple indistinguishabe emails, we preserve just one of them,
Expand Down
1 change: 1 addition & 0 deletions app/server/MergedServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ export class MergedServer {
this.flexServer.addUpdatesCheck();
// todo: add support for home api to standalone app
this.flexServer.addHomeApi();
this.flexServer.addScimApi();
this.flexServer.addBillingApi();
this.flexServer.addNotifier();
this.flexServer.addAuditLogger();
Expand Down
14 changes: 14 additions & 0 deletions app/server/lib/FlexServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ import * as path from 'path';
import * as serveStatic from "serve-static";
import {ConfigBackendAPI} from "app/server/lib/ConfigBackendAPI";
import {IGristCoreConfig} from "app/server/lib/configCore";
import { buildScimRouter } from './scim';

// Health checks are a little noisy in the logs, so we don't show them all.
// We show the first N health checks:
Expand Down Expand Up @@ -888,6 +889,19 @@ export class FlexServer implements GristServer {
new ApiServer(this, this.app, this._dbManager, this._widgetRepository = buildWidgetRepository(this));
}

public addScimApi() {
if (this._check('scim', 'api', 'homedb', 'json', 'api-mw')) { return; }

const scimRouter = isAffirmative(process.env.GRIST_ENABLE_SCIM) ?
buildScimRouter(this._dbManager, this._installAdmin) :
() => {
throw new ApiError('SCIM API is not enabled', 501);
};

this.app.use('/api/scim', scimRouter);
}


public addBillingApi() {
if (this._check('billing-api', 'homedb', 'json', 'api-mw')) { return; }
this._getBilling();
Expand Down
14 changes: 14 additions & 0 deletions app/server/lib/scim/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import * as express from 'express';

import { buildScimRouterv2 } from './v2/ScimV2Api';
import { HomeDBManager } from 'app/gen-server/lib/homedb/HomeDBManager';
import { InstallAdmin } from '../InstallAdmin';

const buildScimRouter = (dbManager: HomeDBManager, installAdmin: InstallAdmin) => {
const v2 = buildScimRouterv2(dbManager, installAdmin);
const scim = express.Router();
scim.use('/v2', v2);
return scim;
};

export { buildScimRouter };
6 changes: 6 additions & 0 deletions app/server/lib/scim/v2/ScimTypes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export interface RequestContext {
path: string;
isAdmin: boolean;
isScimUser: boolean;
}

168 changes: 168 additions & 0 deletions app/server/lib/scim/v2/ScimUserController.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
import { ApiError } from 'app/common/ApiError';
import { HomeDBManager, Scope } from 'app/gen-server/lib/homedb/HomeDBManager';
import SCIMMY from 'scimmy';
import { toSCIMMYUser, toUserProfile } from './ScimUserUtils';
import { RequestContext } from './ScimTypes';
import log from 'app/server/lib/log';

class ScimUserController {
private static _getIdFromResource(resource: any) {
const id = parseInt(resource.id, 10);
if (Number.isNaN(id)) {
throw new SCIMMY.Types.Error(400, 'invalidValue', 'Invalid passed user ID');
}
return id;
}

constructor(
private _dbManager: HomeDBManager,
private _checkAccess: (context: RequestContext) => void
) {}

/**
* Gets a single user with the passed ID.
*
* @param resource The SCIMMY user resource performing the operation
* @param context The request context
*/
public async getSingleUser(resource: any, context: RequestContext) {
return this._runAndHandleErrors(context, async () => {
const id = ScimUserController._getIdFromResource(resource);
const user = await this._dbManager.getUser(id);
if (!user) {
throw new SCIMMY.Types.Error(404, null!, `User with ID ${id} not found`);
}
return toSCIMMYUser(user);
});
}

/**
* Gets all users or filters them based on the passed filter.
*
* @param resource The SCIMMY user resource performing the operation
* @param context The request context
*/
public async getUsers(resource: any, context: RequestContext) {
return this._runAndHandleErrors(context, async () => {
const { filter } = resource;
const scimmyUsers = (await this._dbManager.getUsers()).map(user => toSCIMMYUser(user));
return filter ? filter.match(scimmyUsers) : scimmyUsers;
});
}

/**
* Creates a new user with the passed data.
*
* @param data The data to create the user with
* @param context The request context
*/
public async createUser(data: any, context: RequestContext) {
return this._runAndHandleErrors(context, async () => {
await this._checkEmailCanBeUsed(data.userName);
const userProfile = toUserProfile(data);
const newUser = await this._dbManager.getUserByLoginWithRetry(userProfile.email, {
profile: userProfile
});
return toSCIMMYUser(newUser);
});
}

/**
* Overrides a user with the passed data.
*
* @param resource The SCIMMY user resource performing the operation
* @param data The data to override the user with
* @param context The request context
*/
public async overrideUser(resource: any, data: any, context: RequestContext) {
return this._runAndHandleErrors(context, async () => {
const id = ScimUserController._getIdFromResource(resource);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should not authorize the update and the deletion of Special Ids.

There also is a edge case for GRIST_DEFAULT_EMAIL, should we allow the IdP administrator propagate a change of their email (which would the owner of that email would lose the installation admin right)? I guess it's the responsibility of the IdP admin to be sure of the impacts, also it is easy to fix.

Do you have any thoughts about these cases?

await this._checkEmailCanBeUsed(data.userName, id);
const updatedUser = await this._dbManager.overrideUser(id, toUserProfile(data));
return toSCIMMYUser(updatedUser);
});
}

/**
* Deletes a user with the passed ID.
*
* @param resource The SCIMMY user resource performing the operation
* @param context The request context
*/
public async deleteUser(resource: any, context: RequestContext) {
return this._runAndHandleErrors(context, async () => {
const id = ScimUserController._getIdFromResource(resource);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same problem regarding the Special Ids

const fakeScope: Scope = { userId: id };
// FIXME: deleteUser should probably better not requiring a scope.
await this._dbManager.deleteUser(fakeScope, id);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels like a bit dirty to me to create a fake Scope, but I fear it is hard to get rid of it from UsersManager.deleteUser, especially because the deleteOrg needs it.

Any opinion/idea on that?

});
}

/**
* Runs the passed callback and handles any errors that might occur.
* Also checks if the user has access to the operation.
* Any public method of this class should be run through this method.
*
* @param context The request context to check access for the user
* @param cb The callback to run
*/
private async _runAndHandleErrors<T>(context: RequestContext, cb: () => Promise<T>): Promise<T> {
try {
this._checkAccess(context);
return await cb();
} catch (err) {
if (err instanceof ApiError) {
log.error('[ScimUserController] ApiError: ', err.status, err.message);
if (err.status === 409) {
throw new SCIMMY.Types.Error(err.status, 'uniqueness', err.message);
}
throw new SCIMMY.Types.Error(err.status, null!, err.message);
}
if (err instanceof SCIMMY.Types.Error) {
log.error('[ScimUserController] SCIMMY.Types.Error: ', err.message);
throw err;
}
// By default, return a 500 error
log.error('[ScimUserController] Error: ', err.message);
throw new SCIMMY.Types.Error(500, null!, err.message);
}
}

/**
* Checks if the passed email can be used for a new user or by the existing user.
*
* @param email The email to check
* @param userIdToUpdate The ID of the user to update. Pass this when updating a user,
* so it won't raise an error if the passed email is already used by this user.
*/
private async _checkEmailCanBeUsed(email: string, userIdToUpdate?: number) {
const existingUser = await this._dbManager.getExistingUserByLogin(email);
if (existingUser !== undefined && existingUser.id !== userIdToUpdate) {
throw new SCIMMY.Types.Error(409, 'uniqueness', 'An existing user with the passed email exist.');
}
}
}

export const getScimUserConfig = (
dbManager: HomeDBManager, checkAccess: (context: RequestContext) => void
) => {
const controller = new ScimUserController(dbManager, checkAccess);

return {
egress: async (resource: any, context: RequestContext) => {
if (resource.id) {
return await controller.getSingleUser(resource, context);
}
return await controller.getUsers(resource, context);
},
ingress: async (resource: any, data: any, context: RequestContext) => {
if (resource.id) {
return await controller.overrideUser(resource, data, context);
}
return await controller.createUser(data, context);
},
degress: async (resource: any, context: RequestContext) => {
return await controller.deleteUser(resource, context);
}
};
};
46 changes: 46 additions & 0 deletions app/server/lib/scim/v2/ScimUserUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { normalizeEmail } from "app/common/emails";
import { UserProfile } from "app/common/LoginSessionAPI";
import { User } from "app/gen-server/entity/User.js";
import SCIMMY from "scimmy";

/**
* Converts a user from your database to a SCIMMY user
*/
export function toSCIMMYUser(user: User) {
if (!user.logins) {
throw new Error("User must have at least one login");
}
const locale = user.options?.locale ?? "en";
return new SCIMMY.Schemas.User({
id: String(user.id),
userName: user.loginEmail,
displayName: user.name,
name: {
formatted: user.name,
},
locale,
preferredLanguage: locale, // Assume preferredLanguage is the same as locale
photos: user.picture ? [{
value: user.picture,
type: "photo",
primary: true
}] : undefined,
emails: [{
value: user.logins[0].displayEmail,
primary: true,
}],
});
}

export function toUserProfile(scimUser: any, existingUser?: User): UserProfile {
const emailValue = scimUser.emails?.[0]?.value;
if (emailValue && normalizeEmail(emailValue) !== normalizeEmail(scimUser.userName)) {
throw new SCIMMY.Types.Error(400, 'invalidValue', 'Email and userName must be the same');
fflorent marked this conversation as resolved.
Show resolved Hide resolved
}
return {
name: scimUser.displayName ?? existingUser?.name,
picture: scimUser.photos?.[0]?.value,
locale: scimUser.locale,
email: emailValue ?? scimUser.userName ?? existingUser?.loginEmail,
};
}
Loading
Loading