fix(consent): authenticate the consent flow and make state a single-use token

The Azure admin-consent callback is unauthenticated, and it has to be —
Microsoft redirects the browser to it, so there is no session to present.
Everything it then does is driven by its query parameters, and `state`
was the bare authority id: echoed, never verified.

That let anyone who could reach the deployment reconfigure any authority.
The attacker needs no PlaceOS credentials at all: sign up for a free
Entra tenant, grant admin consent to the PlaceOS app as its own global
admin (the product's intended flow, open to any Microsoft admin), read
the victim's authority id from the unauthenticated /auth/authority
endpoint, then issue one GET at the victim's host with `state` set to it.
The flow builds an oauth strat pointing at the attacker's directory and
writes it to `authority.login_url`, so every user of that domain is
redirected from the genuine PlaceOS URL into an identity provider the
attacker controls. There is no Host binding either, so one request from
anywhere can retarget any authority in the deployment, and a captured
callback URL replays unchanged.

The stacked #442 work makes this materially worse: `upsert_calendar_tenant`
blind-overwrites the staff-api tenant's `platform`, `delegated` and
`credentials`. The same anonymous request therefore replaces a live
customer's Microsoft Graph credential with one minted in the attacker's
directory — encrypted in place with no prior value retained, so it is
unrecoverable — and repoints PlaceOS's server-side calendar client at a
directory they own.

Two changes:

- Starting a flow now requires an administrator. `index` was in the
  `skip_action :authorize!` list along with the callback; only the
  callback needs to be there.

- `state` is now an opaque single-use token (`ConsentState`) rather than
  the authority id. It exists only because an authenticated admin asked
  to start a flow for a specific authority, names that authority server
  side rather than in the URL, expires after 15 minutes, and redeeming it
  destroys it. The callback redeems rather than trusting, so an unknown,
  expired or replayed state is refused. Redemption uses the delete's
  reply count so a concurrent replay cannot slip through the window
  before the delete lands.

Specs cover redeem-once, refuse-twice and refuse-unissued. They could not
be run locally — the Docker spec harness OOMs compiling this repo — so
they run in CI. `crystal build --no-codegen` passes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: camreeves

spec/tenant_consent_spec.cr

@@ -2,6 +2,36 @@ require "./helper"
 
 module PlaceOS::Api
   describe TenantConsent do
+    describe ".consent_state" do
+      # `state` comes back from Microsoft through the user's browser and is the
+      # only thing telling the callback which authority to reconfigure. The
+      # callback cannot be authenticated - Microsoft redirects to it - so these
+      # properties are the whole of its protection.
+      it "redeems a token for the authority it was issued to" do
+        authority_id = "authority-#{Random::Secure.hex(4)}"
+        token = ConsentState.issue(authority_id)
+
+        token.should_not eq authority_id
+        ConsentState.consume(token).should eq authority_id
+      end
+
+      it "refuses a token a second time" do
+        # A captured callback URL must be worthless once it has been used.
+        authority_id = "authority-#{Random::Secure.hex(4)}"
+        token = ConsentState.issue(authority_id)
+
+        ConsentState.consume(token).should eq authority_id
+        ConsentState.consume(token).should be_nil
+      end
+
+      it "refuses a token it never issued" do
+        # i.e. an attacker naming an authority directly, which is what the
+        # parameter used to be.
+        ConsentState.consume("authority-1").should be_nil
+        ConsentState.consume(UUID.random.to_s).should be_nil
+      end
+    end
+
     describe ".upsert_calendar_tenant" do
       app = {client_id: UUID.v4.to_s, client_secret: "sup3r-s3cret"}
       azure_tenant = UUID.v4.to_s

src/placeos-rest-api/controllers/tenant_consent.cr

@@ -6,8 +6,15 @@ module PlaceOS::Api
   class TenantConsent < Application
     base "/api/engine/v2/admin_consent"
 
-    skip_action :authorize!, only: [:index, :azure_admin_consent_callback, :flow_status]
-    skip_action :set_user_id, only: [:index, :azure_admin_consent_callback, :flow_status]
+    # Only the callback is unauthenticated, and it has to be: Microsoft
+    # redirects the browser here, so there is no session to present. It is
+    # guarded instead by the single-use `state` token minted below — see
+    # `ConsentState`. Starting a flow requires an administrator, because that
+    # is what decides which authority the callback is allowed to reconfigure.
+    skip_action :authorize!, only: [:azure_admin_consent_callback, :flow_status]
+    skip_action :set_user_id, only: [:azure_admin_consent_callback, :flow_status]
+
+    before_action :check_admin, only: [:index]
 
     @[AC::Route::Filter(:before_action)]
     def get_host
@@ -29,7 +36,11 @@ module PlaceOS::Api
       authority = ::PlaceOS::Model::Authority.find!(id)
       update_app_redirect_uri
       callback_url = URI.encode_www_form(redirect_url)
-      consent_url = "https://login.microsoftonline.com/common/adminconsent?client_id=#{PLACE_APP_CLIENT_ID}&redirect_uri=#{callback_url}&state=#{authority.id.as(String)}"
+      # Not the authority id. `state` comes back from Microsoft through the
+      # user's browser and is the only thing telling the callback which
+      # authority to reconfigure, so it has to be unguessable and single use.
+      state = ConsentState.issue(authority.id.as(String))
+      consent_url = "https://login.microsoftonline.com/common/adminconsent?client_id=#{PLACE_APP_CLIENT_ID}&redirect_uri=#{callback_url}&state=#{URI.encode_www_form(state)}"
       render json: {"url": consent_url}
     end
 
@@ -46,7 +57,17 @@ module PlaceOS::Api
       @[AC::Param::Info(description: "Description of the error", example: "The admin denied the request")]
       error_description : String? = nil,
     ) : Nil
-      if ((consent = admin_consent) && consent) && (tenant_id = tenant) && (authority_id = state)
+      if ((consent = admin_consent) && consent) && (tenant_id = tenant) && (consent_state = state)
+        # Redeem the token rather than trusting the parameter. This is what
+        # stops an unauthenticated caller naming an authority of their choosing
+        # and having the rest of this method reconfigure it. Redeeming is
+        # single use, so a captured callback URL cannot be replayed either.
+        authority_id = ConsentState.consume(consent_state)
+        unless authority_id
+          Log.warn { "Rejected admin consent callback with an unknown, expired or already used state" }
+          raise Error::NotFound.new("Invalid state value returned in admin consent")
+        end
+
         Log.info { "Received admin consent for tenant #{tenant_id} under authority #{authority_id}" }
         authority = ::PlaceOS::Model::Authority.find?(authority_id)
         raise Error::NotFound.new("Invalid state value returned in admin consent") unless authority

src/placeos-rest-api/utilities/consent-state.cr

@@ -0,0 +1,57 @@
+require "uuid"
+
+module PlaceOS::Api
+  # The `state` parameter for an Azure admin-consent round trip.
+  #
+  # Microsoft redirects the browser to `/admin_consent/callback`, so that route
+  # cannot carry a session and cannot be authenticated. Everything the callback
+  # goes on to do — registering applications, writing an oauth strat, replacing
+  # the staff API tenant's calendar credentials, and overwriting the authority's
+  # `login_url` — is therefore driven entirely by its query parameters.
+  #
+  # Passing the authority id there directly, as this flow used to, means anyone
+  # who can reach the deployment can point any authority at an identity provider
+  # they control, because `state` was echoed rather than verified.
+  #
+  # So `state` is an opaque single-use token instead. It exists only because an
+  # authenticated administrator asked to start a flow for a specific authority,
+  # it names that authority server side rather than carrying it in the URL, it
+  # expires, and redeeming it destroys it. A captured callback URL is worthless
+  # once used, and a guessed one is worth nothing at all.
+  module ConsentState
+    # Long enough for an admin to read Microsoft's consent screen and decide.
+    TTL_SECONDS = 900
+
+    # Records a pending flow for `authority_id` and returns the token to send
+    # to Microsoft as `state`.
+    def self.issue(authority_id : String) : String
+      token = UUID.random.to_s
+      ::PlaceOS::Driver::RedisStorage.with_redis(&.set(
+        redis_key(token), authority_id, ex: TTL_SECONDS
+      ))
+      token
+    end
+
+    # Redeems `token`, returning the authority it was issued for, or `nil` if it
+    # is unknown, expired, or already used.
+    def self.consume(token : String) : String?
+      key = redis_key(token)
+      ::PlaceOS::Driver::RedisStorage.with_redis do |redis|
+        authority_id = redis.get(key)
+        next nil unless authority_id
+
+        # `del` reports how many keys it removed. Anything other than one means
+        # a concurrent request redeemed this token first, and only that request
+        # may proceed — otherwise a replayed callback would still be honoured
+        # in the window before the delete lands.
+        next nil unless redis.del(key) == 1
+
+        authority_id
+      end
+    end
+
+    def self.redis_key(token : String) : String
+      "placeos:admin_consent:state:#{token}"
+    end
+  end
+end