diff --git a/apps/draupnir/src/appservice/AccessControl.ts b/apps/draupnir/src/appservice/AccessControl.ts index 6e8d748..b51c8f2 100644 --- a/apps/draupnir/src/appservice/AccessControl.ts +++ b/apps/draupnir/src/appservice/AccessControl.ts @@ -75,8 +75,9 @@ export class AccessControl { return MPSAccess.getAccessForUser( this.accessControlRevisionIssuer.currentRevision, mxid, - // Appservice provisioning should be gated by explicit user allow rules. - "CHECK_SERVER" + // Appservice provisioning is gated by explicit user allow rules only. + // Do not allow server-level rules to implicitly permit provisioning. + "IGNORE_SERVER" ); } diff --git a/apps/draupnir/src/appservice/AppService.ts b/apps/draupnir/src/appservice/AppService.ts index 3a48ce9..2235954 100644 --- a/apps/draupnir/src/appservice/AppService.ts +++ b/apps/draupnir/src/appservice/AppService.ts @@ -290,6 +290,14 @@ export class MjolnirAppService { `Failed to provision a draupnir for ${mxEvent.sender} after they invited ${this.bridge.botUserId}`, result.error ); + await this.bridge + .getBot() + .getClient() + .sendText( + mxEvent.room_id, + "Please make sure you are allowed to provision a bot. Otherwise please notify the admin. The provisioning request was rejected." + ); + return; } // Send a notice that the invite must be accepted await this.bridge diff --git a/packages/matrix-protection-suite/src/Protection/AccessControl.ts b/packages/matrix-protection-suite/src/Protection/AccessControl.ts index 912f767..fe14d5d 100644 --- a/packages/matrix-protection-suite/src/Protection/AccessControl.ts +++ b/packages/matrix-protection-suite/src/Protection/AccessControl.ts @@ -34,6 +34,10 @@ export interface EntityAccess { readonly rule?: PolicyRule; } +export type AllowRulesPolicy = + | "ALLOW_IF_NO_EXPLICIT_ALLOW_RULE" + | "REQUIRE_EXPLICIT_ALLOW_RULE"; + /** * This allows us to work out the access an entity has to some thing based on a set of watched/unwatched lists. */ @@ -65,12 +69,17 @@ export class AccessControl { userID: StringUserID, policy: "CHECK_SERVER" | "IGNORE_SERVER" ): EntityAccess { + const allowRulesPolicy: AllowRulesPolicy = + policy === "IGNORE_SERVER" + ? "REQUIRE_EXPLICIT_ALLOW_RULE" + : "ALLOW_IF_NO_EXPLICIT_ALLOW_RULE"; const userAccess = AccessControl.getAccessForEntity( revision, userID, - PolicyRuleType.User + PolicyRuleType.User, + allowRulesPolicy ); - if (policy === "IGNORE_SERVER" || userAccess.outcome === Access.Banned) { + if (policy === "IGNORE_SERVER" || userAccess.outcome !== Access.Allowed) { return userAccess; } else { const serverAccess = AccessControl.getAccessForEntity( @@ -78,21 +87,17 @@ export class AccessControl { userServerName(userID), PolicyRuleType.Server ); - if ( - userAccess.outcome === Access.Allowed && - serverAccess.outcome === Access.NotAllowed - ) { - return userAccess; - } else { - return serverAccess; - } + // CHECK_SERVER applies server bans while keeping explicit user allow as + // the source of truth for who is allowed to provision. + return serverAccess.outcome === Access.Banned ? serverAccess : userAccess; } } public static getAccessForEntity( revision: PolicyListRevision, entity: string, - entityType: PolicyRuleType + entityType: PolicyRuleType, + allowRulesPolicy: AllowRulesPolicy = "ALLOW_IF_NO_EXPLICIT_ALLOW_RULE" ): EntityAccess { // Check if the entity is explicitly allowed. // We have to infer that a rule exists for '*' if the allowCache is empty, otherwise you brick the ACL. @@ -101,10 +106,11 @@ export class AccessControl { recommendation: Recommendation.Allow, searchHashedRules: false, }); + const hasAllowRules = + revision.allRulesOfType(entityType, Recommendation.Allow).length !== 0; if ( allowRule === undefined && - // this is gonna be a pita resource wise. - !(revision.allRulesOfType(entityType, Recommendation.Allow).length === 0) + (hasAllowRules || allowRulesPolicy === "REQUIRE_EXPLICIT_ALLOW_RULE") ) { return { outcome: Access.NotAllowed }; }