Merge pull request #5 from PlaceOS/PPT-2536-ensure-matching-continue

PPT-2536: enforce ensure_matching + fix continue lost through SSO

Author: camreeves

spec/controllers/oauth_provider_flows_spec.cr

@@ -423,6 +423,121 @@ module PlaceOS::Auth
       end
     end
 
+    # ---- post-login continue + ensure_matching (PPT-2536) --------------
+
+    session_cookie = ->(result : HTTP::Client::Response, fallback : String) {
+      sc = result.cookies[PlaceOS::Auth::SESSION_COOKIE_NAME]?
+      sc ? "#{sc.name}=#{sc.value}" : fallback
+    }
+
+    google_urls = ->(scopes : String, mappings : Hash(String, String)) {
+      new_oauth_strat.call(
+        "https://accounts.google.com", "/o/oauth2/v2/auth", "/token",
+        "https://openidconnect.googleapis.com/v1/userinfo", scopes, mappings,
+      )
+    }
+
+    describe "post-login continue" do
+      it "returns to the app that started the login (continue survives the SSO round-trip)" do
+        strat = google_urls.call("openid email", {"uid" => "sub", "email" => "email"})
+        uid = "google-cont-#{Random.rand(999999)}"
+
+        WebMock.stub(:post, "https://accounts.google.com/token").to_return(
+          status: 200, headers: json_headers,
+          body: {access_token: "g-access", token_type: "Bearer", expires_in: 3600}.to_json,
+        )
+        WebMock.stub(:get, "https://openidconnect.googleapis.com/v1/userinfo").to_return(
+          status: 200, headers: json_headers,
+          body: {sub: uid, email: "cont-#{Random.rand(999999)}@localhost"}.to_json,
+        )
+
+        # Hop 1: the login entrypoint stores `continue` on the session and
+        # bounces to the provider kickoff.
+        login = client.get(
+          "/auth/login?provider=oauth2&id=#{URI.encode_www_form(strat.id.as(String))}&continue=%2Fbackoffice%2F",
+          headers: HTTP::Headers{"Host" => "localhost"},
+        )
+        login.status_code.should eq 303
+        cookie = session_cookie.call(login, "")
+        cookie.should_not be_empty
+
+        # Hop 2: kickoff -> IdP redirect (state minted, session updated).
+        kick = client.get(login.headers["Location"], headers: HTTP::Headers{
+          "Host" => "localhost", "Cookie" => cookie,
+        })
+        kick.status_code.should eq 303
+        state = URI::Params.parse(kick.headers["Location"].split('?', 2).last)["state"]
+        cookie = session_cookie.call(kick, cookie)
+
+        # Hop 3: provider callback -> must land back on /backoffice/.
+        result = client.get(
+          "/auth/oauth2/callback?id=#{URI.encode_www_form(strat.id.as(String))}&code=g-code&state=#{state}",
+          headers: HTTP::Headers{"Host" => "localhost", "Cookie" => cookie},
+        )
+        result.status_code.should eq 303
+        result.headers["Location"].should eq "/backoffice/"
+      ensure
+        cleanup_login.call(uid) if uid
+        strat.try &.destroy
+      end
+    end
+
+    describe "ensure_matching" do
+      make_restricted_strat = -> {
+        strat = google_urls.call("openid email", {"uid" => "sub", "email" => "email"})
+        strat.ensure_matching = {"email" => ["@corp.example$"]}
+        strat.save!
+        strat
+      }
+
+      stub_round_trip = ->(email : String, uid : String) {
+        WebMock.stub(:post, "https://accounts.google.com/token").to_return(
+          status: 200, headers: json_headers,
+          body: {access_token: "g-access", token_type: "Bearer", expires_in: 3600}.to_json,
+        )
+        WebMock.stub(:get, "https://openidconnect.googleapis.com/v1/userinfo").to_return(
+          status: 200, headers: json_headers,
+          body: {sub: uid, email: email}.to_json,
+        )
+      }
+
+      it "rejects a login whose userinfo fails the restriction (-> /auth/failure)" do
+        strat = make_restricted_strat.call
+        uid = "google-em-reject-#{Random.rand(999999)}"
+        stub_round_trip.call("eve@evil.test", uid)
+
+        k = kickoff.call(strat.id.as(String))
+        result = client.get(
+          "/auth/oauth2/callback?id=#{URI.encode_www_form(strat.id.as(String))}&code=g-code&state=#{k[:state]}",
+          headers: HTTP::Headers{"Host" => "localhost", "Cookie" => k[:cookie]},
+        )
+        result.status_code.should eq 302
+        result.headers["Location"].should eq "/auth/failure"
+        # and no local account was minted for the rejected identity
+        ::PlaceOS::Model::UserAuthLookup.find?("auth-#{authority_id.call}-oauth2-#{uid}").should be_nil
+      ensure
+        cleanup_login.call(uid) if uid
+        strat.try &.destroy
+      end
+
+      it "admits a login whose userinfo satisfies the restriction" do
+        strat = make_restricted_strat.call
+        uid = "google-em-pass-#{Random.rand(999999)}"
+        stub_round_trip.call("alice@corp.example", uid)
+
+        k = kickoff.call(strat.id.as(String))
+        result = client.get(
+          "/auth/oauth2/callback?id=#{URI.encode_www_form(strat.id.as(String))}&code=g-code&state=#{k[:state]}",
+          headers: HTTP::Headers{"Host" => "localhost", "Cookie" => k[:cookie]},
+        )
+        result.status_code.should eq 303
+        ::PlaceOS::Model::UserAuthLookup.find?("auth-#{authority_id.call}-oauth2-#{uid}").should_not be_nil
+      ensure
+        cleanup_login.call(uid) if uid
+        strat.try &.destroy
+      end
+    end
+
     # ---- authorize_params + info_mappings comma-fallback (PPT-2536) ----
 
     describe "authorize_params" do

src/placeos-auth/controllers/provider_callbacks.cr

@@ -127,6 +127,15 @@ module PlaceOS::Auth
         end
       end
 
+      # Pop the pre-login `continue` target NOW: `session_user` below can tear
+      # down a stale session, and `remove_session`/`new_session` deliberately
+      # clear the session store — all of which used to wipe the target stashed
+      # by `/auth/login` before it was read, so every SSO login landed on "/"
+      # instead of returning to the app that started it. Ruby preserved
+      # `continue` across the provider round-trip; consuming it early here
+      # restores that.
+      continue_target = consume_continue
+
       redirect_uri = callback_uri(provider, id)
       engine = ::MultiAuth.make(provider, redirect_uri, id)
 
@@ -149,6 +158,21 @@ module PlaceOS::Auth
         return
       end
 
+      # Enforce the OAuth strategy's hosted-domain / attribute restriction
+      # (`ensure_matching`) against the userinfo we just fetched. The legacy
+      # Ruby strategy did this in `generic_oauth#raw_info`, raising
+      # "Invalid Hosted Domain" which OmniAuth turned into a `/auth/failure`
+      # redirect. Without this, an authority that restricted OAuth logins to
+      # a corporate domain would admit any account from the provider. SAML
+      # flows share this action but have no oauth strat, so gate on the
+      # oauth2 provider.
+      if provider == ExternalProviders::OAUTH2_PROVIDER &&
+         !ExternalProviders.ensure_matching?(id, oauth_user.raw_json)
+        Log.warn { {action: "provider_callbacks.callback", message: "ensure_matching restriction rejected login", provider: provider} }
+        redirect_to "/auth/failure", :found
+        return
+      end
+
       session_user_for_link = session_user
       result = Utils::OAuthUserMapper.map(
         authority: authority,
@@ -163,7 +187,7 @@ module PlaceOS::Auth
 
       LoginEvents.record_login(result.user, oauth_user.provider)
 
-      target = consume_continue || "/"
+      target = continue_target || "/"
       redirect_to target.gsub(' ', "%20"), :see_other
     end
 

src/placeos-auth/external_providers.cr

@@ -72,6 +72,59 @@ module PlaceOS::Auth::ExternalProviders
     )
   end
 
+  # Enforce the OAuth strategy's `ensure_matching` hosted-domain /
+  # attribute restriction against the userinfo JSON the provider returned.
+  # Mirrors the legacy Ruby `OmniAuth::Strategies::GenericOauth#raw_info`
+  # ("Invalid Hosted Domain"): for *every* configured field, at least one
+  # of that field's values must match (case-insensitive, unanchored regex)
+  # at least one of its configured patterns. A missing field — or a field
+  # whose values match no pattern — fails. Returns `true` when there is
+  # nothing to enforce (no strat, or empty config) or when every field
+  # passes; `false` otherwise. Fails closed: an unparseable pattern simply
+  # cannot match anything.
+  def self.ensure_matching?(provider_id : String?, raw_json : String) : Bool
+    strat = find_oauth_strat(provider_id)
+    return true if strat.nil?
+
+    required = strat.ensure_matching
+    return true if required.empty?
+
+    info = JSON.parse(raw_json)
+    required.all? do |field, patterns|
+      values = field_values(info, field)
+      regexes = patterns.compact_map do |pattern|
+        Regex.new(pattern, Regex::CompileOptions::IGNORE_CASE) rescue nil
+      end
+      values.any? { |value| regexes.any?(&.matches?(value)) }
+    end
+  end
+
+  # Coerce a userinfo JSON field into the list of string values to test.
+  # Matches Ruby's `Array(inf[field])`: missing/null -> [], scalar ->
+  # [scalar], array -> its (stringified) elements.
+  private def self.field_values(info : JSON::Any, field : String) : Array(String)
+    value = info[field]?
+    return [] of String if value.nil?
+
+    case raw = value.raw
+    when String
+      [raw]
+    when Array(JSON::Any)
+      raw.compact_map do |element|
+        element_raw = element.raw
+        case element_raw
+        when Nil    then nil
+        when String then element_raw
+        else             element_raw.to_s
+        end
+      end
+    when Nil
+      [] of String
+    else
+      [raw.to_s]
+    end
+  end
+
   # ---- SAML ---------------------------------------------------------
 
   def self.find_saml_strat(provider_id : String?) : ::PlaceOS::Model::SamlAuthentication?