Skip to content

Commit ef996ea

Browse files
committed
remove sig validation after discovering slack doesn't sign oauth reqs
Keeping signature checking logic in `Slack.has_valid_signature` for future use
1 parent 6f2d59b commit ef996ea

File tree

4 files changed

+10
-32
lines changed

4 files changed

+10
-32
lines changed

lib/action.ml

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -233,15 +233,12 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct
233233
log#error "%s" msg;
234234
Lwt.return_unit
235235

236-
let process_slack_oauth (ctx : Context.t) args headers body =
236+
let process_slack_oauth (ctx : Context.t) args =
237237
try%lwt
238238
let secrets = Context.get_secrets_exn ctx in
239239
match secrets.slack_access_token with
240240
| Some _ -> Lwt.return_unit
241241
| None ->
242-
match Slack.has_valid_signature ?signing_key:secrets.slack_signing_secret ~headers body with
243-
| false -> action_error "failed to verify signature of slack authorization request"
244-
| true ->
245242
match Slack.has_valid_state ?oauth_state:secrets.slack_oauth_state ~args with
246243
| false -> action_error "expected state not found in slack authorization request"
247244
| true ->

src/request_handler.ml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ let setup_http ~ctx ~signature ~port ~ip =
4040
ret (Lwt.return "ok")
4141
| _, [ "slack" ] ->
4242
log#info "%s" request.body;
43-
let%lwt () = Action.process_slack_oauth ctx request.args request.headers request.body in
43+
let%lwt () = Action.process_slack_oauth ctx request.args in
4444
ret (Lwt.return "ok")
4545
| _, _ ->
4646
log#error "unknown path : %s" (Httpev.show_request request);

test/slack_oauth.expected

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,8 @@
11
===== slack oauth: success, valid params ====
22
will generate token
3-
===== slack oauth: success, no signing key ====
4-
will generate token
53
===== slack oauth: success, with state ====
64
will generate token
75
===== slack oauth: no-op, token exists ====
86
===== slack oauth: failure, no state arg ====
97
===== slack oauth: failure, bad state arg ====
108
===== slack oauth: failure, no code arg ====
11-
===== slack oauth: failure, bad timestamp ====
12-
===== slack oauth: failure, no signature ====
13-
===== slack oauth: failure, no timestamp ====

test/slack_oauth_test.ml

Lines changed: 8 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,6 @@ open Base
22
open Lib
33
module Action = Action.Action (Api_local.Github) (Api_local.Slack)
44

5-
let signing_key = "8f742231b10e8888abcd99yyyzzz85a5"
6-
7-
let sig_header = "x-slack-signature", "v0=abb033014f155dcde840b8dc150b1fb12433b8f8233ffc2853be34e151bad9e8"
8-
9-
let ts_valid = "x-slack-request-timestamp", "1531420618"
10-
11-
let ts_invalid = "x-slack-request-timestamp", "5000000000"
12-
135
let arg_code = "code", "1591663521684.1613458437648.4a18cf683e541ff9d8fd75075181cac49c7acae9431d7e4ffd424ce1ca8d2543"
146

157
let arg_state = "state", "foo"
@@ -27,30 +19,24 @@ let secrets_default : Config_t.secrets =
2719
slack_access_token = None;
2820
}
2921

30-
let secrets_with_key = { secrets_default with slack_signing_secret = Some signing_key }
31-
3222
let secrets_with_state = { secrets_default with slack_oauth_state = Some "foo" }
3323

3424
let secrets_with_token = { secrets_default with slack_access_token = Some "123" }
3525

3626
let tests =
3727
[
38-
"success, valid params", [ arg_code ], [ sig_header; ts_valid ], "text=hello", secrets_with_key;
39-
"success, no signing key", [ arg_code ], [ sig_header; ts_valid ], "text=hello", secrets_default;
40-
"success, with state", [ arg_code; arg_state ], [ sig_header; ts_valid ], "text=hello", secrets_with_state;
41-
"no-op, token exists", [ arg_code ], [ sig_header; ts_valid ], "text=hello", secrets_with_token;
42-
"failure, no state arg", [ arg_code ], [ sig_header; ts_valid ], "text=hello", secrets_with_state;
43-
"failure, bad state arg", [ arg_code; arg_state_bad ], [ sig_header; ts_valid ], "text=hello", secrets_with_state;
44-
"failure, no code arg", [], [ sig_header; ts_valid ], "text=hello", secrets_with_key;
45-
"failure, bad timestamp", [ arg_code ], [ sig_header; ts_invalid ], "text=hello", secrets_with_key;
46-
"failure, no signature", [ arg_code ], [ ts_valid ], "text=hello", secrets_with_key;
47-
"failure, no timestamp", [ arg_code ], [ sig_header ], "text=hello", secrets_with_key;
28+
"success, valid params", [ arg_code ], secrets_default;
29+
"success, with state", [ arg_code; arg_state ], secrets_with_state;
30+
"no-op, token exists", [ arg_code ], secrets_with_token;
31+
"failure, no state arg", [ arg_code ], secrets_with_state;
32+
"failure, bad state arg", [ arg_code; arg_state_bad ], secrets_with_state;
33+
"failure, no code arg", [], secrets_default;
4834
]
4935

50-
let process (ctx : Context.t) (name, args, headers, body, secrets) =
36+
let process (ctx : Context.t) (name, args, secrets) =
5137
ctx.secrets <- Some secrets;
5238
Stdio.printf "===== slack oauth: %s ====\n" name;
53-
Action.process_slack_oauth ctx args headers body
39+
Action.process_slack_oauth ctx args
5440

5541
let () =
5642
let ctx = Context.make () in

0 commit comments

Comments
 (0)