PPT-2536: auth.cr drop-in fixes (public clients, verified cookie, PKCE, refresh identity, session secret, OAuth provider params)

spec/controllers/oauth_alias_spec.cr

@@ -19,6 +19,7 @@ module PlaceOS::Auth
       app.redirect_uri = redirect
       app.scopes = scopes
       app.owner_id = user.id.as(String)
+      app.confidential = true
       app.save!
       {user, app}
     }

spec/controllers/oauth_provider_flows_spec.cr

@@ -393,5 +393,77 @@ module PlaceOS::Auth
         strat.try &.destroy
       end
     end
+
+    # ---- authorize_params + info_mappings comma-fallback (PPT-2536) ----
+
+    describe "authorize_params" do
+      it "merges the strat's authorize_params into the outbound authorize URL" do
+        strat = new_oauth_strat.call(
+          "https://accounts.google.com", "/o/oauth2/v2/auth", "/token",
+          "https://openidconnect.googleapis.com/v1/userinfo",
+          "openid email", {"uid" => "sub", "email" => "email"},
+        )
+        strat.authorize_params = {"access_type" => "offline", "prompt" => "consent"}
+        strat.save!
+
+        k = kickoff.call(strat.id.as(String))
+        params = URI::Params.parse(k[:location].split('?', 2).last)
+        # the extra params Ruby merged (Google refresh-token flow) are present
+        params["access_type"].should eq "offline"
+        params["prompt"].should eq "consent"
+        # and the standard params + embedded redirect_uri are untouched
+        params["client_id"].should eq strat.client_id
+        params["response_type"].should eq "code"
+        params["redirect_uri"].should eq "http://localhost/auth/oauth2/callback?id=#{strat.id}"
+      ensure
+        strat.try &.destroy
+      end
+    end
+
+    describe "info_mappings comma-fallback" do
+      it "resolves through comma-separated mapping keys, incl. the getter-only uid" do
+        strat = new_oauth_strat.call(
+          "https://login.microsoftonline.com", "/common/oauth2/v2.0/authorize",
+          "/common/oauth2/v2.0/token", "https://graph.microsoft.com/oidc/userinfo",
+          "openid email profile",
+          {
+            "uid"   => "id,sub,oid",                   # getter-only field, 3rd key wins
+            "email" => "email,mail,userPrincipalName", # settable field, 3rd key wins
+            "name"  => "name,displayName",
+          },
+        )
+        uid = "az-oid-#{Random.rand(999999)}"
+        upn = "grace-#{Random.rand(999999)}@contoso.com"
+
+        WebMock.stub(:post, "https://login.microsoftonline.com/common/oauth2/v2.0/token").to_return(
+          status: 200, headers: json_headers,
+          body: {access_token: "az-access", token_type: "Bearer", expires_in: 3600}.to_json,
+        )
+        # profile carries ONLY the fallback keys: `oid` (not id/sub) and
+        # `userPrincipalName` (not email/mail) and `displayName` (not name)
+        WebMock.stub(:get, "https://graph.microsoft.com/oidc/userinfo").to_return(
+          status: 200, headers: json_headers,
+          body: {oid: uid, userPrincipalName: upn, displayName: "Grace Hopper"}.to_json,
+        )
+
+        k = kickoff.call(strat.id.as(String))
+        result = client.get(
+          "/auth/oauth2/callback?id=#{URI.encode_www_form(strat.id.as(String))}&code=az-code&state=#{k[:state]}",
+          headers: HTTP::Headers{"Host" => "localhost", "Cookie" => k[:cookie]},
+        )
+        result.status_code.should eq 303
+
+        # the lookup key is derived from `uid` — proves comma-fallback fixed the
+        # getter-only uid field (only the get_value_from_json patch can)
+        lookup = ::PlaceOS::Model::UserAuthLookup.find?("auth-#{authority_id.call}-oauth2-#{uid}")
+        lookup.should_not be_nil
+        user = ::PlaceOS::Model::User.find!(lookup.not_nil!.user_id.not_nil!)
+        # and the settable email field fell back to userPrincipalName
+        user.email.to_s.should eq upn
+      ensure
+        cleanup_login.call(uid) if uid
+        strat.try &.destroy
+      end
+    end
   end
 end

spec/controllers/oauth_spec.cr

@@ -20,6 +20,10 @@ module PlaceOS::Auth
       app.redirect_uri = redirect
       app.scopes = scopes
       app.owner_id = user.id.as(String)
+      # These fixtures authenticate with a real client_secret, so they
+      # model a confidential client. Public clients are covered separately
+      # (see the "public client" describe block below).
+      app.confidential = true
       app.save!
       {user, app}
     }
@@ -189,6 +193,92 @@ module PlaceOS::Auth
       end
     end
 
+    # ---- POST /auth/token — public clients (no client_secret) ---------
+    #
+    # SPAs / native apps (`confidential: false`) can't hold a secret; they
+    # authenticate via PKCE. Doorkeeper let them exchange a code and refresh
+    # without a `client_secret`, and Backoffice relies on exactly this. See
+    # `AuthlyAdapter::Client#authorized?`.
+    describe "POST /auth/token (public client)" do
+      # Public variant of `new_application`: confidential is left false and
+      # no secret is ever presented at the token endpoint.
+      new_public_application = ->(redirect : String, scopes : String) {
+        authority = ::PlaceOS::Model::Authority.find_by_domain("localhost").not_nil!
+        user = ::PlaceOS::Model::Generator.user(authority)
+        password = "bcrypt-please-#{Random.rand(99999)}"
+        user.password = password
+        user.save!
+        app = ::PlaceOS::Model::DoorkeeperApplication.new
+        app.name = "public-test-#{Random.rand(99999)}"
+        app.redirect_uri = redirect
+        app.scopes = scopes
+        app.owner_id = user.id.as(String)
+        app.confidential = false
+        app.save!
+        {user, app, password}
+      }
+
+      it "exchanges a code and refreshes with no client_secret" do
+        user, app, password = new_public_application.call("https://spa.example/cb", "public")
+        cookie = Spec.signin!(client, user, password)
+
+        authorize_result = client.get(
+          "/auth/authorize?response_type=code" \
+          "&client_id=#{URI.encode_www_form(app.uid.as(String))}" \
+          "&redirect_uri=#{URI.encode_www_form("https://spa.example/cb")}" \
+          "&scope=public",
+          headers: HTTP::Headers{"Host" => "localhost", "Cookie" => cookie},
+        )
+        authorize_result.status_code.should eq 302
+        location = authorize_result.headers["Location"]
+        code = URI::Params.parse(location.split('?', 2).last)["code"]
+
+        # Token exchange WITHOUT client_secret — this is the drop-in fix.
+        token_result = form_post.call("/auth/token", {
+          "grant_type"   => "authorization_code",
+          "client_id"    => app.uid.as(String),
+          "code"         => code,
+          "redirect_uri" => "https://spa.example/cb",
+        })
+        token_result.status_code.should eq 200
+        token_body = JSON.parse(token_result.body)
+        token_body["access_token"].as_s.should_not be_empty
+        refresh_token = token_body["refresh_token"].as_s
+        refresh_token.should_not be_empty
+
+        # Refresh, again with no client_secret.
+        refresh_result = form_post.call("/auth/token", {
+          "grant_type"    => "refresh_token",
+          "client_id"     => app.uid.as(String),
+          "refresh_token" => refresh_token,
+        })
+        refresh_result.status_code.should eq 200
+        JSON.parse(refresh_result.body)["access_token"].as_s.should_not be_empty
+      ensure
+        app.try &.destroy
+        user.try &.destroy
+      end
+
+      it "denies the client_credentials grant to a public client" do
+        user, app, _password = new_public_application.call("https://spa.example/cb2", "public")
+
+        # Even presenting the app's real secret, a public client must not be
+        # able to mint client-credentials tokens — the grant is confidential
+        # only, so a leaked (public) client_id can't be used as a principal.
+        result = form_post.call("/auth/token", {
+          "grant_type"    => "client_credentials",
+          "client_id"     => app.uid.as(String),
+          "client_secret" => app.secret,
+          "scope"         => "public",
+        })
+        result.status_code.should eq 401
+        JSON.parse(result.body)["error"].as_s.should eq "unauthorized_client"
+      ensure
+        app.try &.destroy
+        user.try &.destroy
+      end
+    end
+
     # ---- GET /auth/authorize -----------------------------------------
 
     describe "GET /auth/authorize" do

spec/controllers/oauth_token_claims_spec.cr

@@ -32,6 +32,7 @@ module PlaceOS::Auth
       app.redirect_uri = "https://app.example/cb"
       app.scopes = scopes
       app.owner_id = user.id.as(String)
+      app.confidential = true
       app.save!
 
       cookie = Spec.signin!(client, user, password)
@@ -149,6 +150,7 @@ module PlaceOS::Auth
         app.redirect_uri = "https://app.example/cb"
         app.scopes = "public"
         app.owner_id = user.id.as(String)
+        app.confidential = true
         app.save!
 
         headers = HTTP::Headers{

spec/controllers/pkce_spec.cr

@@ -0,0 +1,104 @@
+require "../helper"
+require "digest/sha256"
+require "base64"
+
+module PlaceOS::Auth
+  # PKCE (RFC 7636) encoding compatibility. ts-client / Backoffice send the
+  # S256 `code_challenge` as base64url (`-`/`_`), but authly validates it
+  # against Crystal's standard-base64 `Digest::SHA256.base64digest`. Without
+  # normalization every real browser PKCE login fails with
+  # `unauthorized_client`. `OAuth#normalize_code_challenge` bridges this.
+  describe OAuth, tags: "pkce" do
+    make_app = ->(redirect : String) {
+      authority = ::PlaceOS::Model::Authority.find_by_domain("localhost").not_nil!
+      user = ::PlaceOS::Model::Generator.user(authority)
+      password = "bcrypt-please-#{Random.rand(99999)}"
+      user.password = password
+      user.save!
+      app = ::PlaceOS::Model::DoorkeeperApplication.new
+      app.name = "pkce-test-#{Random.rand(99999)}"
+      app.redirect_uri = redirect
+      app.scopes = "public"
+      app.owner_id = user.id.as(String)
+      app.save!
+      {user, app, password}
+    }
+
+    # Mirror ts-client `generateChallenge`: base64url(SHA256(verifier)) with
+    # `+`->`-`, `/`->`_`, padding kept.
+    ts_client_challenge = ->(verifier : String) {
+      Base64.strict_encode(Digest::SHA256.digest(verifier)).tr("+/", "-_")
+    }
+
+    it "accepts a base64url S256 challenge (ts-client / browser style)" do
+      user, app, password = make_app.call("https://spa.example/cb")
+      cookie = Spec.signin!(client, user, password)
+      verifier = "dBjftJeZ4CVPmB92K27uhbUJU1p1r-wW1gFWFOEjXk"
+      challenge = ts_client_challenge.call(verifier)
+      # sanity: the challenge really is url-safe (would break authly raw)
+      challenge.should match(/[-_]/)
+
+      authorize = client.get(
+        "/auth/authorize?response_type=code" \
+        "&client_id=#{URI.encode_www_form(app.uid.as(String))}" \
+        "&redirect_uri=#{URI.encode_www_form("https://spa.example/cb")}" \
+        "&scope=public&code_challenge=#{URI.encode_www_form(challenge)}" \
+        "&code_challenge_method=S256",
+        headers: HTTP::Headers{"Host" => "localhost", "Cookie" => cookie},
+      )
+      authorize.status_code.should eq 302
+      code = URI::Params.parse(authorize.headers["Location"].split('?', 2).last)["code"]
+
+      body = URI::Params.build do |fp|
+        fp.add("grant_type", "authorization_code")
+        fp.add("client_id", app.uid.as(String))
+        fp.add("client_secret", app.secret)
+        fp.add("code", code)
+        fp.add("redirect_uri", "https://spa.example/cb")
+        fp.add("code_verifier", verifier)
+      end
+      token = client.post("/auth/token", headers: HTTP::Headers{
+        "Host" => "localhost", "Content-Type" => "application/x-www-form-urlencoded",
+      }, body: body)
+
+      token.status_code.should eq 200
+      JSON.parse(token.body)["access_token"].as_s.should_not be_empty
+    ensure
+      app.try &.destroy
+      user.try &.destroy
+    end
+
+    it "rejects a valid challenge presented with the wrong verifier" do
+      user, app, password = make_app.call("https://spa.example/cb2")
+      cookie = Spec.signin!(client, user, password)
+      challenge = ts_client_challenge.call("the-real-verifier-aaaaaaaaaaaaaaaaaaaaaaaa")
+
+      authorize = client.get(
+        "/auth/authorize?response_type=code" \
+        "&client_id=#{URI.encode_www_form(app.uid.as(String))}" \
+        "&redirect_uri=#{URI.encode_www_form("https://spa.example/cb2")}" \
+        "&scope=public&code_challenge=#{URI.encode_www_form(challenge)}" \
+        "&code_challenge_method=S256",
+        headers: HTTP::Headers{"Host" => "localhost", "Cookie" => cookie},
+      )
+      code = URI::Params.parse(authorize.headers["Location"].split('?', 2).last)["code"]
+
+      body = URI::Params.build do |fp|
+        fp.add("grant_type", "authorization_code")
+        fp.add("client_id", app.uid.as(String))
+        fp.add("client_secret", app.secret)
+        fp.add("code", code)
+        fp.add("redirect_uri", "https://spa.example/cb2")
+        fp.add("code_verifier", "a-different-verifier-bbbbbbbbbbbbbbbbbbbbbbbb")
+      end
+      token = client.post("/auth/token", headers: HTTP::Headers{
+        "Host" => "localhost", "Content-Type" => "application/x-www-form-urlencoded",
+      }, body: body)
+
+      token.status_code.should eq 401
+    ensure
+      app.try &.destroy
+      user.try &.destroy
+    end
+  end
+end