PPT-2536: external-IdP spec coverage (ID-06/07/08/11)

spec/controllers/provider_callbacks_spec.cr

@@ -267,5 +267,255 @@ module PlaceOS::Auth
         strat.try &.destroy
       end
     end
+
+    # ---- info_mappings comma-fallback (matrix ID-08) -----------------
+    #
+    # The legacy Ruby `generic_oauth` strategy treated each info_mappings value
+    # as a COMMA-SEPARATED FALLBACK LIST, using the first key present in the
+    # provider's profile — notably how Azure/Entra surfaces the email as
+    # `userPrincipalName` rather than `email`. multi_auth does a single literal
+    # lookup, so a comma-list resolves to nil and the email arrives EMPTY.
+    #
+    # When that fallback was lost, real logins produced users with no email,
+    # which then duplicated on the next sign-in. `multi_auth_patch.cr` restores
+    # it; these pin both sides of the fallback so it cannot silently regress.
+
+    describe "info_mappings comma-fallback", tags: "idp-mapping" do
+      azure_strat = ->(site : String) {
+        strat = create_strat.call(site, "openid email")
+        strat.info_mappings = {
+          "uid"   => "id",
+          "email" => "email,mail,userPrincipalName",
+          "name"  => "displayName,name",
+        }
+        strat.save!
+        strat
+      }
+
+      callback = ->(strat_id : String, data : NamedTuple(cookie: String, state: String)) {
+        client.get(
+          "/auth/oauth2/callback?id=#{URI.encode_www_form(strat_id)}&code=test-code&state=#{data[:state]}",
+          headers: HTTP::Headers{"Host" => "localhost", "Cookie" => data[:cookie]},
+        )
+      }
+
+      user_for = ->(uid : String) {
+        lookup = ::PlaceOS::Model::UserAuthLookup.where(uid: uid, provider: "oauth2").first?
+        lookup.nil? ? nil : ::PlaceOS::Model::User.find!(lookup.user_id.as(String))
+      }
+
+      it "falls back to a later key when earlier ones are absent (Azure userPrincipalName)" do
+        strat = azure_strat.call("https://idp.example.test")
+        uid = "azure-uid-#{Random.rand(99999)}"
+        upn = "upn-#{Random.rand(99999)}@localhost"
+
+        stub_token_endpoint.call("https://idp.example.test", "idp-token-xyz")
+        # NB: neither `email` nor `mail` is present — only the third candidate.
+        stub_userinfo.call("https://idp.example.test", {
+          "id"                => uid,
+          "userPrincipalName" => upn,
+          "displayName"       => "Azure Person",
+        })
+
+        callback.call(strat.id.as(String), kickoff.call(strat.id.as(String))).status_code.should eq 303
+
+        created = user_for.call(uid)
+        created.should_not be_nil
+        # Without the patch this is "" — and the account duplicates next login.
+        created.not_nil!.email.to_s.should eq upn
+        created.not_nil!.name.should eq "Azure Person"
+      ensure
+        strat.try &.destroy
+      end
+
+      it "prefers the FIRST present key over later fallbacks" do
+        strat = azure_strat.call("https://idp.example.test")
+        uid = "azure-uid-#{Random.rand(99999)}"
+        primary = "primary-#{Random.rand(99999)}@localhost"
+
+        stub_token_endpoint.call("https://idp.example.test", "idp-token-xyz")
+        stub_userinfo.call("https://idp.example.test", {
+          "id"                => uid,
+          "email"             => primary,
+          "mail"              => "wrong-#{Random.rand(99999)}@localhost",
+          "userPrincipalName" => "also-wrong-#{Random.rand(99999)}@localhost",
+          "displayName"       => "Primary Person",
+        })
+
+        callback.call(strat.id.as(String), kickoff.call(strat.id.as(String)))
+
+        user_for.call(uid).not_nil!.email.to_s.should eq primary
+      ensure
+        strat.try &.destroy
+      end
+
+      it "skips blank candidates rather than mapping an empty value" do
+        strat = azure_strat.call("https://idp.example.test")
+        uid = "azure-uid-#{Random.rand(99999)}"
+        upn = "upn-#{Random.rand(99999)}@localhost"
+
+        stub_token_endpoint.call("https://idp.example.test", "idp-token-xyz")
+        # `email` is present but EMPTY — the fallback must continue past it,
+        # not stop and map "".
+        stub_userinfo.call("https://idp.example.test", {
+          "id"                => uid,
+          "email"             => "",
+          "userPrincipalName" => upn,
+          "displayName"       => "Blank First Person",
+        })
+
+        callback.call(strat.id.as(String), kickoff.call(strat.id.as(String)))
+
+        user_for.call(uid).not_nil!.email.to_s.should eq upn
+      ensure
+        strat.try &.destroy
+      end
+    end
+
+    # ---- authorize_params merge (matrix ID-07) -----------------------
+    #
+    # The legacy Ruby service merged the strat's `authorize_params` column into
+    # the outbound authorize URL. Dropping it is silent but consequential:
+    # Google returns NO refresh_token without `access_type=offline`, so every
+    # session dies at the first token expiry with no way to renew.
+
+    describe "authorize_params merge", tags: "idp-authorize-params" do
+      it "merges the strat's authorize_params into the authorize URL" do
+        strat = create_strat.call("https://idp.example.test", "openid")
+        strat.authorize_params = {"access_type" => "offline", "prompt" => "consent"}
+        strat.save!
+
+        result = client.get(
+          "/auth/oauth2?id=#{URI.encode_www_form(strat.id.as(String))}",
+          headers: HTTP::Headers{"Host" => "localhost"},
+        )
+        result.status_code.should eq 303
+        params = URI::Params.parse(result.headers["Location"].split('?', 2).last)
+        params["access_type"].should eq "offline"
+        params["prompt"].should eq "consent"
+        # the merge must not disturb the embedded redirect_uri
+        params["redirect_uri"].should eq "http://localhost/auth/oauth2/callback?id=#{strat.id}"
+      ensure
+        strat.try &.destroy
+      end
+
+      it "leaves the authorize URL untouched when none are configured" do
+        strat = create_strat.call("https://idp.example.test", "openid")
+        result = client.get(
+          "/auth/oauth2?id=#{URI.encode_www_form(strat.id.as(String))}",
+          headers: HTTP::Headers{"Host" => "localhost"},
+        )
+        location = result.headers["Location"]
+        location.should_not contain "access_type"
+        location.should_not contain "&&"
+      ensure
+        strat.try &.destroy
+      end
+    end
+
+    # ---- ensure_matching fails closed (matrix ID-06) -----------------
+    #
+    # The hosted-domain restriction (Ruby's "Invalid Hosted Domain"). Every
+    # configured field must have at least one value matching at least one
+    # pattern. Dropping it admits ANY account the IdP will authenticate — for a
+    # multi-tenant IdP that is the whole internet.
+    #
+    # NB: `oauth_provider_flows_spec.cr` already covers the match and
+    # non-match cases. The gap was the ABSENT-field case, which is the one
+    # that actually matters: a missing field must FAIL, not pass vacuously.
+    # Only that case is added here, to avoid duplicating existing coverage.
+
+    describe "ensure_matching", tags: "idp-ensure-matching" do
+      hd_strat = -> {
+        strat = create_strat.call("https://idp.example.test", "openid email")
+        strat.ensure_matching = {"hd" => ["example\\.com"]}
+        strat.save!
+        strat
+      }
+
+      run_callback = ->(strat : ::PlaceOS::Model::OAuthAuthentication, profile : Hash(String, String)) {
+        stub_token_endpoint.call("https://idp.example.test", "idp-token-xyz")
+        stub_userinfo.call("https://idp.example.test", profile)
+        data = kickoff.call(strat.id.as(String))
+        client.get(
+          "/auth/oauth2/callback?id=#{URI.encode_www_form(strat.id.as(String))}&code=test-code&state=#{data[:state]}",
+          headers: HTTP::Headers{"Host" => "localhost", "Cookie" => data[:cookie]},
+        )
+      }
+
+      it "FAILS CLOSED when the required field is absent entirely" do
+        strat = hd_strat.call
+        result = run_callback.call(strat, {
+          "id"    => "hd-missing-#{Random.rand(99999)}",
+          "email" => "missing-#{Random.rand(99999)}@localhost",
+          "name"  => "No Domain Person",
+          # no `hd` key at all
+        })
+        result.status_code.should eq 302
+        result.headers["Location"].should contain "/auth/failure"
+      ensure
+        strat.try &.destroy
+      end
+    end
+
+    # ---- OAuthUserMapper Link branch (matrix ID-11) ------------------
+    #
+    # Branch (2): a user who is ALREADY signed in completes a callback for a
+    # provider identity we have never seen. That must attach the new identity
+    # to the existing account, not mint a second one — otherwise linking a
+    # second IdP silently forks the user.
+
+    describe "linking a provider to a signed-in user", tags: "idp-link" do
+      it "links the new identity to the session user instead of creating one" do
+        authority = ::PlaceOS::Model::Authority.find_by_domain("localhost").not_nil!
+        strat = create_strat.call("https://idp.example.test", "openid email")
+
+        password = "link-branch-pw-#{Random.rand(99999)}"
+        existing = ::PlaceOS::Model::Generator.user(authority)
+        existing.password = password
+        existing.save!
+
+        # Counted AFTER `existing` is created, so a correct link leaves this
+        # unchanged. If the mapper wrongly took the Created branch instead,
+        # the count goes UP by one — that is the account silently forking.
+        count_with_existing = ::PlaceOS::Model::User.count
+
+        session = Spec.signin!(client, existing, password)
+
+        # kickoff mutates the EXISTING session (adds oauth_state), so the
+        # signed-in uid survives into the callback.
+        kick = client.get(
+          "/auth/oauth2?id=#{URI.encode_www_form(strat.id.as(String))}",
+          headers: HTTP::Headers{"Host" => "localhost", "Cookie" => session},
+        )
+        kick.status_code.should eq 303
+        cookie = kick.cookies[PlaceOS::Auth::SESSION_COOKIE_NAME].try { |c| "#{c.name}=#{c.value}" } || session
+        state = URI::Params.parse(kick.headers["Location"].split('?', 2).last)["state"]
+
+        uid = "link-uid-#{Random.rand(99999)}"
+        stub_token_endpoint.call("https://idp.example.test", "idp-token-xyz")
+        stub_userinfo.call("https://idp.example.test", {
+          "id"    => uid,
+          "email" => "someone-else-#{Random.rand(99999)}@localhost",
+          "name"  => "Linked Identity",
+        })
+
+        result = client.get(
+          "/auth/oauth2/callback?id=#{URI.encode_www_form(strat.id.as(String))}&code=test-code&state=#{state}",
+          headers: HTTP::Headers{"Host" => "localhost", "Cookie" => cookie},
+        )
+        result.status_code.should eq 303
+
+        lookup = ::PlaceOS::Model::UserAuthLookup.where(uid: uid, provider: "oauth2").first?
+        lookup.should_not be_nil
+        # the crux: the new identity points at the ALREADY signed-in user
+        lookup.not_nil!.user_id.should eq existing.id
+        # no SECOND account was minted for the new provider identity
+        ::PlaceOS::Model::User.count.should eq count_with_existing
+      ensure
+        strat.try &.destroy
+        existing.try &.destroy
+      end
+    end
   end
 end