diff --git a/documentation/secret_docs.md b/documentation/secret_docs.md index 87389bce..dadfa277 100644 --- a/documentation/secret_docs.md +++ b/documentation/secret_docs.md @@ -20,9 +20,14 @@ A secrets file stores sensitive information. Unlike the repository configuration | `slack_access_token` | slack bot access token to enable message posting to the workspace | Yes | try to use webhooks defined in `slack_hooks` instead | | `slack_hooks` | list of channel names and their corresponding webhook endpoint | Yes | try to use token defined in `slack_access_token` instead | | `slack_signing_secret` | specify to verify incoming slack requests | Yes | - | +| `slack_client_id` | slack client ID, used for [oauth](https://api.slack.com/authentication/oauth-v2) authentication | Yes | - | +| `slack_client_secret` | slack client secret, used for [oauth](https://api.slack.com/authentication/oauth-v2) authentication | Yes | - | +| `slack_oauth_state` | specify some unique value to maintain state b/w oauth request and callback and prevent CSRF (see [RFC6749](https://tools.ietf.org/html/rfc6749#section-4.1.1)) | Yes | - | Note that either `slack_access_token` or `slack_hooks` must be defined. +The fields `slack_client_id`, `slack_client_secret`, and `slack_oauth_state` only apply if you need to distribute the app to multiple users. + ## `gh_token` Some operations, such as fetching a config file from a private repository, or the commit corresponding to a commit comment event, require a personal access token. Refer [here](https://docs.github.com/en/free-pro-team@latest/github/authenticating-to-github/creating-a-personal-access-token) for detailed instructions on token generation. @@ -35,6 +40,8 @@ Refer [here](https://docs.github.com/en/free-pro-team@latest/developers/webhooks Refer [here](https://api.slack.com/authentication/oauth-v2) for obtaining an access token via OAuth. +If automatic OAuth exchange is set up, the bot will configure this value at runtime. + ## `slack_hooks` *Note: If `slack_access_token` is also defined, the bot will authenticate over Slack's Web API and this option will not be used.* diff --git a/lib/action.ml b/lib/action.ml index 07a5fe73..5ec0960a 100644 --- a/lib/action.ml +++ b/lib/action.ml @@ -280,4 +280,65 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct | Ok () -> match notification.event with | Link_shared event -> process_link_shared_event ctx event + + (** + + If there is a need to distribute the app, automatic OAuth exchange must be enabled. + + The fields `slack_client_id` and `slack_client_secret` must be configured in the + secrets file. The `slack_oauth_state` field can be optionally provided to avoid + forgery attacks during the OAuth exchange. + (see: https://tools.ietf.org/html/rfc6749#section-4.1.1) + + All of these fields are retrievable from the Slack app dashboard. + + Once the server has been configured and launched, it will listen on `/slack/oauth` + for incoming OAuth requests from Slack. Each user should then go to the following + address, replacing the appropriate values (the `state` argument is only needed + if `slack_oauth_state` is set). + + https://slack.com/oauth/v2/authorize?scope=chat:write&client_id=&redirect_uri=/slack/oauth&state= + + A page should open asking the user permission to install the bot to their + workspace. Clicking `allow` will trigger the OAuth exchange. + + *) + let process_slack_oauth (ctx : Context.t) args = + try%lwt + let secrets = Context.get_secrets_exn ctx in + match ctx.state.slack_access_token with + | Some _ -> Lwt.return "ok" + | None -> + match Slack.validate_state ?oauth_state:secrets.slack_oauth_state ~args with + | Error e -> action_error e + | Ok () -> + match List.Assoc.find args "code" ~equal:String.equal with + | None -> action_error "argument `code` not found in slack authorization request" + | Some code -> + ( match%lwt Slack_api.access_token_of_code ~ctx ~code with + | Error e -> action_error e + | Ok access_token -> + State.set_slack_access_token ctx.state access_token; + ( match ctx.state_filepath with + | None -> Lwt.return "ok" + | Some path -> + ( match%lwt State.save ctx.state path with + | Ok () -> Lwt.return "ok" + | Error e -> action_error e + ) + ) + ) + with + | Yojson.Json_error msg -> + let e = Printf.sprintf "failed to parse file as valid JSON (%s)\naborting slack oauth exchange" msg in + log#error "%s" e; + Lwt.return e + | Action_error msg -> + let e = Printf.sprintf "%s\naborting slack oauth exchange" msg in + log#error "%s" e; + Lwt.return e + | Context.Context_error msg -> + let e = Printf.sprintf "%s\naborting slack oauth exchange" msg in + log#error "%s" e; + Lwt.return e end diff --git a/lib/api.ml b/lib/api.ml index 1296272f..fa914442 100644 --- a/lib/api.ml +++ b/lib/api.ml @@ -16,4 +16,6 @@ module type Slack = sig val send_notification : ctx:Context.t -> msg:post_message_req -> (unit, string) Result.t Lwt.t val send_chat_unfurl : ctx:Context.t -> chat_unfurl_req -> (unit, string) Result.t Lwt.t + + val access_token_of_code : ctx:Context.t -> code:string -> (string, string) Result.t Lwt.t end diff --git a/lib/api_local.ml b/lib/api_local.ml index 915faa6d..7c24e2c6 100644 --- a/lib/api_local.ml +++ b/lib/api_local.ml @@ -28,6 +28,8 @@ module Slack_base : Api.Slack = struct let send_notification ~ctx:_ ~msg:_ = Lwt.return @@ Error "undefined for local setup" let send_chat_unfurl ~ctx:_ _ = Lwt.return @@ Error "undefined for local setup" + + let access_token_of_code ~ctx:_ ~code:_ = Lwt.return @@ Error "undefined for local setup" end module Slack : Api.Slack = struct @@ -38,6 +40,8 @@ module Slack : Api.Slack = struct Stdio.printf "will notify #%s\n" msg.channel; Stdio.printf "%s\n" json; Lwt.return @@ Ok () + + let access_token_of_code ~ctx:_ ~code = Lwt.return @@ Ok (Printf.sprintf "token of code %s" code) end module Slack_simple : Api.Slack = struct diff --git a/lib/api_remote.ml b/lib/api_remote.ml index 5fec99a7..7113b31d 100644 --- a/lib/api_remote.ml +++ b/lib/api_remote.ml @@ -120,4 +120,26 @@ module Slack : Api.Slack = struct ) | Error e -> Lwt.return @@ fmt_error "error while querying %s: %s\nfailed to unfurl Slack links" url e ) + + let access_token_of_code ~(ctx : Context.t) ~code = + let secrets = Context.get_secrets_exn ctx in + let body = `Form [ "code", code ] in + match secrets.slack_client_id with + | None -> Lwt.return @@ Error "slack_client_id is undefined" + | Some client_id -> + match secrets.slack_client_secret with + | None -> Lwt.return @@ Error "slack_client_secret is undefined" + | Some client_secret -> + let auth_header = Base64.encode_string @@ sprintf "%s:%s" client_id client_secret in + let headers = [ Printf.sprintf "Authorization: Basic %s" auth_header ] in + ( match%lwt Common.http_request ~body ~headers `POST "https://slack.com/api/oauth.v2.access" with + | Error e -> Lwt.return @@ Error e + | Ok data -> + let response = Slack_j.oauth_access_res_of_string data in + ( match response.access_token, response.error with + | Some access_token, _ -> Lwt.return @@ Ok access_token + | None, Some e -> Lwt.return @@ Error e + | None, None -> Lwt.return @@ Error "an unknown error occurred while getting access token" + ) + ) end diff --git a/lib/config.atd b/lib/config.atd index d90b1828..0e7efed1 100644 --- a/lib/config.atd +++ b/lib/config.atd @@ -49,4 +49,11 @@ type secrets = { ?slack_access_token : string nullable; (* Slack uses this secret to sign requests; provide to verify incoming Slack requests *) ?slack_signing_secret : string nullable; + (* Client ID of the Slack bot; used for OAuth authentication *) + ?slack_client_id : string nullable; + (* Client secret of the Slack bot; used for OAuth authentication *) + ?slack_client_secret : string nullable; + (* if specified, maintains state b/w OAuth request and callback to prevent CSRF + see: https://tools.ietf.org/html/rfc6749#section-4.1.1 *) + ?slack_oauth_state : string nullable; } diff --git a/lib/context.ml b/lib/context.ml index 0c44f68c..4bd9771b 100644 --- a/lib/context.ml +++ b/lib/context.ml @@ -15,17 +15,18 @@ type t = { state : State_t.state; } -let default : t = +let default () : t = { config_filename = "monorobot.json"; secrets_filepath = "secrets.json"; state_filepath = None; secrets = None; config = None; - state = State.empty; + state = State.empty (); } let make ?config_filename ?secrets_filepath ?state_filepath () = + let default = default () in let config_filename = Option.value config_filename ~default:default.config_filename in let secrets_filepath = Option.value secrets_filepath ~default:default.secrets_filepath in { default with config_filename; secrets_filepath; state_filepath } @@ -86,6 +87,12 @@ let refresh_state ctx = | Error e -> fmt_error "error while getting local file: %s\nfailed to get state from file %s" e path | Ok file -> let state = State_j.state_of_string file in + let secrets = get_secrets_exn ctx in + begin + match secrets.slack_access_token with + | None -> () + | Some token -> State.set_slack_access_token ctx.state token + end; Ok { ctx with state } end else Ok ctx diff --git a/lib/slack.atd b/lib/slack.atd index 85942218..692101cd 100644 --- a/lib/slack.atd +++ b/lib/slack.atd @@ -118,3 +118,10 @@ type chat_unfurl_res = { ok: bool; ?error: string option; } + +(* expected payload when exchanging oauth code for access token *) +type oauth_access_res = { + ok: bool; + ?access_token: string option; + ?error: string option; +} diff --git a/lib/slack.ml b/lib/slack.ml index 625c28c1..2522f0a6 100644 --- a/lib/slack.ml +++ b/lib/slack.ml @@ -373,3 +373,14 @@ let validate_signature ?(version = "v0") ?signing_key ~headers body = let basestring = Printf.sprintf "%s:%s:%s" version timestamp body in let expected_signature = Printf.sprintf "%s=%s" version (Common.sign_string_sha256 ~key ~basestring) in if String.equal expected_signature signature then Ok () else Error "signatures don't match" + +let validate_state ?oauth_state ~args = + match oauth_state with + | None -> Ok () + | Some expected_state -> + match List.Assoc.find args "state" ~equal:String.equal with + | None -> Error "argument `state` not found in slack authorization request" + | Some state -> + match String.equal state expected_state with + | false -> Error "argument `state` is present in slack authorization request but does not match expected value" + | true -> Ok () diff --git a/lib/state.atd b/lib/state.atd index 66338a56..b14e34a8 100644 --- a/lib/state.atd +++ b/lib/state.atd @@ -14,5 +14,6 @@ type pipeline_statuses = branch_statuses map_as_object (* The serializable runtime state of the bot *) type state = { - pipeline_statuses : pipeline_statuses + pipeline_statuses : pipeline_statuses; + ?slack_access_token : string nullable; } \ No newline at end of file diff --git a/lib/state.ml b/lib/state.ml index dcc7afe6..b5498038 100644 --- a/lib/state.ml +++ b/lib/state.ml @@ -2,7 +2,7 @@ open Base open Common open Devkit -let empty : State_t.state = { pipeline_statuses = StringMap.empty } +let empty () : State_t.state = { pipeline_statuses = StringMap.empty; slack_access_token = None } let refresh_pipeline_status (state : State_t.state) ~pipeline ~(branches : Github_t.branch list) ~status = let update_pipeline_status branch_statuses = @@ -12,6 +12,8 @@ let refresh_pipeline_status (state : State_t.state) ~pipeline ~(branches : Githu in state.pipeline_statuses <- Map.update state.pipeline_statuses pipeline ~f:update_pipeline_status +let set_slack_access_token (state : State_t.state) access_token = state.slack_access_token <- Some access_token + let log = Log.from "state" let save state path = diff --git a/src/request_handler.ml b/src/request_handler.ml index d89a9b3a..53af0eca 100644 --- a/src/request_handler.ml +++ b/src/request_handler.ml @@ -41,6 +41,9 @@ let setup_http ~ctx ~signature ~port ~ip = | _, [ "slack"; "events" ] -> log#info "%s" request.body; ret @@ Action.process_slack_event ctx request.headers request.body + | _, [ "slack"; "oauth" ] -> + log#info "slack oauth authorization request received"; + ret @@ Action.process_slack_oauth ctx request.args | _, _ -> log#error "unknown path : %s" (Httpev.show_request request); ret_err `Not_found "not found" diff --git a/test/dune b/test/dune index 71cdc617..28325de3 100644 --- a/test/dune +++ b/test/dune @@ -1,5 +1,5 @@ (executables - (names test github_link_test) + (names test github_link_test slack_oauth_test) (libraries lib base base.caml devkit devkit.core extlib lwt.unix stdio yojson) (preprocess @@ -26,3 +26,8 @@ (alias runtest) (action (run ./github_link_test.exe))) + +(rule + (alias runtest) + (action + (run ./slack_oauth_test.exe))) diff --git a/test/slack_oauth_test.ml b/test/slack_oauth_test.ml new file mode 100644 index 00000000..f13f8996 --- /dev/null +++ b/test/slack_oauth_test.ml @@ -0,0 +1,54 @@ +open Base +open Printf +open Lib +module Action = Action.Action (Api_local.Github) (Api_local.Slack) + +let arg_code = "code", "1591663521684.1613458437648.4a18cf683e541ff9d8fd75075181cac49c7acae9431d7e4ffd424ce1ca8d2543" + +let arg_state = "state", "foo" + +let arg_state_bad = "state", "bar" + +let secrets_default () : Config_t.secrets = + { + gh_token = None; + gh_hook_token = None; + slack_hooks = []; + slack_access_token = None; + slack_client_id = Some ""; + slack_client_secret = Some ""; + slack_signing_secret = None; + slack_oauth_state = None; + } + +let secrets_with_state = { (secrets_default ()) with slack_oauth_state = Some "foo" } + +let secrets_with_token = { (secrets_default ()) with slack_access_token = Some "original token" } + +let tests = + [ + "success, valid params", [ arg_code ], secrets_default (), Some (sprintf "token of code %s" (snd arg_code)); + "success, with state", [ arg_code; arg_state ], secrets_with_state, Some (sprintf "token of code %s" (snd arg_code)); + "no-op, token exists", [ arg_code ], secrets_with_token, Some "original token"; + "failure, no state arg", [ arg_code ], secrets_with_state, None; + "failure, bad state arg", [ arg_code; arg_state_bad ], secrets_with_state, None; + "failure, no code arg", [], secrets_default (), None; + ] + +let process (name, args, secrets, expected_token) = + let ctx = Context.make () in + ctx.secrets <- Some secrets; + Option.iter secrets.slack_access_token ~f:(fun t -> ctx.state.slack_access_token <- Some t); + let%lwt _msg = Action.process_slack_oauth ctx args in + match assert ((Option.equal String.equal) ctx.state.slack_access_token expected_token) with + | exception Assert_failure _ -> Lwt.return @@ Error (sprintf "test failed: %s" name) + | () -> Lwt.return @@ Ok () + +let () = + Lwt_main.run + ( match%lwt Lwt_list.map_s process tests |> Lwt.map Result.combine_errors_unit with + | Ok () -> Lwt.return_unit + | Error e -> + List.iter e ~f:Stdio.print_endline; + Caml.exit 1 + )