FEATURE: consume Person API v2

Adds an API class for Person API v2. Returns a profile with public
and verified attributes. Returns a full_name based off first_name,
last_name and alternative_name.

Also adds better logging/error messages to failed OAuth requests.

https://github.com/mozilla/discourse/issues/182

config/settings.yml

@@ -15,3 +15,9 @@ mozilla_iam:
   mozilla_iam_person_api_aud:
     default: 'https://person-api.sso.mozilla.com'
     shadowed_by_global: true
+  mozilla_iam_person_v2_api_url:
+    default: "https://person.api.test.sso.allizom.org"
+    shadowed_by_global: true
+  mozilla_iam_person_v2_api_aud:
+    default: "api.test.sso.allizom.org"
+    shadowed_by_global: true

lib/mozilla_iam.rb

@@ -5,6 +5,7 @@ require_relative 'mozilla_iam/serializer_extensions/mozilla_iam'
 require_relative 'mozilla_iam/api'
 require_relative 'mozilla_iam/api/oauth'
 require_relative 'mozilla_iam/api/person'
+require_relative 'mozilla_iam/api/person_v2'
 require_relative 'mozilla_iam/api/management'
 require_relative 'mozilla_iam/application_extensions'
 require_relative 'mozilla_iam/authenticator'

lib/mozilla_iam/api/oauth.rb

@@ -32,6 +32,11 @@ module MozillaIAM
         if res.code == '200'
           MultiJson.load(res.body, symbolize_keys: true)
         else
+          Rails.logger.error <<~EOF
+            Mozilla IAM: Error fetching /#{path}
+            client_id: #{@client_id}, url: #{@url}
+            response: #{res.body}
+          EOF
           {}
         end
       end
@@ -63,7 +68,15 @@ module MozillaIAM
               audience: @aud
             }
           )
-        MultiJson.load(response.body)['access_token']
+        if response.status == 200
+          MultiJson.load(response.body)['access_token']
+        else
+          raise <<~EOF
+            Mozilla IAM: Error fetching OAuth token:
+            client_id: #{@client_id}, token_endpoint: #{@token_endpoint}, audience: #{@aud}
+            response: #{response.body}
+          EOF
+        end
       end
 
       def verify_token(token)

lib/mozilla_iam/api/person_v2.rb

@@ -0,0 +1,63 @@
+module MozillaIAM
+  module API
+    class PersonV2 < OAuth
+
+      def initialize(config={})
+        config = {
+          url: SiteSetting.mozilla_iam_person_v2_api_url + "/v2",
+          aud: SiteSetting.mozilla_iam_person_v2_api_aud
+        }.merge(config)
+        super(config)
+      end
+
+      def profile(uid)
+        res = get("user/user_id/#{uid}")
+        Profile.new(res)
+      end
+
+      class Profile
+        attr_reader :username
+        attr_reader :pronouns
+        attr_reader :full_name
+        attr_reader :fun_title
+        attr_reader :description
+        attr_reader :location
+
+        def initialize(raw)
+          @raw = raw
+          @username = process :primary_username
+          @pronouns = process :pronouns
+          @full_name = process_full_name
+          @fun_title = process :fun_title
+          @description = process :description
+          @location = process :location
+        end
+
+        private
+
+        def process(name)
+          if field = @raw[name]
+            metadata = field[:metadata]
+            if metadata[:display] == "public" && metadata[:verified] == true
+              return field[:value]
+            end
+          end
+          nil
+        end
+
+        def process_full_name
+          first = process :first_name
+          last = process :last_name
+          return "#{first} #{last}" unless first.blank? || last.blank?
+
+          alternative = process :alternative_name
+          return "#{alternative}" unless alternative.blank?
+
+          return "#{first}" unless first.blank?
+          return "#{last}" unless last.blank?
+        end
+
+      end
+    end
+  end
+end

plugin.rb

@@ -1,6 +1,6 @@
 # name: mozilla-iam
 # about: A plugin to integrate Discourse with Mozilla's Identity and Access Management (IAM) system
-# version: 1.2.0-alpha.1
+# version: 1.2.0-alpha.2
 # authors: Leo McArdle
 # url: https://github.com/mozilla/discourse-mozilla-iam
 

spec/components/mozilla_iam/api/oauth_spec.rb

@@ -85,9 +85,10 @@ describe MozillaIAM::API::OAuth do
       expect(api.send(:get, "", foo: 'bar')[:success]).to eq "true"
     end
 
-    it "should return an empty hash if the the status code isn't 200" do
-      stub_request(:get, "https://example.com/api/").to_return(status: 403)
-      expect(api.send(:get, "")).to eq({})
+    it "logs error and returns empty hash if the the status code isn't 200" do
+      stub_request(:get, "https://example.com/api/user/uid").to_return(status: 403)
+      Rails.logger.expects(:error).with { |e| e.start_with? "Mozilla IAM: Error fetching /user/uid"}
+      expect(api.send(:get, "user/uid")).to eq({})
     end
   end
 
@@ -136,6 +137,12 @@ describe MozillaIAM::API::OAuth do
       token = api.send(:fetch_token)
       expect(token).to eq "fetched_token"
     end
+
+    it "throws error if fetch isn't successful" do
+      stub_request(:post, "https://example.com/oauth/token")
+        .to_return(status: 403, body: '{"error":"Error"}')
+      expect { api.send(:fetch_token) }.to raise_error(/Mozilla IAM: Error fetching OAuth token:/)
+    end
   end
 
   context "#verify_token" do

spec/components/mozilla_iam/api/person_v2_spec.rb

@@ -0,0 +1,168 @@
+require_relative "../../../iam_helper"
+
+describe MozillaIAM::API::PersonV2 do
+  let(:api) { described_class.new }
+  before do
+    SiteSetting.mozilla_iam_person_v2_api_url = "https://personv2.com"
+    SiteSetting.mozilla_iam_person_v2_api_aud = "personv2.com"
+  end
+
+  context "#initialize" do
+    it "sets url and aud based on SiteSetting" do
+      expect(api.instance_variable_get(:@url)).to eq "https://personv2.com/v2"
+      expect(api.instance_variable_get(:@aud)).to eq "personv2.com"
+    end
+  end
+
+  context "#profile" do
+    it "returns the profile for a specific user" do
+      api.expects(:get).with("user/user_id/uid").returns(profile: "profile")
+      expect(api.profile("uid").instance_variable_get(:@raw)).to eq({profile: "profile"})
+    end
+
+    it "returns an empty hash if a profile doesn't exist" do
+      api.expects(:get).with("user/user_id/uid").returns({})
+      expect(api.profile("uid").instance_variable_get(:@raw)).to eq({})
+    end
+  end
+
+  describe described_class::Profile do
+    def single_attribute(value=nil, metadata={})
+      metadata[:verified] = true if metadata[:verified].nil?
+      metadata[:public] = true if metadata[:public].nil?
+      {
+        metadata: {
+          verified: metadata[:verified],
+          display: metadata[:public] ? "public" : "staff"
+        },
+        value: value
+      }
+    end
+
+    def profile_with(attributes, value=nil, metadata={})
+      raw = {}
+      if attributes.is_a? Hash
+        attributes.each do |name, value|
+          raw[name] = single_attribute(value)
+        end
+      else
+        raw[attributes] = single_attribute(value, metadata)
+      end
+      described_class.new(raw)
+    end
+
+    shared_examples "one-to-one mapping" do |method, attribute, value|
+      describe "##{method}" do
+        context "with no #{attribute} attribute" do
+          let(:profile) { described_class.new({}) }
+
+          it "returns nil" do
+            expect(profile.public_send(method)).to be_nil
+          end
+        end
+
+        context "with unverified #{attribute} attribute" do
+          let(:profile) { profile_with(attribute, value, verified: false) }
+
+          it "returns nil" do
+            expect(profile.public_send(method)).to be_nil
+          end
+        end
+
+        context "with non-public #{attribute} attribute" do
+          let(:profile) { profile_with(attribute, value, public: false) }
+
+          it "returns nil" do
+            expect(profile.public_send(method)).to be_nil
+          end
+        end
+
+        let(:profile) { profile_with(attribute, value) }
+
+        it "returns #{attribute}" do
+          expect(profile.public_send(method)).to eq value
+        end
+      end
+    end
+
+    include_examples "one-to-one mapping", :username, :primary_username, "janedoe"
+    include_examples "one-to-one mapping", :pronouns, :pronouns, "she/her"
+    include_examples "one-to-one mapping", :fun_title, :fun_title, "Fun job title"
+    include_examples "one-to-one mapping", :description, :description, "I have a fun job"
+    include_examples "one-to-one mapping", :location, :location, "Somewhere"
+
+    describe "#full_name" do
+      let(:profile) do
+        profile_with ({
+          first_name: "Jane",
+          last_name: "Doe",
+          alternative_name: "Janette Smith"
+        })
+      end
+
+      it "returns first_name + last_name" do
+        expect(profile.full_name).to eq "Jane Doe"
+      end
+
+      context "without first_name" do
+        let(:profile) do
+          profile_with ({
+            last_name: "Doe",
+            alternative_name: "Janette Smith"
+          })
+        end
+
+        it "returns alternative_name" do
+          expect(profile.full_name).to eq "Janette Smith"
+        end
+
+        context "without alternative_name" do
+          let(:profile) do
+            profile_with ({
+              last_name: "Doe"
+            })
+          end
+
+          it "returns last_name" do
+            expect(profile.full_name).to eq "Doe"
+          end
+        end
+      end
+
+      context "without last_name" do
+        let(:profile) do
+          profile_with ({
+            first_name: "Jane",
+            alternative_name: "Janette Smith"
+          })
+        end
+
+        it "returns alternative_name" do
+          expect(profile.full_name).to eq "Janette Smith"
+        end
+
+        context "without alternative_name" do
+          let(:profile) do
+            profile_with ({
+              first_name: "Jane"
+            })
+          end
+
+          it "returns first_name" do
+            expect(profile.full_name).to eq "Jane"
+          end
+        end
+      end
+
+      context "without first_name, last_name or alternative_name" do
+        let(:profile) do
+          profile_with({})
+        end
+
+        it "returns nil" do
+          expect(profile.full_name).to be_nil
+        end
+      end
+    end
+  end
+end