OAuth 2.0 authorization and token exchange
Remove exchange_form_body and refresh_form_body from public API
These functions exposed client_secret in cleartext and relied on the
caller to POST over TLS. The secure-by-default API is exchange_code
and refresh_token which handle transport internally.
The form encoding functions remain as internal helpers but are no
longer exported. Tests that called them directly have been removed.
+9
-157
-19
fuzz/fuzz_github_oauth.ml
-19
fuzz/fuzz_github_oauth.ml
···
19
19
check (String.length url > 0);
20
20
check (String.sub url 0 8 = "https://")
21
21
22
-
(* Test that exchange_form_body produces valid form-encoded body *)
23
-
let test_exchange_body_valid client_id client_secret code =
24
-
let body =
25
-
Oauth.exchange_form_body ~client_id ~client_secret ~code ~redirect_uri:redir
26
-
()
27
-
in
28
-
check (String.length body > 0);
29
-
check (String.contains body '=')
30
-
31
-
(* Test that refresh_form_body produces valid form-encoded body *)
32
-
let test_refresh_body_valid client_id client_secret refresh_token =
33
-
let body = Oauth.refresh_form_body ~client_id ~client_secret ~refresh_token in
34
-
check (String.length body > 0);
35
-
check (String.contains body '=')
36
-
37
22
(* Test that parse_token_response handles arbitrary input without crashing *)
38
23
let test_parse_no_crash input =
39
24
let _ = Oauth.parse_token_response input in
···
97
82
test_case "authorization_url valid"
98
83
[ bytes; bytes; list bytes ]
99
84
test_authorization_url_valid;
100
-
test_case "exchange_body valid" [ bytes; bytes; bytes ]
101
-
test_exchange_body_valid;
102
-
test_case "refresh_body valid" [ bytes; bytes; bytes ]
103
-
test_refresh_body_valid;
104
85
test_case "parse_no_crash" [ bytes ] test_parse_no_crash;
105
86
test_case "token roundtrip"
106
87
[ bytes; option int; option bytes ]
+6
-29
lib/oauth.mli
+6
-29
lib/oauth.mli
···
30
30
let callback_state = (* [state] query param from callback URL *) in
31
31
if not (Oauth.validate_state ~expected:state ~actual:callback_state) then
32
32
failwith "CSRF state mismatch";
33
-
let body =
34
-
Oauth.exchange_form_body ~client_id:"xxx" ~client_secret:"yyy"
35
-
~code ~redirect_uri
36
-
~code_verifier:verifier ()
37
-
in
38
-
(* POST [body] over TLS to [Oauth.token_url Github] with
39
-
Content-Type: application/x-www-form-urlencoded
40
-
Accept: application/json *)
33
+
match
34
+
Oauth.exchange_code http Github ~client_id:"xxx" ~client_secret:"yyy"
35
+
~code ~redirect_uri ~code_verifier:verifier ()
36
+
with
37
+
| Ok token -> (* use [token.access_token] *)
38
+
| Error e -> (* handle error *)
41
39
]} *)
42
40
43
41
(** {1:providers Providers} *)
···
271
269
{!exchange_code} and {!refresh_token} which handle the HTTP transport. *)
272
270
273
271
val pp_parse_token_error : Format.formatter -> parse_token_error -> unit
274
-
275
-
(** {2 Low-level Form Encoding}
276
-
277
-
These produce raw form bodies. Prefer {!exchange_code} and {!refresh_token}
278
-
which handle TLS transport. *)
279
-
280
-
val exchange_form_body :
281
-
client_id:string ->
282
-
client_secret:string ->
283
-
code:string ->
284
-
redirect_uri:redirect_uri ->
285
-
?code_verifier:string ->
286
-
unit ->
287
-
string
288
-
(** [exchange_form_body] produces the form-encoded body for a token exchange.
289
-
Contains [client_secret] in cleartext — use {!exchange_code} instead. *)
290
-
291
-
val refresh_form_body :
292
-
client_id:string -> client_secret:string -> refresh_token:string -> string
293
-
(** [refresh_form_body] produces the form-encoded body for a token refresh.
294
-
Contains [client_secret] in cleartext — use {!refresh_token} instead. *)
295
272
296
273
(** {1:userinfo Userinfo Parsing} *)
297
274
-64
test/test_github_oauth.ml
-64
test/test_github_oauth.ml
···
104
104
"contains google.com" true
105
105
(is_substring url ~substring:"accounts.google.com")
106
106
107
-
let test_exchange_request_body () =
108
-
let body =
109
-
Oauth.exchange_form_body ~client_id:"test_client"
110
-
~client_secret:"test_secret" ~code:"auth_code"
111
-
~redirect_uri:(redir "https://example.com/callback")
112
-
()
113
-
in
114
-
Alcotest.(check bool)
115
-
"contains client_id" true
116
-
(is_substring body ~substring:"client_id=");
117
-
Alcotest.(check bool)
118
-
"contains client_secret" true
119
-
(is_substring body ~substring:"client_secret=");
120
-
Alcotest.(check bool)
121
-
"contains code" true
122
-
(is_substring body ~substring:"code=");
123
-
Alcotest.(check bool)
124
-
"contains redirect_uri" true
125
-
(is_substring body ~substring:"redirect_uri=")
126
-
127
107
let test_parse_token_oauth_app () =
128
108
let json = {|{"access_token":"gho_abc123"}|} in
129
109
match Oauth.parse_token_response json with
···
186
166
| Error _ -> ()
187
167
| Ok _ -> Alcotest.fail "expected error on empty list"
188
168
189
-
let test_refresh_request_body () =
190
-
let body =
191
-
Oauth.refresh_form_body ~client_id:"test_client"
192
-
~client_secret:"test_secret" ~refresh_token:"ghr_abc123"
193
-
in
194
-
Alcotest.(check bool)
195
-
"contains client_id" true
196
-
(is_substring body ~substring:"client_id=");
197
-
Alcotest.(check bool)
198
-
"contains grant_type" true
199
-
(is_substring body ~substring:"grant_type=");
200
-
Alcotest.(check bool)
201
-
"contains refresh_token" true
202
-
(is_substring body ~substring:"refresh_token=")
203
-
204
169
(* ── PKCE (RFC 7636) ─────────────────────────────────────────────── *)
205
170
206
171
let test_code_verifier_length () =
···
266
231
"contains code_challenge_method=plain" true
267
232
(is_substring url ~substring:"code_challenge_method=plain")
268
233
269
-
let test_exchange_with_code_verifier () =
270
-
let body =
271
-
Oauth.exchange_form_body ~client_id:"test_client"
272
-
~client_secret:"test_secret" ~code:"auth_code"
273
-
~redirect_uri:(redir "https://example.com/callback")
274
-
~code_verifier:"dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk" ()
275
-
in
276
-
Alcotest.(check bool)
277
-
"contains code_verifier" true
278
-
(is_substring body ~substring:"code_verifier=")
279
-
280
-
let test_exchange_without_code_verifier () =
281
-
let body =
282
-
Oauth.exchange_form_body ~client_id:"test_client"
283
-
~client_secret:"test_secret" ~code:"auth_code"
284
-
~redirect_uri:(redir "https://example.com/callback")
285
-
()
286
-
in
287
-
Alcotest.(check bool)
288
-
"no code_verifier" true
289
-
(not (is_substring body ~substring:"code_verifier="))
290
-
291
234
let suite =
292
235
( "oauth",
293
236
[
···
319
262
test_authorization_url_multiple_scopes;
320
263
Alcotest.test_case "authorization_url google" `Quick
321
264
test_authorization_url_google;
322
-
Alcotest.test_case "exchange request body" `Quick
323
-
test_exchange_request_body;
324
265
Alcotest.test_case "parse_token OAuth App" `Quick
325
266
test_parse_token_oauth_app;
326
267
Alcotest.test_case "parse_token GitHub App" `Quick
···
329
270
test_parse_token_extra_fields;
330
271
Alcotest.test_case "parse_token invalid json" `Quick
331
272
test_parse_token_invalid_json;
332
-
Alcotest.test_case "refresh request body" `Quick test_refresh_request_body;
333
273
Alcotest.test_case "PKCE verifier length" `Quick test_code_verifier_length;
334
274
Alcotest.test_case "PKCE verifier charset" `Quick
335
275
test_code_verifier_charset;
···
341
281
test_authorization_url_with_pkce;
342
282
Alcotest.test_case "PKCE authorization_url plain" `Quick
343
283
test_authorization_url_pkce_plain;
344
-
Alcotest.test_case "PKCE exchange with verifier" `Quick
345
-
test_exchange_with_code_verifier;
346
-
Alcotest.test_case "PKCE exchange without verifier" `Quick
347
-
test_exchange_without_code_verifier;
348
284
] )
+3
-45
test/test_regressions.ml
+3
-45
test/test_regressions.ml
···
33
33
"response_type=code" (Some "code")
34
34
(Uri.get_query_param uri "response_type")
35
35
36
-
let test_exchange_request_body_uses_form_encoding () =
37
-
let body =
38
-
Oauth.exchange_form_body ~client_id:"test_client"
39
-
~client_secret:"test_secret" ~code:"auth_code"
40
-
~redirect_uri:(redir "https://example.com/callback")
41
-
()
42
-
in
43
-
let query = Uri.query_of_encoded body in
44
-
Alcotest.(check (option (list string)))
45
-
"grant_type=authorization_code" (Some [ "authorization_code" ])
46
-
(query_param "grant_type" query);
47
-
Alcotest.(check (option (list string)))
48
-
"client_id preserved" (Some [ "test_client" ])
49
-
(query_param "client_id" query);
50
-
Alcotest.(check (option (list string)))
51
-
"client_secret preserved" (Some [ "test_secret" ])
52
-
(query_param "client_secret" query);
53
-
Alcotest.(check (option (list string)))
54
-
"code preserved" (Some [ "auth_code" ]) (query_param "code" query);
55
-
Alcotest.(check (option (list string)))
56
-
"redirect_uri preserved" (Some [ "https://example.com/callback" ])
57
-
(query_param "redirect_uri" query)
58
-
59
-
let test_refresh_request_body_uses_form_encoding () =
60
-
let body =
61
-
Oauth.refresh_form_body ~client_id:"test_client"
62
-
~client_secret:"test_secret" ~refresh_token:"ghr_abc123"
63
-
in
64
-
let query = Uri.query_of_encoded body in
65
-
Alcotest.(check (option (list string)))
66
-
"grant_type=refresh_token" (Some [ "refresh_token" ])
67
-
(query_param "grant_type" query);
68
-
Alcotest.(check (option (list string)))
69
-
"client_id preserved" (Some [ "test_client" ])
70
-
(query_param "client_id" query);
71
-
Alcotest.(check (option (list string)))
72
-
"client_secret preserved" (Some [ "test_secret" ])
73
-
(query_param "client_secret" query);
74
-
Alcotest.(check (option (list string)))
75
-
"refresh_token preserved" (Some [ "ghr_abc123" ])
76
-
(query_param "refresh_token" query)
36
+
(* exchange_form_body and refresh_form_body are no longer public —
37
+
form encoding correctness is tested indirectly via exchange_code
38
+
and refresh_token which use them internally. *)
77
39
78
40
let parse_token_error = Alcotest.testable Oauth.pp_parse_token_error ( = )
79
41
···
347
309
[
348
310
Alcotest.test_case "authorization_url includes response_type" `Quick
349
311
test_authorization_url_includes_response_type_code;
350
-
Alcotest.test_case "exchange_request_body uses form encoding" `Quick
351
-
test_exchange_request_body_uses_form_encoding;
352
-
Alcotest.test_case "refresh_request_body uses form encoding" `Quick
353
-
test_refresh_request_body_uses_form_encoding;
354
312
Alcotest.test_case "parse_token_response ok" `Quick
355
313
test_parse_token_response_ok;
356
314
Alcotest.test_case "parse_token_response invalid json" `Quick