Skip to content

Commit 8f17e2f

Browse files
seratchseratch
committed
Fix #468 by implementing short-time cache for authorization middleware
1 parent 6cd2da3 commit 8f17e2f

File tree

5 files changed

+496
-2
lines changed

5 files changed

+496
-2
lines changed

‎bolt/src/main/java/com/slack/api/bolt/AppConfig.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,12 @@ public boolean isDistributedApp() {
6969
@Builder.Default
7070
private boolean oAuthCallbackEnabled = false;
7171

72+
73+
@Builder.Default
74+
private boolean authTestCacheEnabled = false;
75+
@Builder.Default
76+
private long authTestCacheExpirationMillis = 3000L;
77+
7278
@Builder.Default
7379
// https://api.slack.com/authentication/migration
7480
private boolean classicAppPermissionsEnabled = false;

‎bolt/src/main/java/com/slack/api/bolt/middleware/builtin/MultiTeamsAuthorization.java‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,18 @@
1111
import com.slack.api.bolt.response.Responder;
1212
import com.slack.api.bolt.response.Response;
1313
import com.slack.api.bolt.service.InstallationService;
14+
import com.slack.api.methods.MethodsClient;
1415
import com.slack.api.methods.SlackApiException;
1516
import com.slack.api.methods.response.auth.AuthTestResponse;
1617
import com.slack.api.model.block.LayoutBlock;
18+
import lombok.AllArgsConstructor;
19+
import lombok.Data;
1720
import lombok.extern.slf4j.Slf4j;
1821

1922
import java.io.IOException;
2023
import java.util.List;
24+
import java.util.concurrent.ConcurrentHashMap;
25+
import java.util.concurrent.ConcurrentMap;
2126

2227
import static com.slack.api.bolt.middleware.MiddlewareOps.isNoAuthRequiredRequest;
2328
import static com.slack.api.bolt.response.ResponseTypes.ephemeral;
@@ -31,6 +36,16 @@ public class MultiTeamsAuthorization implements Middleware {
3136
private final AppConfig config;
3237
private final InstallationService installationService;
3338

39+
@Data
40+
@AllArgsConstructor
41+
static class CachedAuthTestResponse {
42+
private AuthTestResponse response;
43+
private long cachedMillis;
44+
}
45+
46+
// token -> auth.test response
47+
private final ConcurrentMap<String, CachedAuthTestResponse> tokenToAuthTestCache = new ConcurrentHashMap<>();
48+
3449
private boolean alwaysRequestUserTokenNeeded;
3550

3651
public boolean isAlwaysRequestUserTokenNeeded() {
@@ -110,7 +125,7 @@ public Response apply(Request req, Response resp, MiddlewareChain chain) throws
110125

111126
try {
112127
String token = botToken != null ? botToken : userToken;
113-
AuthTestResponse authTestResponse = context.client().authTest(r -> r.token(token));
128+
AuthTestResponse authTestResponse = callAuthTest(token, config, context.client());
114129
if (authTestResponse.isOk()) {
115130
context.setBotToken(botToken);
116131
context.setRequestUserToken(userToken);
@@ -131,6 +146,24 @@ public Response apply(Request req, Response resp, MiddlewareChain chain) throws
131146
}
132147
}
133148

149+
protected AuthTestResponse callAuthTest(String token, AppConfig config, MethodsClient client) throws IOException, SlackApiException {
150+
if (config.isAuthTestCacheEnabled()) {
151+
CachedAuthTestResponse cachedResponse = tokenToAuthTestCache.get(token);
152+
if (cachedResponse != null) {
153+
long millisToExpire = cachedResponse.getCachedMillis() + config.getAuthTestCacheExpirationMillis();
154+
if (millisToExpire > System.currentTimeMillis()) {
155+
return cachedResponse.getResponse();
156+
}
157+
}
158+
AuthTestResponse response = client.authTest(r -> r.token(token));
159+
CachedAuthTestResponse newCache = new CachedAuthTestResponse(response, System.currentTimeMillis());
160+
tokenToAuthTestCache.put(token, newCache);
161+
return response;
162+
} else {
163+
return client.authTest(r -> r.token(token));
164+
}
165+
}
166+
134167
protected Response handleAuthTestError(
135168
String errorCode,
136169
Bot foundBot,

‎bolt/src/main/java/com/slack/api/bolt/middleware/builtin/SingleTeamAuthorization.java‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,15 @@
88
import com.slack.api.bolt.request.Request;
99
import com.slack.api.bolt.response.Response;
1010
import com.slack.api.bolt.service.InstallationService;
11+
import com.slack.api.methods.MethodsClient;
12+
import com.slack.api.methods.SlackApiException;
1113
import com.slack.api.methods.response.auth.AuthTestResponse;
1214
import lombok.extern.slf4j.Slf4j;
1315

16+
import java.io.IOException;
17+
import java.util.Optional;
18+
import java.util.concurrent.atomic.AtomicLong;
19+
1420
import static com.slack.api.bolt.middleware.MiddlewareOps.isNoAuthRequiredRequest;
1521

1622
/**
@@ -22,6 +28,9 @@ public class SingleTeamAuthorization implements Middleware {
2228
private final AppConfig appConfig;
2329
private final InstallationService installationService;
2430

31+
private Optional<AuthTestResponse> cachedAuthTestResponse = Optional.empty();
32+
private AtomicLong lastCachedMillis = new AtomicLong(0L);
33+
2534
public SingleTeamAuthorization(AppConfig appConfig, InstallationService installationService) {
2635
this.appConfig = appConfig;
2736
this.installationService = installationService;
@@ -34,7 +43,7 @@ public Response apply(Request req, Response resp, MiddlewareChain chain) throws
3443
}
3544

3645
Context context = req.getContext();
37-
AuthTestResponse authResult = context.client().authTest(r -> r.token(appConfig.getSingleTeamBotToken()));
46+
AuthTestResponse authResult = callAuthTest(appConfig, context.client());
3847
if (authResult.isOk()) {
3948
if (context.getBotToken() == null) {
4049
context.setBotToken(appConfig.getSingleTeamBotToken());
@@ -72,4 +81,20 @@ public Response apply(Request req, Response resp, MiddlewareChain chain) throws
7281
.build();
7382
}
7483
}
84+
85+
protected AuthTestResponse callAuthTest(AppConfig config, MethodsClient client) throws IOException, SlackApiException {
86+
if (config.isAuthTestCacheEnabled()) {
87+
long millisToExpire = lastCachedMillis.get() + config.getAuthTestCacheExpirationMillis();
88+
if (cachedAuthTestResponse.isPresent() && millisToExpire > System.currentTimeMillis()) {
89+
return cachedAuthTestResponse.get();
90+
}
91+
AuthTestResponse response = client.authTest(r -> r.token(config.getSingleTeamBotToken()));
92+
cachedAuthTestResponse = Optional.of(response); // response here is not null for sure
93+
lastCachedMillis.set(System.currentTimeMillis());
94+
return response;
95+
} else {
96+
return client.authTest(r -> r.token(config.getSingleTeamBotToken()));
97+
}
98+
}
99+
75100
}

‎bolt/src/test/java/test_locally/app/MultiTeamsAuthTestCacheTest.java‎

Lines changed: 234 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,234 @@
1+
package test_locally.app;
2+
3+
import com.slack.api.Slack;
4+
import com.slack.api.SlackConfig;
5+
import com.slack.api.app_backend.SlackSignature;
6+
import com.slack.api.bolt.App;
7+
import com.slack.api.bolt.AppConfig;
8+
import com.slack.api.bolt.model.Bot;
9+
import com.slack.api.bolt.model.Installer;
10+
import com.slack.api.bolt.model.builtin.DefaultBot;
11+
import com.slack.api.bolt.model.builtin.DefaultInstaller;
12+
import com.slack.api.bolt.request.RequestHeaders;
13+
import com.slack.api.bolt.request.builtin.GlobalShortcutRequest;
14+
import com.slack.api.bolt.response.Response;
15+
import com.slack.api.bolt.service.InstallationService;
16+
import com.slack.api.methods.SlackApiException;
17+
import lombok.extern.slf4j.Slf4j;
18+
import org.eclipse.jetty.server.Server;
19+
import org.eclipse.jetty.servlet.ServletHandler;
20+
import org.junit.AfterClass;
21+
import org.junit.BeforeClass;
22+
import org.junit.Test;
23+
import util.PortProvider;
24+
25+
import javax.servlet.annotation.WebServlet;
26+
import javax.servlet.http.HttpServlet;
27+
import javax.servlet.http.HttpServletRequest;
28+
import javax.servlet.http.HttpServletResponse;
29+
import java.io.IOException;
30+
import java.net.URLEncoder;
31+
import java.util.Arrays;
32+
import java.util.HashMap;
33+
import java.util.List;
34+
import java.util.Map;
35+
import java.util.concurrent.ConcurrentHashMap;
36+
import java.util.concurrent.ConcurrentMap;
37+
import java.util.concurrent.atomic.AtomicInteger;
38+
39+
import static org.junit.Assert.assertEquals;
40+
import static org.junit.Assert.fail;
41+
42+
@Slf4j
43+
public class MultiTeamsAuthTestCacheTest {
44+
45+
static SlackConfig config = new SlackConfig();
46+
static Slack slack = Slack.getInstance(config);
47+
static CountdownAuthTestServer server = new CountdownAuthTestServer();
48+
49+
final String secret = "foo-bar-baz";
50+
final SlackSignature.Generator generator = new SlackSignature.Generator(secret);
51+
52+
String realPayload = "{\"type\":\"shortcut\",\"token\":\"legacy-fixed-value\",\"action_ts\":\"123.123\",\"team\":{\"id\":\"T123\",\"domain\":\"seratch-test\"},\"user\":{\"id\":\"U123\",\"username\":\"seratch\",\"team_id\":\"T123\"},\"callback_id\":\"test-global-shortcut\",\"trigger_id\":\"123.123.123\"}\n";
53+
54+
@BeforeClass
55+
public static void setUp() throws Exception {
56+
server.start();
57+
config.setMethodsEndpointUrlPrefix(server.getMethodsEndpointPrefix());
58+
}
59+
60+
@AfterClass
61+
public static void tearDown() throws Exception {
62+
server.stop();
63+
}
64+
65+
@Test
66+
public void cacheEnabled() throws Exception {
67+
App app = buildApp(true);
68+
app.globalShortcut("test-global-shortcut", (req, ctx) -> ctx.ack());
69+
70+
String requestBody = "payload=" + URLEncoder.encode(realPayload, "UTF-8");
71+
72+
Map<String, List<String>> rawHeaders = new HashMap<>();
73+
String timestamp = String.valueOf(System.currentTimeMillis() / 1000);
74+
setRequestHeaders(requestBody, rawHeaders, timestamp);
75+
76+
GlobalShortcutRequest req = new GlobalShortcutRequest(requestBody, realPayload, new RequestHeaders(rawHeaders));
77+
assertEquals("seratch", req.getPayload().getUser().getUsername()); // a bit different from message_actions payload
78+
79+
Response response = app.run(req);
80+
assertEquals(200L, response.getStatusCode().longValue());
81+
82+
response = app.run(req);
83+
assertEquals(200L, response.getStatusCode().longValue());
84+
85+
Thread.sleep(300L);
86+
response = app.run(req);
87+
assertEquals(200L, response.getStatusCode().longValue());
88+
89+
Thread.sleep(300L);
90+
response = app.run(req);
91+
assertEquals(200L, response.getStatusCode().longValue());
92+
93+
Thread.sleep(3000L);
94+
95+
response = app.run(req);
96+
assertEquals(503L, response.getStatusCode().longValue());
97+
}
98+
99+
@Test
100+
public void cacheDisabled() throws Exception {
101+
App app = buildApp(false);
102+
app.globalShortcut("test-global-shortcut", (req, ctx) -> ctx.ack());
103+
104+
String requestBody = "payload=" + URLEncoder.encode(realPayload, "UTF-8");
105+
106+
Map<String, List<String>> rawHeaders = new HashMap<>();
107+
String timestamp = String.valueOf(System.currentTimeMillis() / 1000);
108+
setRequestHeaders(requestBody, rawHeaders, timestamp);
109+
110+
GlobalShortcutRequest req = new GlobalShortcutRequest(requestBody, realPayload, new RequestHeaders(rawHeaders));
111+
assertEquals("seratch", req.getPayload().getUser().getUsername()); // a bit different from message_actions payload
112+
113+
Response response = app.run(req);
114+
assertEquals(200L, response.getStatusCode().longValue());
115+
116+
response = app.run(req);
117+
assertEquals(503L, response.getStatusCode().longValue());
118+
}
119+
120+
App buildApp(boolean authTestCacheEnabled) {
121+
App app = new App(AppConfig.builder()
122+
.authTestCacheEnabled(authTestCacheEnabled)
123+
.signingSecret(secret)
124+
.clientId("test")
125+
.clientSecret("test-test")
126+
.slack(slack)
127+
.build());
128+
app.service(new InstallationService() {
129+
@Override
130+
public boolean isHistoricalDataEnabled() {
131+
return false;
132+
}
133+
134+
@Override
135+
public void setHistoricalDataEnabled(boolean isHistoricalDataEnabled) {
136+
}
137+
138+
@Override
139+
public void saveInstallerAndBot(Installer installer) {
140+
}
141+
142+
@Override
143+
public void deleteBot(Bot bot) {
144+
}
145+
146+
@Override
147+
public void deleteInstaller(Installer installer) {
148+
}
149+
150+
@Override
151+
public Bot findBot(String enterpriseId, String teamId) {
152+
DefaultBot bot = new DefaultBot();
153+
bot.setTeamId("T111");
154+
bot.setBotAccessToken("B111");
155+
bot.setBotUserId("U111");
156+
bot.setInstalledAt(System.currentTimeMillis());
157+
bot.setBotAccessToken("xoxb-1234567890-123456789012-12345678901234567890" + authTestCacheEnabled);
158+
return bot;
159+
}
160+
161+
@Override
162+
public Installer findInstaller(String enterpriseId, String teamId, String userId) {
163+
return null;
164+
}
165+
});
166+
return app;
167+
}
168+
169+
void setRequestHeaders(String requestBody, Map<String, List<String>> rawHeaders, String timestamp) {
170+
rawHeaders.put(SlackSignature.HeaderNames.X_SLACK_REQUEST_TIMESTAMP, Arrays.asList(timestamp));
171+
rawHeaders.put(SlackSignature.HeaderNames.X_SLACK_SIGNATURE, Arrays.asList(generator.generate(timestamp, requestBody)));
172+
}
173+
174+
public static class CountdownAuthTestServer {
175+
176+
static String ok = "{\n" +
177+
" \"ok\": true,\n" +
178+
" \"url\": \"https://java-slack-sdk-test.slack.com/\",\n" +
179+
" \"team\": \"java-slack-sdk-test\",\n" +
180+
" \"user\": \"test_user\",\n" +
181+
" \"team_id\": \"T1234567\",\n" +
182+
" \"user_id\": \"U1234567\",\n" +
183+
" \"bot_id\": \"B12345678\",\n" +
184+
" \"enterprise_id\": \"E12345678\"\n" +
185+
"}";
186+
187+
@WebServlet
188+
public static class CountdownAuthTestMockEndpoint extends HttpServlet {
189+
190+
private ConcurrentMap<String, AtomicInteger> remaining = new ConcurrentHashMap<>();
191+
192+
@Override
193+
protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException {
194+
String token = req.getHeader("Authorization").replaceFirst("Bearer ", "");
195+
int count = remaining.computeIfAbsent(token, (t) -> new AtomicInteger(2)).getAndDecrement();
196+
if (count <= 0) {
197+
resp.setStatus(500);
198+
return;
199+
}
200+
resp.setStatus(200);
201+
resp.setContentType("application/json");
202+
resp.getWriter().write(ok);
203+
}
204+
}
205+
206+
private final int port;
207+
private final Server server;
208+
209+
public CountdownAuthTestServer() {
210+
this(PortProvider.getPort(CountdownAuthTestServer.class.getName()));
211+
}
212+
213+
public CountdownAuthTestServer(int port) {
214+
this.port = port;
215+
server = new Server(this.port);
216+
ServletHandler handler = new ServletHandler();
217+
server.setHandler(handler);
218+
handler.addServletWithMapping(CountdownAuthTestMockEndpoint.class, "/*");
219+
}
220+
221+
public String getMethodsEndpointPrefix() {
222+
return "http://localhost:" + port + "/api/";
223+
}
224+
225+
public void start() throws Exception {
226+
server.start();
227+
}
228+
229+
public void stop() throws Exception {
230+
server.stop();
231+
}
232+
}
233+
234+
}

0 commit comments

Comments
 (0)