feat: store app-only Graph credentials in the staff-api tenant (PPT-2000)

spec/admin_consent_flow_spec.cr

@@ -8,7 +8,7 @@ module PlaceOS::Api
 
       loaded = AdminConsentFlow.load(flow.id).not_nil!
       loaded.state.should eq "running"
-      loaded.steps.size.should eq 4
+      loaded.steps.size.should eq 5
       loaded.steps.all? { |step| step.state == "pending" }.should be_true
       loaded.redirect.should eq "/backoffice/#/domains/authority-test/authentication"
 

spec/tenant_consent_spec.cr

@@ -0,0 +1,114 @@
+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
+
+      it "creates a staff-api tenant holding the app-only credential when the domain has none" do
+        domain = "consent-create-#{Random::Secure.hex(4)}.example.com"
+        begin
+          tenant = TenantConsent.upsert_calendar_tenant(domain, azure_tenant, app, "Test Org")
+
+          tenant.persisted?.should be_true
+          tenant.name.should eq "Test Org"
+          tenant.platform.should eq "office365"
+          tenant.delegated.should be_false
+
+          stored = ::PlaceOS::Model::Tenant.find_by?(domain: domain).not_nil!
+          # encrypted at rest, and decrypts back to exactly the credential written
+          stored.credentials.starts_with?('\e').should be_true
+          creds = JSON.parse(PlaceOS::Encryption.decrypt(string: stored.credentials, id: domain, level: :never_display))
+          creds["tenant"].as_s.should eq azure_tenant
+          creds["client_id"].as_s.should eq app[:client_id]
+          creds["client_secret"].as_s.should eq app[:client_secret]
+
+          # the app-only calendar client builds from the stored credential
+          stored.place_calendar_client.should be_a(PlaceCalendar::Client)
+        ensure
+          ::PlaceOS::Model::Tenant.find_by?(domain: domain).try &.destroy
+        end
+      end
+
+      # the flow can be driven from Backoffice on any host, so it must key off
+      # the authority being integrated, never the host in the request
+      it "writes to the authority's own domain, leaving another domain's tenant alone" do
+        authority_domain = "consent-authority-#{Random::Secure.hex(4)}.example.com"
+        other_domain = "consent-bystander-#{Random::Secure.hex(4)}.example.com"
+        begin
+          bystander = ::PlaceOS::Model::Tenant.create!(
+            name: "Bystander", domain: other_domain, platform: "office365",
+            delegated: true, credentials: {conference_type: "teamsForBusiness"}.to_json,
+          )
+          before = ::PlaceOS::Model::Tenant.find!(bystander.id).credentials
+
+          TenantConsent.upsert_calendar_tenant(authority_domain, azure_tenant, app, "Authority Org")
+
+          ::PlaceOS::Model::Tenant.find_by?(domain: authority_domain).should_not be_nil
+          untouched = ::PlaceOS::Model::Tenant.find!(bystander.id)
+          untouched.delegated.should be_true
+          untouched.credentials.should eq before
+        ensure
+          ::PlaceOS::Model::Tenant.find_by?(domain: authority_domain).try &.destroy
+          ::PlaceOS::Model::Tenant.find_by?(domain: other_domain).try &.destroy
+        end
+      end
+
+      it "switches an existing delegated tenant onto the app-only credential" do
+        domain = "consent-update-#{Random::Secure.hex(4)}.example.com"
+        begin
+          existing = ::PlaceOS::Model::Tenant.create!(
+            name: "Existing", domain: domain, platform: "office365",
+            delegated: true, credentials: {conference_type: "teamsForBusiness"}.to_json,
+          )
+
+          tenant = TenantConsent.upsert_calendar_tenant(domain, azure_tenant, app, "ignored - tenant exists")
+
+          # same row updated, not a duplicate
+          tenant.id.should eq existing.id
+          ::PlaceOS::Model::Tenant.where(domain: domain).count.should eq 1
+
+          stored = ::PlaceOS::Model::Tenant.find_by?(domain: domain).not_nil!
+          stored.name.should eq "Existing"
+          stored.delegated.should be_false
+          creds = JSON.parse(PlaceOS::Encryption.decrypt(string: stored.credentials, id: domain, level: :never_display))
+          creds["client_id"].as_s.should eq app[:client_id]
+          stored.place_calendar_client.should be_a(PlaceCalendar::Client)
+        ensure
+          ::PlaceOS::Model::Tenant.find_by?(domain: domain).try &.destroy
+        end
+      end
+    end
+  end
+end

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
@@ -84,27 +105,35 @@ module PlaceOS::Api
     @flow : AdminConsentFlow? = nil
 
     private def run_consent_flow(flow : AdminConsentFlow, tenant_id : String, authority : ::PlaceOS::Model::Authority) : Nil
+      # Everything this flow configures belongs to the authority being
+      # integrated, which is not necessarily the host the admin happens to be
+      # browsing - Backoffice can drive the flow for any domain.
+      authority_domain = authority.domain
+
       flow.start_step("visualiser")
-      create_app(tenant_id)
+      visualiser_app = create_app(tenant_id)
+
+      flow.start_step("calendar")
+      self.class.upsert_calendar_tenant(authority_domain, tenant_id, visualiser_app, authority.name)
 
       flow.start_step("auth_app")
       strat = create_strat(tenant_id, authority.id.as(String))
-      auth_app = create_delegated_app(tenant_id, authority.domain, strat.id.as(String))
+      auth_app = create_delegated_app(tenant_id, authority_domain, strat.id.as(String))
 
       flow.start_step("outlook")
       create_outlook_repo
-      add_outlook_plugin_auth(auth_app[:client_id])
-      create_outlook_config(auth_app[:client_id])
+      add_outlook_plugin_auth(auth_app[:client_id], authority_domain)
+      create_outlook_config(auth_app[:client_id], authority_domain)
 
       flow.start_step("saving")
       strat.update!(client_id: auth_app[:client_id], client_secret: auth_app[:client_secret])
       update_auth(authority, strat.id.as(String))
 
       flow.complete!
       Log.info { {message: "admin consent flow complete", flow_id: flow.id, authority_id: flow.authority_id} }
-    rescue flow_error
-      Log.error(exception: flow_error) { {message: "admin consent flow failed", flow_id: flow.id, authority_id: flow.authority_id} }
-      flow.fail!(flow_error.message || flow_error.class.name)
+    rescue error
+      Log.error(exception: error) { {message: "admin consent flow failed", flow_id: flow.id, authority_id: flow.authority_id} }
+      flow.fail!(error.message || error.class.name)
     end
 
     # surfaces replication-retry waits on the progress page
@@ -225,6 +254,7 @@ module PlaceOS::Api
       ra << {id: "ef54d2bf-783f-4e0f-bca1-3210c0444d99", type: "Role"} # Calendars.ReadWrite
       ra << {id: "5b567255-7703-4780-807c-7be8301ae99b", type: "Role"} # Group.Read.All
       ra << {id: "df021288-bdef-4463-88db-98f22de89214", type: "Role"} # User.Read.All
+      ra << {id: "913b9306-0ce1-42b8-9137-6a7df690a760", type: "Role"} # Place.Read.All - room discovery via the places API
 
       client = get_client(tenant_id)
 
@@ -240,7 +270,8 @@ module PlaceOS::Api
         end
       end
 
-      created_app.app_id.as(String)
+      secret = GraphReplicationRetry.run(on_retry: replication_progress) { client.application_add_pwd(created_app.app_id.as(String), "PlaceOS Bookings Visualiser Secret") }
+      {client_id: created_app.app_id.as(String), client_secret: secret.secret_text.as(String)}
     end
 
     private def create_delegated_app(tenant_id : String, domain : String, strat_id : String)
@@ -296,16 +327,16 @@ module PlaceOS::Api
       end
     end
 
-    private def add_outlook_plugin_auth(app_id : String) : Nil
+    private def add_outlook_plugin_auth(app_id : String, domain : String) : Nil
       client = get_client
       app = GraphReplicationRetry.run(on_retry: replication_progress) { client.get_application(app_id) }
       app_redirect_uris = app.web.try &.redirect_uris || [] of String
-      app_redirect_uris.push("#{domain_url}/outlook/#/book/spaces")
+      app_redirect_uris.push("https://#{domain}/outlook/#/book/spaces")
 
       scope_id = UUID.v4.to_s
 
       updated = {
-        "identifierUris": ["api://#{domain_host}/#{app_id}"],
+        "identifierUris": ["api://#{domain}/#{app_id}"],
         "web":            {
           "redirectUris":          app_redirect_uris,
           "implicitGrantSettings": {"enableAccessTokenIssuance" => true, "enableIdTokenIssuance" => true},
@@ -342,6 +373,36 @@ module PlaceOS::Api
       GraphReplicationRetry.run(on_retry: replication_progress) { client.update_application(app_id, updated.to_json) }
     end
 
+    # Store the visualiser app's app-only Graph credential in the staff-api
+    # tenant for this domain - this is what gives PlaceOS calendar access
+    # without a signed-in user. The flow owns the domain's Microsoft
+    # configuration (like login_url and outlook_config), so an existing
+    # tenant is switched to these credentials; delegated mode can be
+    # re-enabled afterwards in Backoffice if a customer prefers it.
+    def self.upsert_calendar_tenant(domain : String, azure_tenant_id : String, app : NamedTuple(client_id: String, client_secret: String), name : String?) : ::PlaceOS::Model::Tenant
+      credentials = {
+        tenant:        azure_tenant_id,
+        client_id:     app[:client_id],
+        client_secret: app[:client_secret],
+      }.to_json
+
+      if tenant = ::PlaceOS::Model::Tenant.find_by?(domain: domain)
+        tenant.platform = "office365"
+        tenant.delegated = false
+        tenant.credentials = credentials
+        tenant.save!
+        tenant
+      else
+        ::PlaceOS::Model::Tenant.create!(
+          name: name,
+          domain: domain,
+          platform: "office365",
+          delegated: false,
+          credentials: credentials,
+        )
+      end
+    end
+
     private def create_outlook_repo : Nil
       return if on_primary { ::PlaceOS::Model::Repository.where(name: "Outlook Plugin", uri: "https://github.com/placeos/user-interfaces", branch: "build/outlook-rooms-addin/prod",
                   folder_name: "outlookplugin", repo_type: ::PlaceOS::Model::Repository::Type::Interface.value).count } > 0
@@ -352,16 +413,16 @@ module PlaceOS::Api
       )
     end
 
-    private def create_outlook_config(app_id : String) : Nil
-      tenant = ::PlaceOS::Model::Tenant.find_by?(domain: domain_host)
+    private def create_outlook_config(app_id : String, domain : String) : Nil
+      tenant = ::PlaceOS::Model::Tenant.find_by?(domain: domain)
       unless tenant
-        Log.error { {message: "Tenant not found", domain: domain_host} }
+        Log.error { {message: "Tenant not found", domain: domain} }
         return
       end
 
       outlook_config = {
-        app_id: app_id, base_path: "outlook", app_domain: "#{domain_url}/outlook/",
-        app_resource: "api://#{domain_host}/#{app_id}", source_location: "",
+        app_id: app_id, base_path: "outlook", app_domain: "https://#{domain}/outlook/",
+        app_resource: "api://#{domain}/#{app_id}", source_location: "",
       }
       tenant.outlook_config = ::PlaceOS::Model::Tenant::OutlookConfig.from_json(outlook_config.to_json)
       tenant.save!

src/placeos-rest-api/utilities/admin-consent-flow.cr

@@ -14,6 +14,7 @@ module PlaceOS::Api
 
     STEP_DEFINITIONS = [
       {"visualiser", "Register the Bookings Visualiser application"},
+      {"calendar", "Connect room calendar access"},
       {"auth_app", "Register the User Authentication application"},
       {"outlook", "Configure the Outlook add-in"},
       {"saving", "Save the authentication configuration"},
@@ -62,7 +63,7 @@ module PlaceOS::Api
     end
 
     def complete! : Nil
-      @steps.each { |step| step.state = "done" }
+      @steps.each(&.state=("done"))
       @state = "complete"
       @detail = nil
       save

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