Merge pull request #1289 from chef/sr/pool-606/oc-id-api-coverage-in-…

…pedant

[POOL-606] oc-chef-pedant: improve oc_id API test coverage

oc-chef-pedant/lib/pedant/rspec/common.rb

@@ -387,6 +387,16 @@ def self.isolate(message = nil, &examples)
           end
         end
 
+        # Used to easily documented endpoints that don't have tests
+        def self.untested_endpoint(endpoint_name, reason)
+          context endpoint_name do
+            it "returns success" do
+              pending(reason)
+              raise NotImplementedError
+            end
+          end
+        end
+
         # Run the given block once (using 'key' to identify
         # the block).  This allows us to do "before :each"
         # cleanup to clean up from previous test runs, since

oc-chef-pedant/spec/api/oc_id_spec.rb

@@ -2,6 +2,37 @@
 require 'openssl'
 
 describe "oc_id API", :oc_id do
+  let(:request_headers) do
+    {
+      "Content-Type" => "application/x-www-form-urlencoded",
+      "Accept" => "text/html",
+      "Cookie" => csrf[:cookie]
+    }
+  end
+
+  let(:username) {  "pedant-oc-test-user-#{rand(10**7...10**8).to_s}" }
+  let(:password) { "foobar" } # hardcoded at the platform layer
+  let(:user_overrides) do
+    {
+      :overrides => {
+        "first_name" => "#{username}-first-name",
+        "middle_name" => "#{username}-middle-name",
+        "last_name" => "#{username}-last-name",
+        "display_name" => "#{username}-display-name",
+        "email" => "#{username}[email protected]",
+      }
+    }
+  end
+
+  # create user before each test so that we don't regen the keys
+  # of the standard user used in other tests.
+  let(:oc_id_user) { platform.create_user(username, user_overrides) }
+
+  # delete user after each test
+  after(:each) do
+    platform.delete_user(oc_id_user)
+  end
+
   context "status endpoint" do
     let(:request_url) { "#{platform.server}/id/v1/status" }
     let(:good_status) do
@@ -11,55 +42,37 @@
     end
 
     context "GET /id/v1/status" do
-      it "retuns 200" do
+      it "returns 200" do
         get(request_url, platform.superuser).should look_like({:status => 200,
                                                                :body => good_status})
       end
     end
   end
 
-  let(:username) {  "pedant-oc-test-user-#{rand(10**7...10**8).to_s}" }
-
-  let(:password) { "foobar" } #hardcoded at the platform layer
-
-  # create user before each test so that we don't regen the keys
-  # of the standard user used in other tests.
-  let(:reset_user) {
-    platform.create_user(username)
-  }
-
-  # delete user after each test
-  after(:each) do
-    platform.delete_user(reset_user)
+  def signin(with_callback = true)
+    response = get("#{platform.server}/id/signin", oc_id_user, headers: {"Accept" => "text/html"})
+    cookie = response.headers[:set_cookie][1].split(";").first
+    # I KNOW. I'll leave it up to reviewers whether we should pull
+    # in nokogiri or hpricot just do to this
+    re = /<meta name="csrf-token" content="(.*)" \/>/
+    token = response.match(re)[1]
+    headers = { "Content-Type" => "application/x-www-form-urlencoded", "Cookie" => cookie }
+    body = "username=#{username}&password=#{password}&authenticity_token=#{CGI.escape(token)}&commit=Sign+In"
+    # Now sign in
+    if with_callback
+      response = post("#{platform.server}/id/auth/chef/callback", oc_id_user, headers: headers, payload: body)
+      cookie = response.headers[:set_cookie][1].split(";").first
+    end
+    { cookie: cookie, token: token}
   end
 
   context "key reset" do
 
-    let(:request_headers) do
-      {
-        "Content-Type" => "application/x-www-form-urlencoded",
-        "Cookie" => csrf[:cookie]
-      }
-    end
-
-    let(:csrf) do
-      response = get("#{platform.server}/id/signin", reset_user, headers: {"Accept" => "text/html"})
-      cookie = response.headers[:set_cookie][1].split(";").first
-      # I KNOW. I'll leave it up to reviewers whether we should pull
-      # in nokogiri or hpricot just do to this
-      re = /<meta name="csrf-token" content="(.*)" \/>/
-      token = response.match(re)[1]
-      headers = { "Content-Type" => "application/x-www-form-urlencoded", "Cookie" => cookie }
-      body = "username=#{username}&password=#{password}&authenticity_token=#{CGI.escape(token)}&commit=Sign+In"
-      # Now sign in
-      response = post("#{platform.server}/id/auth/chef/callback", reset_user, headers: headers, payload: body)
-      cookie = response.headers[:set_cookie][1].split(";").first
-      { cookie: cookie, token: token}
-    end
-
+    let(:csrf) { signin }
     let(:request_url) { "#{platform.server}/id/profile/regen_key" }
     let(:request_body) { "authenticity_token=#{CGI.escape(csrf[:token])}&commit=Get+a+New+Key" }
-    let(:response) { post(request_url, reset_user, headers: request_headers, payload: request_body) }
+    let(:response) { post(request_url, oc_id_user, headers: request_headers, payload: request_body) }
+
     it "returns a non-zero file" do
       expect(response.code).to eq(200)
       expect(response.body.length).to_not eq(0)
@@ -69,44 +82,30 @@
       expect(response.code).to eq(200)
       # This will raise if it isn't a valid key
       expect(OpenSSL::PKey::RSA.new(response.body).class).to eq(OpenSSL::PKey::RSA)
-      expect(response.body).to_not eq(reset_user.signing_key.to_s)
+      expect(response.body).to_not eq(oc_id_user.signing_key.to_s)
     end
   end
 
   context "signin" do
     let(:request_url) { "#{platform.server}/id/auth/chef/callback" }
     let(:request_body) { "username=#{username}&password=#{password}&authenticity_token=#{CGI.escape(csrf[:token])}&commit=Sign+In" }
-    let(:request_headers) do
-      {
-        "Content-Type" => "application/x-www-form-urlencoded",
-        "Cookie" => csrf[:cookie]
-      }
-    end
-
-    let(:csrf) do
-      response = get("#{platform.server}/id/signin", platform.superuser, headers: {"Accept" => "text/html"})
-      cookie = response.headers[:set_cookie][1].split(";").first
-      # I KNOW. I'll leave it up to reviewers whether we should pull
-      # in nokogiri or hpricot just do to this
-      re = /<meta name="csrf-token" content="(.*)" \/>/
-      token = response.match(re)[1]
-      { cookie: cookie, token: token}
-    end
-
-    let(:response) { post(request_url, reset_user, headers: request_headers, payload: request_body) }
+    let(:csrf) { signin(false) }
+    let(:response) { post(request_url, oc_id_user, headers: request_headers, payload: request_body) }
 
     context "with correct password" do
-      let(:password) { "foobar" } #hardcoded at the platform layer
+      let(:password) { "foobar" } # hardcoded at the platform layer
+
       context "POST /id/auth/chef/callback" do
         it "redirects us to authorized applications" do
           expect(response.code).to eq(302)
-          expect(response.headers[:location]).to match(%r{/id/oauth/authorized_applications})
+          expect(response.headers[:location]).to end_with('/id/oauth/authorized_applications')
         end
       end
     end
 
     context "with incorrect password" do
       let(:password) { "WRONGWRONGWRONG" }
+
       context "POST /id/auth/chef/callback" do
         it "redirects us to an error" do
           expect(response.code).to eq(302)
@@ -115,4 +114,97 @@
       end
     end
   end
+
+  context "/id/profile#show" do
+    let(:csrf) { signin }
+    let(:request_url) { "#{platform.server}/id/profile" }
+    let(:request_body) { "authenticity_token=#{CGI.escape(csrf[:token])}" }
+    let(:response) { get(request_url, oc_id_user, headers: request_headers, payload: request_body) }
+
+    it "shows the profile" do
+      expect(response.code).to eq(200)
+      expect(response.body).to match(%r{Signed in as: #{username}})
+      expect(response.body).to match(/<input[^>]*value="#{username}-first-name"[^>]*id="user_first_name"\W*\/>/)
+      expect(response.body).to match(/<input[^>]*value="#{username}-middle-name"[^>]*id="user_middle_name"\W*\/>/)
+      expect(response.body).to match(/<input[^>]*value="#{username}-last-name"[^>]*id="user_last_name"\W*\/>/)
+      expect(response.body).to match(/<input[^>]*value="#{username}-email@host\.com"[^>]*id="user_email"\W*\/>/)
+    end
+  end
+
+  # NOTE 2017/06/02 sr: we don't update the email, since that makes oc_id send
+  #                     an email and that's not something we'd like to do from
+  #                     pedant running against real deployments
+  context "/id/profile#update" do
+    let(:csrf) { signin }
+    let(:request_url) { "#{platform.server}/id/profile" }
+    let(:request_body) { "authenticity_token=#{CGI.escape(csrf[:token])}&"\
+                         "#{CGI.escape('user[first_name]')}=#{username.upcase}-FIRST-NAME&"\
+                         "#{CGI.escape('user[last_name]')}=#{username.upcase}-LAST-NAME&"\
+                         "#{CGI.escape('user[middle_name]')}=#{username.upcase}-MIDDLE-NAME&"\
+                         "_method=PUT&"\
+                         "commit=Save+Changes" }
+    let(:response) { post(request_url, oc_id_user, headers: request_headers, payload: request_body) }
+
+    # NOTE: We're using the API here because another query for profile#show
+    # would, with test code laid out like this, cause another user to be created
+    # and used for login -- hence we'd see that other user's profile.
+    # For the same reason, everything happens in one it block :/
+    it "redirects to /id/profile and updates the user record" do
+      expect(response.code).to eq(302)
+      expect(response.headers[:location]).to end_with('/id/profile')
+
+      request_url = "#{platform.server}/users/#{username}"
+      user_body = {
+        "first_name" => "#{username.upcase}-FIRST-NAME",
+        "middle_name" => "#{username.upcase}-MIDDLE-NAME",
+        "last_name" => "#{username.upcase}-LAST-NAME",
+        "email" => "#{username}[email protected]", # don't want to update, see note
+        # these currently can't be changed via profile update
+        "display_name" => "#{username}-display-name",
+        "username" => username,
+        "public_key" => oc_id_user.signing_key.public_key.to_s + "\n", # What?
+      }
+      get(request_url, platform.superuser).should look_like({
+        :status => 200,
+        :body_exact => user_body
+      })
+    end
+  end
+
+  context "/id/profile#password" do
+    let(:new_password) { "barfoo" }
+    let(:csrf) { signin }
+    let(:request_url) { "#{platform.server}/id/profile/password" }
+    let(:request_body) { "authenticity_token=#{CGI.escape(csrf[:token])}&"\
+                         "username=#{username}&"\
+                         "current_password=#{password}&"\
+                         "new_password=#{new_password}&"\
+                         "password_confirmation=#{new_password}&"\
+                         "_method=PUT" }
+    let(:response) { post(request_url, oc_id_user, headers: request_headers, payload: request_body) }
+
+    it "redirects to /id/profile and updates the user's password" do
+      expect(response.code).to eq(302)
+      expect(response.headers[:location]).to end_with('/id/profile')
+
+      request_url = "#{platform.server}/authenticate_user"
+      request_body = { username: username, password: new_password }
+      post(request_url, platform.superuser, payload: request_body).should look_like({
+        :status => 200,
+        :body => { "user" => { "username" => username } },
+      })
+    end
+  end
+
+  #
+  # Untested endpoints
+  #
+  # The following endpoints are not tested because they require
+  # additional test-system setup.
+  #
+  untested_endpoint("/v1/users", "requires a configured oc-id application")
+  untested_endpoint("/v1/me", "requires a configured oc-id application")
+  untested_endpoint("/v1/me/organizations", "requires a configured oc-id application")
+  untested_endpoint("/zendesk", "requires zendseks configuration")
+  untested_endpoint("/password-reset", "requires email configuration")
 end

src/oc-id/app/controllers/profiles_controller.rb

@@ -37,7 +37,7 @@ def update
   end
 
   #
-  # PUT /id/profile/change_password
+  # PUT /id/profile/password
   #
   # Changes the user's password.
   #