diff --git a/.gitignore b/.gitignore index 307875b246..f0bee63c4e 100644 --- a/.gitignore +++ b/.gitignore @@ -94,3 +94,6 @@ xunit.xml /docker-compose-examples/*/persist /docker-compose-examples/*/secrets /docker-compose-examples/grist-traefik-oidc-auth/.env + +# Sample grist documents +/samples/ diff --git a/app/gen-server/entity/Login.ts b/app/gen-server/entity/Login.ts index 98b7d85862..516e4e8eea 100644 --- a/app/gen-server/entity/Login.ts +++ b/app/gen-server/entity/Login.ts @@ -1,4 +1,4 @@ -import {BaseEntity, Column, Entity, JoinColumn, ManyToOne, PrimaryColumn} from "typeorm"; +import {BaseEntity, Column, Entity, Index, JoinColumn, ManyToOne, PrimaryColumn} from "typeorm"; import {User} from "./User"; @@ -9,6 +9,7 @@ export class Login extends BaseEntity { public id: number; // This is the normalized email address we use for equality and indexing. + @Index() @Column({type: String}) public email: string; diff --git a/app/gen-server/lib/homedb/HomeDBManager.ts b/app/gen-server/lib/homedb/HomeDBManager.ts index 280177e7da..4fc4deb622 100644 --- a/app/gen-server/lib/homedb/HomeDBManager.ts +++ b/app/gen-server/lib/homedb/HomeDBManager.ts @@ -2016,30 +2016,28 @@ export class HomeDBManager extends EventEmitter { const result = await this._connection.transaction(async manager => { const analysis = await this._usersManager.verifyAndLookupDeltaEmails(userId, delta, false, manager); let {userIdDelta} = analysis; - let wsQuery = this._workspace(scope, wsId, { + const options = { manager, markPermissions: analysis.permissionThreshold, - }) - // Join the workspace's ACL rules and groups/users so we can edit them. - .leftJoinAndSelect('workspaces.aclRules', 'acl_rules') - .leftJoinAndSelect('acl_rules.group', 'workspace_groups') - .leftJoinAndSelect('workspace_groups.memberUsers', 'workspace_users') - // Join the workspace's org and org member groups so we know what should be inherited. - .leftJoinAndSelect('workspaces.org', 'org') - .leftJoinAndSelect('org.aclRules', 'org_acl_rules') - .leftJoinAndSelect('org_acl_rules.group', 'org_groups') - .leftJoinAndSelect('org_groups.memberUsers', 'org_users'); - wsQuery = this._addFeatures(wsQuery, 'org'); + }; + let wsQuery = this._buildWorkspaceWithACLRules(scope, wsId, options); wsQuery = this._withAccess(wsQuery, userId, 'workspaces'); - const queryResult = await verifyEntity(wsQuery); - if (queryResult.status !== 200) { + const wsQueryResult = await verifyEntity(wsQuery); + + if (wsQueryResult.status !== 200) { // If the query for the workspace failed, return the failure result. - return queryResult; + return wsQueryResult; } - this._failIfPowerfulAndChangingSelf(analysis, queryResult); - const ws: Workspace = queryResult.data; + this._failIfPowerfulAndChangingSelf(analysis, wsQueryResult); + const ws: Workspace = wsQueryResult.data; + + const orgId = ws.org.id; + let orgQuery = this._buildOrgWithACLRulesQuery(scope, orgId, options); + orgQuery = this._addFeatures(orgQuery); + const orgQueryResult = await orgQuery.getRawAndEntities(); + const org: Organization = orgQueryResult.entities[0]; // Get all the non-guest groups on the org. - const orgGroups = getNonGuestGroups(ws.org); + const orgGroups = getNonGuestGroups(org); // Get all the non-guest groups to be updated by the delta. const groups = getNonGuestGroups(ws); if ('maxInheritedRole' in delta) { @@ -2062,7 +2060,7 @@ export class HomeDBManager extends EventEmitter { await this._updateUserPermissions(groups, userIdDelta, manager); this._checkUserChangeAllowed(userId, groups); const nonOrgMembersAfter = this._usersManager.getUserDifference(groups, orgGroups); - const features = ws.org.billingAccount.getFeatures(); + const features = org.billingAccount.getFeatures(); const limit = features.maxSharesPerWorkspace; if (limit !== undefined) { this._restrictShares(null, limit, removeRole(nonOrgMembersBefore), @@ -2072,7 +2070,7 @@ export class HomeDBManager extends EventEmitter { await manager.save(groups); // If the users in workspace were changed, make a call to repair the guests in the org. if (userIdDelta) { - await this._repairOrgGuests(scope, ws.org.id, manager); + await this._repairOrgGuests(scope, orgId, manager); notifications.push(this._inviteNotification(userId, ws, userIdDelta, membersBefore)); } return { @@ -4652,9 +4650,8 @@ export class HomeDBManager extends EventEmitter { return { personal: true, public: !realAccess }; } - private _getWorkspaceWithACLRules(scope: Scope, wsId: number, options: Partial = {}) { - const query = this._workspace(scope, wsId, { - markPermissions: Permissions.VIEW, + private _buildWorkspaceWithACLRules(scope: Scope, wsId: number, options: Partial = {}) { + return this._workspace(scope, wsId, { ...options }) // Join the workspace's ACL rules (with 1st level groups/users listed). @@ -4664,6 +4661,13 @@ export class HomeDBManager extends EventEmitter { .leftJoinAndSelect('workspace_groups.memberGroups', 'workspace_group_groups') .leftJoinAndSelect('workspace_group_users.logins', 'workspace_user_logins') .leftJoinAndSelect('workspaces.org', 'org'); + } + + private _getWorkspaceWithACLRules(scope: Scope, wsId: number, options: Partial = {}) { + const query = this._buildWorkspaceWithACLRules(scope, wsId, { + markPermissions: Permissions.VIEW, + ...options + }); return verifyEntity(query); } diff --git a/app/gen-server/lib/homedb/UsersManager.ts b/app/gen-server/lib/homedb/UsersManager.ts index e2be5b1d21..d7fa907b77 100644 --- a/app/gen-server/lib/homedb/UsersManager.ts +++ b/app/gen-server/lib/homedb/UsersManager.ts @@ -337,15 +337,21 @@ export class UsersManager { email: string, manager?: EntityManager ): Promise { - const normalizedEmail = normalizeEmail(email); - return await (manager || this._connection).createQueryBuilder() - .select('user') - .from(User, 'user') - .leftJoinAndSelect('user.logins', 'logins') - .where('email = :email', {email: normalizedEmail}) + return await this._buildExistingUsersByLoginRequest([email], manager) .getOne() || undefined; } + /** + * Find some users by their emails. Don't create the users if they don't already exist. + */ + public async getExistingUsersByLogin( + emails: string[], + manager?: EntityManager + ): Promise { + return await this._buildExistingUsersByLoginRequest(emails, manager) + .getMany(); + } + /** * * Fetches a user record based on an email address. If a user record already @@ -471,7 +477,18 @@ export class UsersManager { }); } - /** + /* + * This function is an alias of getUserByLogin + * Its purpose is to be more expressive and avoid confusion when reading code. + * FIXME :In the future it may be used to split getUserByLogin in two distinct functions + * One for creation + * the other for retrieving users in order to make it more maintainable + */ + public async createUser(email: string, options: GetUserOptions = {}): Promise { + return await this.getUserByLogin(email, options); + } + + /* * Deletes a user from the database. For the moment, the only person with the right * to delete a user is the user themselves. * Users have logins, a personal org, and entries in the group_users table. All are @@ -612,11 +629,16 @@ export class UsersManager { // Lookup emails const emailMap = delta.users; const emails = Object.keys(emailMap); - const emailUsers = await Promise.all( - emails.map(async email => await this.getUserByLogin(email, {manager: transaction})) - ); - emails.forEach((email, i) => { - const userIdAffected = emailUsers[i]!.id; + const existingUsers = await this.getExistingUsersByLogin(emails, transaction); + const emailsExistingUsers = existingUsers.map(user => user.loginEmail); + const emailsUsersToCreate = emails.filter(email => !emailsExistingUsers.includes(email)); + const emailUsers = new Map(existingUsers.map(user => [user.loginEmail, user])); + for (const email of emailsUsersToCreate) { + const user = await this.createUser(email, {manager: transaction}); + emailUsers.set(user.loginEmail, user); + } + emails.forEach((email) => { + const userIdAffected = emailUsers.get(normalizeEmail(email))!.id; // Org-level sharing with everyone would allow serious spamming - forbid it. if (emailMap[email] !== null && // allow removing anything userId !== this.getSupportUserId() && // allow support user latitude @@ -766,4 +788,16 @@ export class UsersManager { } delta.users = users; } + + private _buildExistingUsersByLoginRequest( + emails: string[], + manager?: EntityManager + ) { + const normalizedEmails = emails.map(email=> normalizeEmail(email)); + return (manager || this._connection).createQueryBuilder() + .select('user') + .from(User, 'user') + .leftJoinAndSelect('user.logins', 'logins') + .where('email IN (:...emails)', {emails: normalizedEmails}); + } } diff --git a/app/gen-server/migration/1729754662550-LoginsEmailIndex.ts b/app/gen-server/migration/1729754662550-LoginsEmailIndex.ts new file mode 100644 index 0000000000..3435ec9971 --- /dev/null +++ b/app/gen-server/migration/1729754662550-LoginsEmailIndex.ts @@ -0,0 +1,16 @@ +import {MigrationInterface, QueryRunner, TableIndex} from "typeorm"; + +export class LoginsEmailsIndex1729754662550 implements MigrationInterface { + + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.createIndex("logins", new TableIndex({ + name: "logins__email", + columnNames: ["email"] + })); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.dropIndex("logins", "logins__email"); + } +} + diff --git a/app/server/lib/OIDCConfig.ts b/app/server/lib/OIDCConfig.ts index 9e500d6c4f..b14a269350 100644 --- a/app/server/lib/OIDCConfig.ts +++ b/app/server/lib/OIDCConfig.ts @@ -79,6 +79,7 @@ import { SendAppPageFunction } from 'app/server/lib/sendAppPage'; import { StringUnionError } from 'app/common/StringUnion'; import { EnabledProtection, EnabledProtectionString, ProtectionsManager } from './oidc/Protections'; import { SessionObj } from './BrowserSession'; +import { getOriginUrl } from './requestUtils'; const CALLBACK_URL = '/oauth2/callback'; @@ -278,18 +279,21 @@ export class OIDCConfig { }); } - public async getLogoutRedirectUrl(req: express.Request, redirectUrl: URL): Promise { + public async getLogoutRedirectUrl(req: express.Request): Promise { const session: SessionObj|undefined = (req as RequestWithLogin).session; + const stableRedirectUri = new URL('/signed-out', getOriginUrl(req)).href; // For IdPs that don't have end_session_endpoint, we just redirect to the logout page. if (this._skipEndSessionEndpoint) { - return redirectUrl.href; + // Ignore redirectUrl because OIDC providers don't allow variable redirect URIs + return stableRedirectUri; } // Alternatively, we could use a logout URL specified by configuration. if (this._endSessionEndpoint) { return this._endSessionEndpoint; } return this._client.endSessionUrl({ - post_logout_redirect_uri: redirectUrl.href, + // Ignore redirectUrl because OIDC providers don't allow variable redirect URIs + post_logout_redirect_uri: stableRedirectUri, id_token_hint: session?.oidc?.idToken, }); } diff --git a/static/locales/zh_Hans.client.json b/static/locales/zh_Hans.client.json index 97cbf4fdeb..415ea42652 100644 --- a/static/locales/zh_Hans.client.json +++ b/static/locales/zh_Hans.client.json @@ -36,7 +36,8 @@ "Remove column {{- colId }} from {{- tableId }} rules": "从 {{- tableId }} 规则中删除列 {{- colId }}", "When adding table rules, automatically add a rule to grant OWNER full access.": "添加表规则时，自动添加规则以授予 OWNER 完全访问权限。", "Allow editors to edit structure (e.g. modify and delete tables, columns, layouts), and to write formulas, which give access to all data regardless of read restrictions.": "允许编辑者编辑结构（例如修改和删除表、列、布局）和编写公式，无论读取限制如何，都可以访问所有数据。", - "This default should be changed if editors' access is to be limited. ": "如果要限制编辑者的访问权限，则应更改此默认值。 " + "This default should be changed if editors' access is to be limited. ": "如果要限制编辑者的访问权限，则应更改此默认值。 ", + "Add Table-wide Rule": "添加全表规则" }, "AccountPage": { "API": "API", @@ -212,7 +213,9 @@ "Legacy": "遗产", "Personal Site": "个人网站", "Team Site": "团队网站", - "Grist Templates": "Grist 模板" + "Grist Templates": "Grist 模板", + "Billing Account": "计费账户", + "Manage Team": "管理团队" }, "AppModel": { "This team site is suspended. Documents can be read, but not modified.": "此团队网站已挂起。可以读取文档，但不能修改文档。" diff --git a/test/gen-server/migrations.ts b/test/gen-server/migrations.ts index fc608d9e02..2795fd2529 100644 --- a/test/gen-server/migrations.ts +++ b/test/gen-server/migrations.ts @@ -47,6 +47,8 @@ import {UserLastConnection1713186031023 import {ActivationEnabled1722529827161 as ActivationEnabled} from 'app/gen-server/migration/1722529827161-Activation-Enabled'; import {Configs1727747249153 as Configs} from 'app/gen-server/migration/1727747249153-Configs'; +import {LoginsEmailsIndex1729754662550 + as LoginsEmailsIndex} from 'app/gen-server/migration/1729754662550-LoginsEmailIndex'; const home: HomeDBManager = new HomeDBManager(); @@ -56,7 +58,7 @@ const migrations = [Initial, Login, PinDocs, UserPicture, DisplayEmail, DisplayE ExternalBilling, DocOptions, Secret, UserOptions, GracePeriodStart, DocumentUsage, Activations, UserConnectId, UserUUID, UserUniqueRefUUID, Forks, ForkIndexes, ActivationPrefs, AssistantLimit, Shares, BillingFeatures, - UserLastConnection, ActivationEnabled, Configs]; + UserLastConnection, ActivationEnabled, Configs, LoginsEmailsIndex]; // Assert that the "members" acl rule and group exist (or not). function assertMembersGroup(org: Organization, exists: boolean) { diff --git a/test/server/lib/OIDCConfig.ts b/test/server/lib/OIDCConfig.ts index 1701489602..d7f46814d2 100644 --- a/test/server/lib/OIDCConfig.ts +++ b/test/server/lib/OIDCConfig.ts @@ -752,7 +752,7 @@ describe('OIDCConfig', () => { }); describe('getLogoutRedirectUrl', () => { - const REDIRECT_URL = new URL('http://localhost:8484/docs/signed-out'); + const STABLE_LOGOUT_URL = new URL('http://localhost:8484/signed-out'); const URL_RETURNED_BY_CLIENT = 'http://localhost:8484/logout_url_from_issuer'; const ENV_VALUE_GRIST_OIDC_IDP_END_SESSION_ENDPOINT = 'http://localhost:8484/logout'; const FAKE_SESSION = { @@ -767,7 +767,7 @@ describe('OIDCConfig', () => { env: { GRIST_OIDC_IDP_SKIP_END_SESSION_ENDPOINT: 'true', }, - expectedUrl: REDIRECT_URL.href, + expectedUrl: STABLE_LOGOUT_URL.href, }, { itMsg: 'should use the GRIST_OIDC_IDP_END_SESSION_ENDPOINT when it is set', env: { @@ -778,14 +778,14 @@ describe('OIDCConfig', () => { itMsg: 'should call the end session endpoint with the expected parameters', expectedUrl: URL_RETURNED_BY_CLIENT, expectedLogoutParams: { - post_logout_redirect_uri: REDIRECT_URL.href, + post_logout_redirect_uri: STABLE_LOGOUT_URL.href, id_token_hint: FAKE_SESSION.oidc!.idToken, } }, { itMsg: 'should call the end session endpoint with no idToken if session is missing', expectedUrl: URL_RETURNED_BY_CLIENT, expectedLogoutParams: { - post_logout_redirect_uri: REDIRECT_URL.href, + post_logout_redirect_uri: STABLE_LOGOUT_URL.href, id_token_hint: undefined, }, session: null @@ -798,9 +798,12 @@ describe('OIDCConfig', () => { clientStub.endSessionUrl.returns(URL_RETURNED_BY_CLIENT); const config = await OIDCConfigStubbed.buildWithStub(clientStub.asClient()); const req = { + headers: { + host: STABLE_LOGOUT_URL.host + }, session: 'session' in ctx ? ctx.session : FAKE_SESSION } as unknown as RequestWithLogin; - const url = await config.getLogoutRedirectUrl(req, REDIRECT_URL); + const url = await config.getLogoutRedirectUrl(req); assert.equal(url, ctx.expectedUrl); if (ctx.expectedLogoutParams) { assert.isTrue(clientStub.endSessionUrl.calledOnce);