Merge branch 'mp/fix-refresh' of github.com:mortenpi/PkgAuthentication.jl into mortenpi-mp/fix-refresh

Author: pfitzseb

src/PkgAuthentication.jl

@@ -122,6 +122,7 @@ function authenticate(;
         state = initial(server)
         try
             while !(isa(state, Success) || isa(state, Failure))
+                @debug "Calling step(::$(typeof(state)))"
                 state = step(state)
             end
         catch err
@@ -227,20 +228,33 @@ function step(state::NeedRefresh)::Union{HasNewToken, NoAuthentication}
     # errors are recoverable by just getting a new token:
     if response isa Downloads.Response && response.status == 200
         try
-            body = JSON.parse(String(take!(output)))
-            if haskey(body, "token")
-                return HasNewToken(state.server, body["token"])
+            body = TOML.parse(String(take!(output)))
+            let msg = "token refresh response"
+                assert_dict_keys(body, "access_token", "id_token"; msg=msg)
+                assert_dict_keys(body, "expires_in"; msg=msg)
+                assert_dict_keys(body, "expires", "expires_at"; msg=msg)
             end
+            return HasNewToken(state.server, body)
         catch err
             @debug "invalid body received while refreshing token" exception=(err, catch_backtrace())
         end
         return NoAuthentication(state.server)
     else
-        @debug "request for refreshing token failed" exception=(err, catch_backtrace())
+        @debug "request for refreshing token failed" response
         return NoAuthentication(state.server)
     end
 end
 
+function assert_dict_keys(dict::Dict, keys...; msg::AbstractString)
+    any(haskey(dict, key) for key in keys) && return nothing
+    if length(keys) == 1
+        error("Key '$(first(keys))' not present in $msg")
+    else
+        keys = join(string.("'", keys, "'"), ", ")
+        error("None of $keys present in $msg")
+    end
+end
+
 """
 Takes the token from the previous step and writes it to the auth.toml file. In order
 to handle potential race conditions with other writes, it will check that the write

test/authserver.jl

@@ -1,4 +1,5 @@
 using HTTP, Random, JSON
+import TOML
 
 const EXPIRY = 30
 const CHALLENGE_EXPIRY = 10
@@ -39,7 +40,7 @@ function response_handler(req)
     TOKEN[] = Dict(
         "user_name" => "firstname lastname",
         "user_email" => "[email protected]",
-        "id_token" => ID_TOKEN,
+        "id_token" => "full-" * ID_TOKEN,
         "refresh_token" => refresh_token,
         "refresh_url" => "http://localhost:$(PORT)/auth/renew/token.toml/v2/",
         "expires_in" => EXPIRY,
@@ -85,10 +86,9 @@ function renew_handler(req)
 
     TOKEN[]["refresh_token"] = Random.randstring(10)
     TOKEN[]["expires_at"] = ceil(Int, time() + EXPIRY)
+    TOKEN[]["id_token"] = "refresh-" * ID_TOKEN
 
-    return HTTP.Response(200, JSON.json(Dict(
-        "token" => TOKEN[]
-    )))
+    return HTTP.Response(200, sprint(TOML.print, TOKEN[]))
 end
 
 function check_validity(req)

test/runtests.jl

@@ -7,6 +7,9 @@ import Pkg
 include("util.jl")
 
 @testset "PkgAuthentication" begin
+    @testset "Utility functions" begin
+        include("utilities_test.jl")
+    end
     with_temp_depot() do
         include("tests.jl")
     end

test/tests.jl

@@ -45,6 +45,7 @@ PkgAuthentication.register_open_browser_hook(url -> HTTP.get(url))
 
     @test success isa PkgAuthentication.Success
     @test success.token["expires_at"] > time()
+    @test startswith(success.token["id_token"], "full-")
 
     sleeptimer = ceil(Int, success.token["expires_at"]  - time() + 1)
     @info "sleep for $(sleeptimer)s (until refresh necessary)"
@@ -55,6 +56,7 @@ PkgAuthentication.register_open_browser_hook(url -> HTTP.get(url))
     @test success2 isa PkgAuthentication.Success
     @test success2.token["expires_at"] > time()
     @test success2.token["refresh_token"] !== success.token["refresh_token"]
+    @test startswith(success2.token["id_token"], "refresh-")
 end
 
 @testset "PkgAuthentication.install" begin

test/utilities_test.jl

@@ -0,0 +1,11 @@
+@testset "assert_dict_keys" begin
+    @test_throws ErrorException PkgAuthentication.assert_dict_keys(Dict(), "foo"; msg="")
+    @test PkgAuthentication.assert_dict_keys(Dict("foo" => 0), "foo"; msg="") === nothing
+    @test_throws ErrorException PkgAuthentication.assert_dict_keys(Dict("bar" => 0), "foo"; msg="")
+
+    @test PkgAuthentication.assert_dict_keys(Dict("foo" => 0, "bar" => 0), "foo", "bar"; msg="") === nothing
+    @test PkgAuthentication.assert_dict_keys(Dict("foo" => 0), "foo", "bar"; msg="") === nothing
+    @test PkgAuthentication.assert_dict_keys(Dict("bar" => 0), "foo", "bar"; msg="") === nothing
+    @test_throws ErrorException PkgAuthentication.assert_dict_keys(Dict(), "foo", "bar"; msg="")
+    @test_throws ErrorException PkgAuthentication.assert_dict_keys(Dict("baz" => 0), "foo", "bar"; msg="")
+end