Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/develop' into release-1.3.0
Browse files Browse the repository at this point in the history
  • Loading branch information
Half-Shot committed May 12, 2020
2 parents 622aa0b + 6021ee5 commit d933716
Show file tree
Hide file tree
Showing 17 changed files with 89 additions and 53 deletions.
2 changes: 1 addition & 1 deletion .dockerignore
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,4 @@ node_modules

# typescript
lib

tsconfig.tsbuildinfo
16 changes: 8 additions & 8 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
FROM node:current-alpine AS BUILD
COPY . /tmp/src
FROM node:14-alpine AS BUILD
COPY . /src

# git is needed to install Half-Shot/slackdown
RUN apk add git
RUN cd /tmp/src \
&& npm install \
&& npm run build
WORKDIR /src
RUN npm install
RUN npm run build

FROM node:current-alpine
FROM node:14-alpine

VOLUME /data/ /config/

COPY package.json /usr/src/app/
COPY package-lock.json /usr/src/app/

COPY --from=BUILD /tmp/src/config /usr/src/app/config
COPY --from=BUILD /tmp/src/lib /usr/src/app/lib
COPY --from=BUILD /src/config /usr/src/app/config
COPY --from=BUILD /src/lib /usr/src/app/lib

WORKDIR /usr/src/app

Expand Down
1 change: 1 addition & 0 deletions changelog.d/403.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Allow bridging to private channels via the provisioner
1 change: 1 addition & 0 deletions changelog.d/404.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix exception on missing `error` in createTeamClient
1 change: 1 addition & 0 deletions changelog.d/405.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix postgress configurations failing to start when using the offical docker image.
1 change: 1 addition & 0 deletions changelog.d/408.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Bridge will no longer update user's displayname with a bots name when a bot is modified
2 changes: 2 additions & 0 deletions config/config.sample.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ provisioning:
enabled: true
# Should the bridge deny users bridging channels to private rooms.
require_public_room: true
# Should the bridge allow usesr to bridge private channels.
allow_private_channels: true
limits:
room_count: 20
team_count: 1
Expand Down
2 changes: 2 additions & 0 deletions config/slack-config-schema.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ properties:
type: boolean
require_public_room:
type: boolean
allow_private_channels:
type: boolean
limits:
type: object
properties:
Expand Down
48 changes: 24 additions & 24 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"description": "A Matrix <--> Slack bridge",
"main": "app.js",
"scripts": {
"postinstall": "npm run build",
"prepare": "npm run build",
"start": "node ./lib/app.js",
"build": "tsc",
"test": "npm run test:unit && npm run test:integration",
Expand Down Expand Up @@ -40,7 +40,7 @@
"nedb": "^1.8.0",
"node-emoji": "^1.10.0",
"p-queue": "^6.3.0",
"pg-promise": "^10.4.4",
"pg-promise": "^10.5.3",
"quick-lru": "^5.0.0",
"randomstring": "^1",
"request-promise-native": "^1.0.8",
Expand Down
1 change: 1 addition & 0 deletions src/IConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ export interface IConfig {
provisioning?: {
enable: boolean;
require_public_room?: boolean;
allow_private_channels?: boolean;
limits?: {
team_count?: number;
room_count?: number;
Expand Down
4 changes: 3 additions & 1 deletion src/Main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,9 @@ export class Main {
this.clientfactory = new SlackClientFactory(this.datastore, this.config, (method: string) => {
this.incRemoteCallCounter(method);
}, (teamId: string, delta: number) => {
this.metricPuppets.inc({ team_id: teamId }, delta);
if (this.metricPuppets) {
this.metricPuppets.inc({ team_id: teamId }, delta);
}
});
let puppetsWaiting: Promise<unknown> = Promise.resolve();
if (this.slackRtm) {
Expand Down
36 changes: 26 additions & 10 deletions src/Provisioning.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,21 @@ export class Provisioner {
return (currentCount >= this.main.config.provisioning?.limits?.room_count);
}

private async determineSlackIdForRequest(matrixUserId, teamId) {
const matrixUser = await this.main.datastore.getMatrixUser(matrixUserId);
if (!matrixUser) {
// No Slack user entry found for MXID
return null;
}
const accounts: {[userId: string]: {team_id: string}} = matrixUser.get("accounts");
for (const [key, value] of Object.entries(accounts)) {
if (value.team_id === teamId) {
return key;
}
}
return null;
}

@command()
private async getconfig(_, res) {
const hasRoomLimit = this.main.config.provisioning?.limits?.room_count;
Expand Down Expand Up @@ -150,25 +165,26 @@ export class Provisioner {
@command("user_id", "team_id")
private async channels(req, res, userId, teamId) {
log.debug(`${userId} for ${teamId} requested their channels`);
const matrixUser = await this.main.datastore.getMatrixUser(userId);
const isAllowed = matrixUser !== null &&
Object.values(matrixUser.get("accounts") as {[key: string]: {team_id: string}}).find((acct) =>
acct.team_id === teamId,
);
if (!isAllowed) {
res.status(HTTP_CODES.CLIENT_ERROR).json({error: "User is not part of this team!"});
throw undefined;
const slackUserId = await this.determineSlackIdForRequest(userId, teamId);
if (!slackUserId) {
return res.status(HTTP_CODES.CLIENT_ERROR).json({error: "User is not part of this team!"});
}
const team = await this.main.datastore.getTeam(teamId);
if (team === null) {
throw Error("No team token for this team_id");
}
const cli = await this.main.clientFactory.getTeamClient(teamId);
try {
const response = (await cli.conversations.list({
let types = "public_channel";
// Unless we *explicity* set this to false, allow it.
if (this.main.config.provisioning?.allow_private_channels !== false) {
types = `public_channel,private_channel`;
}
const response = (await cli.users.list({
exclude_archived: true,
limit: 1000, // TODO: Pagination
types: "public_channel", // TODO: In order to show private channels, we need the identity of the caller.
user: slackUserId, // In order to show private channels, we need the identity of the caller.
types,
})) as ConversationsListResponse;
if (!response.ok) {
throw Error(response.error);
Expand Down
3 changes: 2 additions & 1 deletion src/SlackClientFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,8 @@ export class SlackClientFactory {
}
return { slackClient, team: teamInfo.team, auth, user };
} catch (ex) {
throw Error("Could not create team client: " + ex.data.error);
log.error("Could not create team client: " + (ex.data?.error || ex));
throw Error("Could not create team client");
}
}
}
12 changes: 10 additions & 2 deletions src/SlackGhost.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,11 @@ export class SlackGhost {
let displayName = message.username || message.user_name;

if (room.SlackClient) { // We can be smarter if we have the bot.
if (message.bot_id) {
if (message.bot_id && message.user_id) {
// In the case of operations on bots, we will have both a bot_id and a user_id.
// Ignore updating the displayname in this case.
return;
} else if (message.bot_id) {
displayName = await this.getBotName(message.bot_id, room.SlackClient);
} else if (message.user_id) {
displayName = await this.getDisplayname(room.SlackClient);
Expand Down Expand Up @@ -245,7 +249,11 @@ export class SlackGhost {
}
let avatarUrl;
let hash: string|undefined;
if (message.bot_id) {
if (message.bot_id && message.user_id) {
// In the case of operations on bots, we will have both a bot_id and a user_id.
// Ignore updating the displayname in this case.
return;
} else if (message.bot_id) {
avatarUrl = await this.getBotAvatarUrl(message.bot_id, room.SlackClient);
hash = avatarUrl;
} else if (message.user_id) {
Expand Down
4 changes: 2 additions & 2 deletions src/datastore/postgres/PgDatastore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,15 @@ export class PgDatastore implements Datastore {
});
}

public async getUser(id: string): Promise<UserEntry | null> {
public async getUser(id: string): Promise<UserEntry|null> {
const dbEntry = await this.postgresDb.oneOrNone("SELECT * FROM users WHERE userId = ${id}", { id });
if (!dbEntry) {
return null;
}
return JSON.parse(dbEntry.json);
}

public async getMatrixUser(userId: string): Promise<MatrixUser|undefined> {
public async getMatrixUser(userId: string): Promise<MatrixUser|null> {
userId = new MatrixUser(userId).getId(); // Ensure ID correctness
const userData = await this.getUser(userId);
return userData !== null ? new MatrixUser(userId, userData) : null;
Expand Down
4 changes: 2 additions & 2 deletions src/tests/unit/SlackClientFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ describe("SlackClientFactory", () => {
await factory.getTeamClient("faketeam");
throw Error("Call didn't throw as expected");
} catch (ex) {
expect(ex.message).to.equal("Could not create team client: Team not allowed for test");
expect(ex.message).to.equal("Could not create team client");
expect(ds.teams[0].status).to.equal("bad_auth");
}
});
Expand All @@ -146,7 +146,7 @@ describe("SlackClientFactory", () => {
await factory.getTeamClient("faketeam");
throw Error("Call didn't throw as expected");
} catch (ex) {
expect(ex.message).to.equal("Could not create team client: Team not allowed for test");
expect(ex.message).to.equal("Could not create team client");
expect(ds.teams[0].status).to.equal("bad_auth");
}
});
Expand Down

0 comments on commit d933716

Please sign in to comment.