From 04b002f18d7c987a2ea0d46971bfdc47360b699a Mon Sep 17 00:00:00 2001 From: Cam Reeves Date: Tue, 4 Aug 2026 20:22:59 +1000 Subject: [PATCH] feat(tenant_consent): live progress page for the admin-consent flow (PPT-2032) The consent callback previously held the browser for the full Graph sequence - up to minutes when directory replication is slow - while the tab still displayed Microsoft's consent page, indistinguishable from a hang. It also risked browser/proxy timeouts (54s callbacks measured against nginx's 60s default). Now the callback spawns the work into a fiber and responds immediately with a self-contained progress page (served by rest-api, no frontend build involvement) that polls GET /admin_consent/flow/:id and shows: - each step with pending/running/done/failed states - live replication-retry detail ("Waiting for Microsoft to replicate (attempt N)") via a new optional on_retry hook on GraphReplicationRetry - an elapsed timer, connectivity-loss notice, and a friendly error panel on failure - on completion: BroadcastChannel announcement (for any future Backoffice listener) and redirect to the domain's authentication tab (also fixes the double-slash redirect that landed on /domains/-/about) Flow state lives in redis with a 15 minute TTL so any replica can answer the polling. The flow id is an unguessable capability token and the payload contains no secrets. Specs cover the state machine. --- spec/admin_consent_flow_spec.cr | 53 +++++ .../controllers/tenant_consent.cr | 202 +++++++++++++++--- .../utilities/admin-consent-flow.cr | 94 ++++++++ .../utilities/graph-replication-retry.cr | 3 +- 4 files changed, 326 insertions(+), 26 deletions(-) create mode 100644 spec/admin_consent_flow_spec.cr create mode 100644 src/placeos-rest-api/utilities/admin-consent-flow.cr diff --git a/spec/admin_consent_flow_spec.cr b/spec/admin_consent_flow_spec.cr new file mode 100644 index 00000000..e0e45c09 --- /dev/null +++ b/spec/admin_consent_flow_spec.cr @@ -0,0 +1,53 @@ +require "./helper" + +module PlaceOS::Api + describe AdminConsentFlow do + it "round-trips flow state through redis" do + flow = AdminConsentFlow.new(UUID.v4.to_s, "authority-test", "/backoffice/#/domains/authority-test/authentication") + flow.save + + loaded = AdminConsentFlow.load(flow.id).not_nil! + loaded.state.should eq "running" + loaded.steps.size.should eq 4 + loaded.steps.all? { |step| step.state == "pending" }.should be_true + loaded.redirect.should eq "/backoffice/#/domains/authority-test/authentication" + + flow.start_step("auth_app") + loaded = AdminConsentFlow.load(flow.id).not_nil! + loaded.steps.find! { |step| step.key == "auth_app" }.state.should eq "running" + + flow.detail("Waiting for Microsoft to replicate (attempt 2)") + AdminConsentFlow.load(flow.id).not_nil!.detail.should eq "Waiting for Microsoft to replicate (attempt 2)" + + flow.complete! + loaded = AdminConsentFlow.load(flow.id).not_nil! + loaded.state.should eq "complete" + loaded.steps.all? { |step| step.state == "done" }.should be_true + loaded.detail.should be_nil + end + + it "advances earlier steps to done when a later step starts" do + flow = AdminConsentFlow.new(UUID.v4.to_s, "authority-test", "/x") + flow.start_step("visualiser") + flow.start_step("outlook") + loaded = AdminConsentFlow.load(flow.id).not_nil! + loaded.steps.find! { |step| step.key == "visualiser" }.state.should eq "done" + loaded.steps.find! { |step| step.key == "outlook" }.state.should eq "running" + loaded.steps.find! { |step| step.key == "saving" }.state.should eq "pending" + end + + it "marks the running step failed and records the error" do + flow = AdminConsentFlow.new(UUID.v4.to_s, "authority-test", "/x") + flow.start_step("visualiser") + flow.fail!("boom") + loaded = AdminConsentFlow.load(flow.id).not_nil! + loaded.state.should eq "failed" + loaded.error.should eq "boom" + loaded.steps.find! { |step| step.key == "visualiser" }.state.should eq "failed" + end + + it "returns nil for unknown flows" do + AdminConsentFlow.load("unknown-#{UUID.v4}").should be_nil + end + end +end diff --git a/src/placeos-rest-api/controllers/tenant_consent.cr b/src/placeos-rest-api/controllers/tenant_consent.cr index d91562cc..11b44097 100644 --- a/src/placeos-rest-api/controllers/tenant_consent.cr +++ b/src/placeos-rest-api/controllers/tenant_consent.cr @@ -6,8 +6,8 @@ module PlaceOS::Api class TenantConsent < Application base "/api/engine/v2/admin_consent" - skip_action :authorize!, only: [:index, :azure_admin_consent_callback] - skip_action :set_user_id, only: [:index, :azure_admin_consent_callback] + skip_action :authorize!, only: [:index, :azure_admin_consent_callback, :flow_status] + skip_action :set_user_id, only: [:index, :azure_admin_consent_callback, :flow_status] @[AC::Route::Filter(:before_action)] def get_host @@ -46,26 +46,178 @@ module PlaceOS::Api @[AC::Param::Info(description: "Description of the error", example: "The admin denied the request")] error_description : String? = nil, ) : Nil - redirect_back = "/backoffice/#/domains/" if ((consent = admin_consent) && consent) && (tenant_id = tenant) && (authority_id = state) 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 - begin - redirect_back = "#{redirect_back}/#{authority_id}/authentication" - create_app(tenant_id) - create_outlook_repo - strat = create_strat(tenant_id, authority.id.as(String)) - auth_app = create_delegated_app(tenant_id, authority.domain, strat.id.as(String)) - add_outlook_plugin_auth(auth_app[:client_id]) - create_outlook_config(auth_app[:client_id]) - strat.update!(client_id: auth_app[:client_id], client_secret: auth_app[:client_secret]) - update_auth(authority, strat.id.as(String)) - end + + flow = AdminConsentFlow.new(UUID.v4.to_s, authority_id, "/backoffice/#/domains/#{authority_id}/authentication") + flow.save + @flow = flow + + # The Microsoft Graph work can take minutes when directory replication + # is slow - run it in a fiber and respond immediately with a progress + # page that polls the flow status. Request-derived state (domain_host + # etc.) is already captured in ivars, safe to use after the response. + spawn { run_consent_flow(flow, tenant_id, authority) } + + render html: progress_page(flow.id) else Log.warn { {message: "Admin declined consent", error: error.to_s, description: error_description.to_s} } + redirect_to "/backoffice/#/domains/", status: :see_other end - redirect_to redirect_back, status: :see_other + end + + # progress of an in-flight admin-consent flow, polled by the progress page. + # The flow id is an unguessable capability token; the payload contains no + # secrets. + @[AC::Route::GET("/flow/:flow_id")] + def flow_status( + @[AC::Param::Info(description: "Flow identifier issued by the consent callback", example: "uuid-1234")] + flow_id : String, + ) : AdminConsentFlow + flow = AdminConsentFlow.load(flow_id) + raise Error::NotFound.new("unknown or expired flow") unless flow + flow + end + + @flow : AdminConsentFlow? = nil + + private def run_consent_flow(flow : AdminConsentFlow, tenant_id : String, authority : ::PlaceOS::Model::Authority) : Nil + flow.start_step("visualiser") + create_app(tenant_id) + + 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)) + + flow.start_step("outlook") + create_outlook_repo + add_outlook_plugin_auth(auth_app[:client_id]) + create_outlook_config(auth_app[:client_id]) + + 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) + end + + # surfaces replication-retry waits on the progress page + private def replication_progress : Proc(Int32, Nil)? + return nil unless flow = @flow + ->(attempt : Int32) { flow.detail("Waiting for Microsoft to replicate (attempt #{attempt})") } + end + + # Self-contained progress page shown in the consent tab while the fiber + # works. Polls the flow endpoint; no frontend build involvement. + private def progress_page(flow_id : String) : String + <<-HTML + + + + + + Microsoft Integration - PlaceOS + + + +
+

Setting up your Microsoft integration

+

Microsoft consent accepted — PlaceOS is registering the applications in your directory. This can take a couple of minutes while Microsoft replicates changes.

+ +
+
+ +
+ + + + HTML end private def create_app(tenant_id : String) @@ -79,11 +231,11 @@ module PlaceOS::Api app = Office365::Application.single_tenant_app("PlaceOS Bookings Visualiser") .add_required_resource(ra) - created_app = GraphReplicationRetry.run { client.create_application(app) } + created_app = GraphReplicationRetry.run(on_retry: replication_progress) { client.create_application(app) } Log.debug { {message: "App registerd with Application permissions", tenant: tenant_id, client_id: created_app.app_id.as(String)} } ra.each do |resource| - GraphReplicationRetry.run do + GraphReplicationRetry.run(on_retry: replication_progress) do client.application_add_app_role_assignment(created_app.app_id.as(String), resource["id"]) end end @@ -107,13 +259,13 @@ module PlaceOS::Api .add_web_redirect_uri("https://#{domain}/auth/oauth2/callback?id=#{strat_id}") .add_required_resource(ra) - created_app = GraphReplicationRetry.run { client.create_application(app) } + created_app = GraphReplicationRetry.run(on_retry: replication_progress) { client.create_application(app) } Log.debug { {message: "App registerd with Delegated permissions", tenant: tenant_id, client_id: created_app.app_id.as(String)} } - GraphReplicationRetry.run do + GraphReplicationRetry.run(on_retry: replication_progress) do client.application_add_oauth2_permission_grant(created_app.app_id.as(String), "Calendars.ReadWrite Calendars.ReadWrite.Shared Group.Read.All User.Read.All offline_access openid profile") end - secret = GraphReplicationRetry.run { client.application_add_pwd(created_app.app_id.as(String), "PlaceOS User Auth Secret") } + secret = GraphReplicationRetry.run(on_retry: replication_progress) { client.application_add_pwd(created_app.app_id.as(String), "PlaceOS User Auth Secret") } {client_id: created_app.app_id.as(String), client_secret: secret.secret_text.as(String)} end @@ -123,7 +275,7 @@ module PlaceOS::Api private def update_app_redirect_uri(add : Bool = true) : Nil client = get_client - app = GraphReplicationRetry.run { client.get_application(PLACE_APP_CLIENT_ID, "id,web") } + app = GraphReplicationRetry.run(on_retry: replication_progress) { client.get_application(PLACE_APP_CLIENT_ID, "id,web") } app_redirect_uris = app.web.try &.redirect_uris || [] of String return nil if add && app_redirect_uris.includes?(redirect_url) @@ -137,7 +289,7 @@ module PlaceOS::Api app.web.not_nil!.redirect_uris = app_redirect_uris web = {"web" => app.web} begin - GraphReplicationRetry.run { client.update_application(PLACE_APP_CLIENT_ID, web.to_json) } + GraphReplicationRetry.run(on_retry: replication_progress) { client.update_application(PLACE_APP_CLIENT_ID, web.to_json) } rescue ex : Office365::Exception return nil if already_exists_error?(ex.http_body) raise ex @@ -146,7 +298,7 @@ module PlaceOS::Api private def add_outlook_plugin_auth(app_id : String) : Nil client = get_client - app = GraphReplicationRetry.run { client.get_application(app_id) } + 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") @@ -173,7 +325,7 @@ module PlaceOS::Api ], }, } - GraphReplicationRetry.run { client.update_application(app_id, updated.to_json) } + GraphReplicationRetry.run(on_retry: replication_progress) { client.update_application(app_id, updated.to_json) } updated = { "api": { @@ -187,7 +339,7 @@ module PlaceOS::Api ], }, } - GraphReplicationRetry.run { client.update_application(app_id, updated.to_json) } + GraphReplicationRetry.run(on_retry: replication_progress) { client.update_application(app_id, updated.to_json) } end private def create_outlook_repo : Nil diff --git a/src/placeos-rest-api/utilities/admin-consent-flow.cr b/src/placeos-rest-api/utilities/admin-consent-flow.cr new file mode 100644 index 00000000..b3a33ded --- /dev/null +++ b/src/placeos-rest-api/utilities/admin-consent-flow.cr @@ -0,0 +1,94 @@ +require "json" + +module PlaceOS::Api + # Progress state for an in-flight Azure admin-consent flow. + # + # The consent callback spawns the Microsoft Graph work into a fiber and + # responds immediately with a progress page; state lives in redis (with a + # TTL) so any replica can answer the page's polling. + class AdminConsentFlow + include JSON::Serializable + + # generous - a flow riding out replication retries can run for minutes + TTL_SECONDS = 900 + + STEP_DEFINITIONS = [ + {"visualiser", "Register the Bookings Visualiser application"}, + {"auth_app", "Register the User Authentication application"}, + {"outlook", "Configure the Outlook add-in"}, + {"saving", "Save the authentication configuration"}, + ] + + class Step + include JSON::Serializable + + property key : String + property label : String + property state : String # pending | running | done | failed + + def initialize(@key, @label, @state = "pending") + end + end + + property id : String + property state : String # running | complete | failed + property steps : Array(Step) + property detail : String? + property error : String? + property redirect : String + property authority_id : String + property updated_at : Int64 + + def initialize(@id, @authority_id, @redirect) + @state = "running" + @steps = STEP_DEFINITIONS.map { |(key, label)| Step.new(key, label) } + @updated_at = Time.utc.to_unix + end + + def start_step(key : String) : Nil + @steps.each do |step| + case step.key + when key then step.state = "running" + else step.state = "done" if step.state == "running" + end + end + @detail = nil + save + end + + def detail(message : String) : Nil + @detail = message + save + end + + def complete! : Nil + @steps.each { |step| step.state = "done" } + @state = "complete" + @detail = nil + save + end + + def fail!(message : String) : Nil + @steps.each { |step| step.state = "failed" if step.state == "running" } + @state = "failed" + @error = message + @detail = nil + save + end + + def save : Nil + @updated_at = Time.utc.to_unix + payload = to_json + ::PlaceOS::Driver::RedisStorage.with_redis(&.set(self.class.redis_key(id), payload, ex: TTL_SECONDS)) + end + + def self.load(id : String) : AdminConsentFlow? + payload = ::PlaceOS::Driver::RedisStorage.with_redis(&.get(redis_key(id))) + payload ? from_json(payload) : nil + end + + def self.redis_key(id : String) : String + "placeos:admin_consent:flow:#{id}" + end + end +end diff --git a/src/placeos-rest-api/utilities/graph-replication-retry.cr b/src/placeos-rest-api/utilities/graph-replication-retry.cr index a81ba234..157ba74e 100644 --- a/src/placeos-rest-api/utilities/graph-replication-retry.cr +++ b/src/placeos-rest-api/utilities/graph-replication-retry.cr @@ -17,7 +17,7 @@ module PlaceOS::Api # taking 25s+. BACKOFF = {1, 2, 4, 8, 8, 12} - def self.run(backoff = BACKOFF, & : -> T) : T forall T + def self.run(backoff = BACKOFF, on_retry : Proc(Int32, Nil)? = nil, & : -> T) : T forall T attempt = 0 loop do begin @@ -27,6 +27,7 @@ module PlaceOS::Api raise error unless delay && replication_lag?(error) attempt += 1 Log.warn { "graph resource not replicated yet (attempt #{attempt}), retrying in #{delay}s" } + on_retry.try &.call(attempt) sleep delay.seconds end end