Fix race on group (#105)

* Add better auth error propogation from interactor to controller; added
interactor unit test

* Fix flakey test race condition; remove unused cleanup func

* Deflake test with unique sub and group for each test

* redundant advisory lock (already locking in private method)

app/lib/clients/vault/policy.rb

@@ -13,10 +13,8 @@ def rotate_token
       def assign_entity_policy(identity, policy_name)
         sub = identity.sub
         email = identity.email
-        Domain.with_advisory_lock(sub) do
-          put_entity(sub, [ policy_name ])
-          put_entity_alias(sub, email, "oidc")
-        end
+        put_entity(sub, [ policy_name ])
+        put_entity_alias(sub, email, "oidc")
       end
 
       def assign_groups_policy(groups, policy_name)

test/integration/secrets_test.rb

@@ -1,6 +1,14 @@
 require "test_helper"
 
 class SecretsTest < ActionDispatch::IntegrationTest
+  setup do
+    jwt_json = unique_jwt
+    @jwt_groups = jwt_json["groups"]
+    @jwt_authorized = make_signed_jwt(jwt_json)
+    jwt_read_group_json = unique_jwt.tap { |h| h["groups"] = @jwt_groups }
+    @jwt_read_group = make_signed_jwt(jwt_read_group_json)
+  end
+
   test "#create unauthorized" do
     post secrets_path
     assert_response :unauthorized
@@ -22,7 +30,7 @@ class SecretsTest < ActionDispatch::IntegrationTest
   test "#update an existing secret with same user is authorized" do
     existing_path = create_secret
     assert_response :success
-    create_secret(jwt_authorized, existing_path)
+    create_secret(@jwt_authorized, existing_path)
     assert_response :success
   end
 
@@ -36,7 +44,7 @@ class SecretsTest < ActionDispatch::IntegrationTest
   test "#show" do
     path = create_secret
     # view the secret
-    get secret_path(path), headers: { "Authorization" => "Bearer #{jwt_authorized}" }
+    get secret_path(path), headers: { "Authorization" => "Bearer #{@jwt_authorized}" }
     assert_response :success
     %w[ data metadata lease_id ].each do |key|
       assert_includes response.parsed_body["secret"].keys, key
@@ -46,7 +54,7 @@ class SecretsTest < ActionDispatch::IntegrationTest
   test "#show with read_group is authorized" do
     path = create_secret
     # view the secret
-    get secret_path(path), headers: { "Authorization" => "Bearer #{jwt_read_group}" }
+    get secret_path(path), headers: { "Authorization" => "Bearer #{@jwt_read_group}" }
     assert_response :success
     %w[ data metadata lease_id ].each do |key|
       assert_includes response.parsed_body["secret"].keys, key
@@ -56,27 +64,28 @@ class SecretsTest < ActionDispatch::IntegrationTest
   test "#delete" do
     path = create_secret
     # delete the secret
-    delete destroy_secret_path(path), headers: { "Authorization" => "Bearer #{jwt_authorized}" }
+    delete destroy_secret_path(path), headers: { "Authorization" => "Bearer #{@jwt_authorized}" }
     assert_response :success
   end
 
   test "#delete with a read-authorized user is unauthorized" do
     path = create_secret
     # delete the secret
-    delete destroy_secret_path(path), headers: { "Authorization" => "Bearer #{jwt_read_group}" }
+    delete destroy_secret_path(path), headers: { "Authorization" => "Bearer #{@jwt_read_group}" }
     assert_response :unauthorized
   end
 
   private
 
-  def create_secret(jwt = jwt_authorized, path = "top/secret/#{SecureRandom.hex}")
+  def create_secret(jwt = @jwt_authorized, path = "top/secret/#{SecureRandom.hex}", groups = @jwt_groups)
     # create the secret
     post secrets_path, headers: { "Authorization" => "Bearer #{jwt}" },
-         params: { secret: { path: path, data: { password: "sicr3t" }, groups: "read_group" } }
+         params: { secret: { path: path, data: { password: "sicr3t" }, groups: groups.join(",") } }
     path
   end
 
-  def remove_pki_engine
-    vault_client.sys.unmount "pki_astral"
+  def unique_jwt
+    { "sub"=>SecureRandom.hex, "name"=>"John Doe", "iat"=>1516239022,
+      "groups"=>[ SecureRandom.hex ], "aud"=>"astral" }
   end
 end

test/test_helper.rb

@@ -14,16 +14,21 @@ class TestCase
     fixtures :all
 
     # Helper methods
+
+    def make_signed_jwt(json)
+      JWT.encode(json, Config[:jwt_signing_key])
+    end
+
     def jwt_authorized
-      @@authorized_token ||= JWT.encode(@@authorized_data, Config[:jwt_signing_key])
+      @@authorized_token ||= make_signed_jwt(@@authorized_data)
     end
 
     def jwt_unauthorized
       @@unauthorized_token ||= JWT.encode(@@unauthorized_data, "bad_secret")
     end
 
     def jwt_read_group
-      @@read_group_token ||= JWT.encode(@@read_group_data, Config[:jwt_signing_key])
+      @@read_group_token ||= make_signed_jwt(@@read_group_data)
     end
 
     private