Shred overwritten cached credentials (#28436)

JuliaLang · Aug 4, 2018 · dd248bf · dd248bf · nanosoldier · Aug 5, 2018
1 parent 1dfc4d1
commit dd248bf
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 4 deletions.
diff --git a/stdlib/LibGit2/src/types.jl b/stdlib/LibGit2/src/types.jl
@@ -1303,13 +1303,17 @@ end
 
 function approve(cache::CachedCredentials, cred::AbstractCredential, url::AbstractString)
     cred_id = credential_identifier(url)
+    if haskey(cache.cred, cred_id) && cred !== cache.cred[cred_id]
+         Base.shred!(cache.cred[cred_id])
+    end
     cache.cred[cred_id] = cred
     nothing
 end
 
 function reject(cache::CachedCredentials, cred::AbstractCredential, url::AbstractString)
     cred_id = credential_identifier(url)
     if haskey(cache.cred, cred_id)
+        Base.shred!(cache.cred[cred_id])
         delete!(cache.cred, cred_id)
     end
     nothing

diff --git a/stdlib/LibGit2/test/libgit2.jl b/stdlib/LibGit2/test/libgit2.jl
@@ -1734,14 +1734,28 @@ mktempdir() do dir
         @test haskey(cache, cred_id)
         @test cache[cred_id] === cred
 
-        # Reject an approved should cause it to be removed
+        # Approve the same credential again which does not overwrite
+        LibGit2.approve(cache, cred, url)
+        @test haskey(cache, cred_id)
+        @test cache[cred_id] === cred
+
+        # Overwrite an already cached credential
+        dup_cred = deepcopy(cred)
+        LibGit2.approve(cache, dup_cred, url)  # Shreds `cred`
+        @test haskey(cache, cred_id)
+        @test cache[cred_id] === dup_cred
+        @test dup_cred.pass == password
+        @test cred.pass != password
+
+        cred = dup_cred
+
+        # Reject an approved should cause it to be removed and shredded
         LibGit2.reject(cache, cred, url)
         @test !haskey(cache, cred_id)
-        @test cred.user == "julia"
-        @test cred.pass == password
+        @test cred.user != "julia"
+        @test cred.pass != password
 
         Base.shred!(cache)
-        Base.shred!(cred)
         Base.shred!(password)
     end