mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-06 16:02:23 +00:00
fix: guest app-mode decided once at the on-behalf resolver; clear a stale guest session before offering another app's sign-in
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BayTppRCstWX6qTf3LMco5
This commit is contained in:
co-authored by
Claude Opus 5
parent
03e0504d95
commit
2ecbe31e91
@@ -4611,10 +4611,9 @@ struct EditGuestAccess {
|
||||
|
||||
/// Turn guest sessions on or off for this workspace. Off by default, and off is
|
||||
/// authoritative and immediate: the switch is re-read where a guest session is
|
||||
/// minted, where a guest reads an app and where a guest runs its components
|
||||
/// (`guest_app_admits` / `is_guest_access_enabled`), so an app whose policy already
|
||||
/// says `guest` — pushed by git-sync, say — closes to guests on the next request,
|
||||
/// sessions already issued included.
|
||||
/// minted (`guest_app_admits`) and at the auth door on every guest request, so an app
|
||||
/// whose policy already says `guest` — pushed by git-sync, say — closes to guests on
|
||||
/// the next request, sessions already issued included.
|
||||
async fn edit_guest_access(
|
||||
authed: ApiAuthed,
|
||||
Extension(db): Extension<DB>,
|
||||
|
||||
@@ -5751,10 +5751,10 @@ paths:
|
||||
summary: enable or disable guest sessions for this workspace
|
||||
description: >-
|
||||
Guests are people the identity provider authenticates who are members of no
|
||||
workspace, so they take no seat. Off by default. Re-read wherever a guest is
|
||||
admitted — minting a session, reading an app, running its components — so
|
||||
turning it off takes effect immediately, for sessions already issued and for
|
||||
apps whose policy already says `guest`.
|
||||
workspace, so they take no seat. Off by default. Re-read where a guest session
|
||||
is minted and at the auth door on every guest request, so turning it off takes
|
||||
effect immediately, for sessions already issued and for apps whose policy
|
||||
already says `guest`.
|
||||
operationId: editGuestAccess
|
||||
tags:
|
||||
- workspace
|
||||
@@ -33222,8 +33222,8 @@ components:
|
||||
the app is already deployed under. Neither `anonymous`, which makes
|
||||
the app publicly executable, nor `guest`, which opens it to anyone the
|
||||
identity provider authenticates, is ever assumed. A guest is only
|
||||
admitted where the workspace also has `guest_access_enabled`, which
|
||||
gates both minting a guest session and running an app's components
|
||||
admitted where the workspace also has `guest_access_enabled`, which is
|
||||
checked when the session is minted and on every guest request after
|
||||
on_behalf_of:
|
||||
type: string
|
||||
on_behalf_of_email:
|
||||
|
||||
@@ -3748,6 +3748,19 @@ async fn get_on_behalf_details_from_policy_and_authed(
|
||||
policy: &Policy,
|
||||
opt_authed: &Option<ApiAuthed>,
|
||||
) -> Result<(String, String, String)> {
|
||||
// A guest acts only through an app that is open to guests. Any other mode means
|
||||
// the policy changed after the session was issued. Decided here because this is
|
||||
// the one resolver every on-behalf path — component runs, S3 reads, uploads —
|
||||
// goes through, so no route has to remember it.
|
||||
if opt_authed
|
||||
.as_ref()
|
||||
.is_some_and(|a| windmill_api_auth::scopes::has_guest_sentinel(a.scopes.as_deref()))
|
||||
&& !matches!(policy.execution_mode(), ExecutionMode::Guest)
|
||||
{
|
||||
return Err(Error::PermissionDenied(
|
||||
"this app is not open to guests".to_string(),
|
||||
));
|
||||
}
|
||||
let (username, permissioned_as, email) = match policy.execution_mode() {
|
||||
ExecutionMode::Anonymous => {
|
||||
let username = opt_authed
|
||||
@@ -4081,14 +4094,9 @@ async fn execute_component(
|
||||
|
||||
// A guest session holds no ACL of its own, so the read-permit probe below would
|
||||
// deny every guest. What confines it is the scope the session was minted with,
|
||||
// naming the one app it may run — and the app has to be open to guests at all.
|
||||
// The workspace's guest switch is enforced by `AuthCache` on every guest request.
|
||||
// naming the one app it may run. The app's mode and the workspace's switch are
|
||||
// decided elsewhere: the on-behalf resolver and `AuthCache` respectively.
|
||||
if let Some(authed) = opt_authed.as_ref().filter(|_| is_guest_caller) {
|
||||
if !matches!(policy.execution_mode(), ExecutionMode::Guest) {
|
||||
return Err(Error::PermissionDenied(format!(
|
||||
"app {path} is not open to guests"
|
||||
)));
|
||||
}
|
||||
check_scopes(authed, || format!("apps:run:{}", path))?;
|
||||
}
|
||||
|
||||
|
||||
@@ -778,11 +778,11 @@ pub struct BillableSeats {
|
||||
/// Whether `w_id` admits guest sessions — someone the identity provider authenticated
|
||||
/// who is a member of no workspace, and who therefore takes no seat.
|
||||
///
|
||||
/// Read uncached, at every door a guest passes: where the session is minted
|
||||
/// ([`guest_app_admits`]), where a guest reads an app, and where a guest runs its
|
||||
/// components. An app carries its own `execution_mode` in its definition, so git-sync
|
||||
/// and the CLI can push `guest` past every deploy-time gate; re-reading here is what
|
||||
/// makes turning the switch off take effect on sessions already issued.
|
||||
/// Read uncached where a session is minted ([`guest_app_admits`]) and then once per
|
||||
/// request at the auth door (`AuthCache::get_opt_job_authed`) for every guest. An app
|
||||
/// carries its own `execution_mode` in its definition, so git-sync and the CLI can
|
||||
/// push `guest` past every deploy-time gate; the per-request read is what makes
|
||||
/// turning the switch off take effect on sessions already issued.
|
||||
pub async fn is_guest_access_enabled(db: &crate::DB, w_id: &str) -> Result<bool> {
|
||||
Ok(sqlx::query_scalar!(
|
||||
"SELECT guest_access_enabled FROM workspace_settings WHERE workspace_id = $1",
|
||||
|
||||
@@ -23,7 +23,7 @@
|
||||
* bearer token (no cookie); the raw wrapper document always carries `CSP: sandbox`.
|
||||
*/
|
||||
import { BROWSER } from 'esm-env'
|
||||
import { OpenAPI } from '$lib/gen'
|
||||
import { OpenAPI, UserService } from '$lib/gen'
|
||||
import { page } from '$app/state'
|
||||
import { onDestroy, onMount, setContext, type Snippet } from 'svelte'
|
||||
import { Alert, Skeleton } from '$lib/components/common'
|
||||
@@ -214,13 +214,27 @@
|
||||
* member of this workspace lands here. */
|
||||
let signedInHere = $state(false)
|
||||
let deniedStatus: number | undefined = $state(undefined)
|
||||
/** The sign-in card belongs on a 401, and on a 403 against an app that admits
|
||||
* guests (a session for another app of the same workspace; signing in again
|
||||
* replaces it). */
|
||||
/** The sign-in card belongs on a 401, and on a 403 unless discovery has settled
|
||||
* that the app is not open to guests — a 403 on a guest app is a session for a
|
||||
* different app of the same workspace, which a fresh sign-in replaces. While
|
||||
* discovery is still pending this stays true so the skeleton shows rather than
|
||||
* a flash of "Not found". */
|
||||
let offerSignIn = $derived(
|
||||
status === 'noPermission' || (status === 'notExists' && deniedStatus === 403 && guestEntry === 'guest')
|
||||
status === 'noPermission' ||
|
||||
(status === 'notExists' && deniedStatus === 403 && guestEntry !== 'none')
|
||||
)
|
||||
let signInDidNotHelp = $derived(offerSignIn && signedInHere)
|
||||
|
||||
// The stale guest session must go before the card mounts: it still authenticates
|
||||
// in this workspace, so the popup's success poll would see it and complete the
|
||||
// sign-in before the new session ever lands.
|
||||
let staleGuestCleared = $state(false)
|
||||
$effect(() => {
|
||||
if (deniedStatus === 403 && guestEntry === 'guest' && !staleGuestCleared) {
|
||||
staleGuestCleared = true
|
||||
UserService.logout().catch(() => {})
|
||||
}
|
||||
})
|
||||
let embedToken: string | null = $state(null)
|
||||
let iframeEl: HTMLIFrameElement | undefined = $state(undefined)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user