Skip to content

Commit 496a898

Browse files
committed
Return invalid_token 401 error for expired oauth token
1 parent fd38dac commit 496a898

File tree

5 files changed

+49
-22
lines changed

5 files changed

+49
-22
lines changed

pegasus/lib/api/account_/permissions.ml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ let post_handler =
9595
Oauth.Queries.delete_oauth_tokens_by_client ctx.db ~did
9696
~client_id
9797
in
98+
Oauth.Dpop.revoke_tokens_for_did did ;
9899
Dream.redirect ctx.req "/account/permissions"
99100
| None ->
100101
Dream.redirect ctx.req "/account/permissions" )
@@ -113,6 +114,7 @@ let post_handler =
113114
Oauth.Queries.delete_oauth_tokens_by_device ctx.db ~did
114115
~last_ip ~last_user_agent
115116
in
117+
Oauth.Dpop.revoke_tokens_for_did did ;
116118
Dream.redirect ctx.req "/account/permissions"
117119
| _ ->
118120
Dream.redirect ctx.req "/account/permissions" )

pegasus/lib/auth.ml

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ module Verifiers = struct
258258
~url:(Dream.target req) ~dpop_header ()
259259
with
260260
| Error "use_dpop_nonce" ->
261-
Lwt.return_error @@ Errors.use_dpop_nonce_auth ()
261+
Lwt.return_error @@ Errors.dpop_auth "use_dpop_nonce"
262262
| Error e ->
263263
Log.debug (fun log -> log "dpop error: %s" e) ;
264264
Lwt.return_error @@ Errors.invalid_request ("dpop error: " ^ e)
@@ -279,7 +279,7 @@ module Verifiers = struct
279279
~access_token:token ()
280280
with
281281
| Error "use_dpop_nonce" ->
282-
Lwt.return_error @@ Errors.use_dpop_nonce_resource ()
282+
Lwt.return_error @@ Errors.dpop_resource "use_dpop_nonce"
283283
| Error e ->
284284
Log.debug (fun log -> log "dpop error: %s" e) ;
285285
Lwt.return_error @@ Errors.invalid_request ("dpop error: " ^ e)
@@ -292,6 +292,7 @@ module Verifiers = struct
292292
let open Yojson.Safe.Util in
293293
try
294294
let did = claims |> member "sub" |> to_string in
295+
let iat = claims |> member "iat" |> to_int in
295296
let exp = claims |> member "exp" |> to_int in
296297
let jkt_claim =
297298
claims |> member "cnf" |> member "jkt" |> to_string
@@ -307,6 +308,8 @@ module Verifiers = struct
307308
else if exp < now then
308309
Lwt.return_error
309310
@@ Errors.invalid_request ~name:"ExpiredToken" "token expired"
311+
else if Oauth.Dpop.is_token_revoked ~did ~iat then
312+
Lwt.return_error @@ Errors.dpop_resource "invalid_token"
310313
else if not (Oauth.Scopes.has_atproto scopes) then
311314
Lwt.return_error
312315
@@ Errors.invalid_request ~name:"InvalidToken"

pegasus/lib/errors.ml

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@ exception NotFoundError of (string * string)
88

99
exception Redirect of string
1010

11-
(* HTTP 400, { error: "use_dpop_nonce" } — https://datatracker.ietf.org/doc/html/rfc9449#section-8 *)
12-
exception UseDpopNonceAuthError
11+
(* HTTP 400, { error: "..." } — https://datatracker.ietf.org/doc/html/rfc9449#section-8 *)
12+
exception DpopAuthError of string
1313

14-
(* HTTP 401, WWW-Authenticate — https://datatracker.ietf.org/doc/html/rfc9449#section-9 *)
15-
exception UseDpopNonceResourceError
14+
(* HTTP 401, WWW-Authenticate=DPoP error="..." — https://datatracker.ietf.org/doc/html/rfc9449#section-9 *)
15+
exception DpopResourceError of string
1616

1717
let is_xrpc_error = function
1818
| InvalidRequestError _
@@ -34,9 +34,9 @@ let auth_required ?(name = "AuthRequired") msg = raise (AuthError (name, msg))
3434

3535
let not_found ?(name = "NotFound") msg = raise (NotFoundError (name, msg))
3636

37-
let use_dpop_nonce_auth () = raise UseDpopNonceAuthError
37+
let dpop_auth error = raise (DpopAuthError error)
3838

39-
let use_dpop_nonce_resource () = raise UseDpopNonceResourceError
39+
let dpop_resource error = raise (DpopResourceError error)
4040

4141
let printer = function
4242
| InvalidRequestError (error, message) ->
@@ -47,10 +47,10 @@ let printer = function
4747
Some (Printf.sprintf "Auth error (%s): %s" error message)
4848
| NotFoundError (error, message) ->
4949
Some (Printf.sprintf "Not found (%s): %s" error message)
50-
| UseDpopNonceAuthError ->
51-
Some "Use DPoP nonce"
52-
| UseDpopNonceResourceError ->
53-
Some "Use DPoP nonce"
50+
| DpopAuthError error ->
51+
Some (Printf.sprintf "DPoP auth error (%s)" error)
52+
| DpopResourceError error ->
53+
Some (Printf.sprintf "DPoP resource error (%s)" error)
5454
| _ ->
5555
None
5656

@@ -72,18 +72,18 @@ let exn_to_response exn =
7272
| NotFoundError (error, message) ->
7373
Log.debug (fun log -> log "not found error: %s - %s" error message) ;
7474
format_response error message `Not_Found
75-
| UseDpopNonceAuthError ->
76-
Log.debug (fun log -> log "use_dpop_nonce auth error") ;
75+
| DpopAuthError e ->
76+
Log.debug (fun log -> log "dpop auth error") ;
7777
Dream.json ~status:`Bad_Request
7878
~headers:[("Access-Control-Expose-Headers", "DPoP-Nonce")]
79-
{|{ "error": "use_dpop_nonce" }|}
80-
| UseDpopNonceResourceError ->
81-
Log.debug (fun log -> log "use_dpop_nonce resource error") ;
79+
(Format.sprintf {|{ "error": "%s" }|} e)
80+
| DpopResourceError e ->
81+
Log.debug (fun log -> log "dpop resource error") ;
8282
Dream.json ~status:`Unauthorized
8383
~headers:
84-
[ ("WWW-Authenticate", {|DPoP error="use_dpop_nonce"|})
84+
[ ("WWW-Authenticate", Format.sprintf {|DPoP error="%s"|} e)
8585
; ("Access-Control-Expose-Headers", "DPoP-Nonce, WWW-Authenticate") ]
86-
{|{ "error": "use_dpop_nonce" }|}
86+
(Format.sprintf {|{ "error": "%s" }|} e)
8787
| e ->
8888
Log.warn (fun log ->
8989
log "unexpected internal error: %s" (Printexc.to_string e) ) ;

pegasus/lib/oauth/dpop.ml

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,28 @@ let cleanup_jti_cache () =
2020
(fun _ expires_at -> if expires_at > now then Some expires_at else None)
2121
jti_cache
2222

23+
let revocation_cache : (string, int) Hashtbl.t = Hashtbl.create 1000
24+
25+
let cleanup_revocation_cache () =
26+
let now_s = int_of_float (Unix.gettimeofday ()) in
27+
let max_token_age_s = Constants.access_token_expiry_ms / 1000 in
28+
Hashtbl.filter_map_inplace
29+
(fun _ revoked_at ->
30+
if now_s - revoked_at < max_token_age_s then Some revoked_at else None )
31+
revocation_cache
32+
33+
let revoke_tokens_for_did did =
34+
let now_s = int_of_float (Unix.gettimeofday ()) in
35+
Hashtbl.replace revocation_cache did now_s ;
36+
if Hashtbl.length revocation_cache mod 50 = 0 then cleanup_revocation_cache ()
37+
38+
let is_token_revoked ~did ~iat =
39+
match Hashtbl.find_opt revocation_cache did with
40+
| Some revoked_at ->
41+
iat < revoked_at
42+
| None ->
43+
false
44+
2345
let compute_nonce secret counter =
2446
let data = Bytes.create 8 in
2547
Bytes.set_int64_be data 0 (Int64.of_int counter) ;

pegasus/lib/xrpc.ml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ let add_dpop_nonce_if_needed res =
108108
in
109109
let () =
110110
let to_expose =
111-
(* see comments on UseDpopNonce____Error in errors.ml *)
111+
(* see comments on Dpop____Error in errors.ml *)
112112
if Dream.status res = `Unauthorized then "DPoP-Nonce, WWW-Authenticate"
113113
else if Dream.status res = `Bad_Request then "DPoP-Nonce"
114114
else ""
@@ -146,7 +146,7 @@ let handler ?(auth : Auth.Verifiers.t = Any)
146146
let%lwt res = exn_to_response e in
147147
Lwt.return
148148
( match e with
149-
| UseDpopNonceAuthError | UseDpopNonceResourceError ->
149+
| DpopAuthError _ | DpopResourceError _ ->
150150
add_dpop_nonce_if_needed res
151151
| _ ->
152152
res )
@@ -155,7 +155,7 @@ let handler ?(auth : Auth.Verifiers.t = Any)
155155
Dream.redirect init.req r
156156
| Rate_limiter.Rate_limit_exceeded status ->
157157
rate_limit_response status
158-
| (UseDpopNonceAuthError | UseDpopNonceResourceError) as e ->
158+
| (DpopAuthError _ | DpopResourceError _) as e ->
159159
let%lwt res = exn_to_response e in
160160
Lwt.return (add_dpop_nonce_if_needed res)
161161
| e ->

0 commit comments

Comments
 (0)