mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-06 16:02:23 +00:00
fix: a guest may use anonymous apps; await the stale-session logout; trim comments
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
2ecbe31e91
commit
636c027b58
@@ -144,12 +144,10 @@ impl AuthCache {
|
||||
// from a job's WM_TOKEN, even on an AUTH_CACHE hit whose cached authed
|
||||
// predates this field.
|
||||
opt_job_authed.authed.job_id = opt_job_authed.job_id;
|
||||
// The workspace's guest switch is enforced here, at the door, for every
|
||||
// request a guest makes — not in the handlers, where each guest-reachable
|
||||
// route would have to remember to re-check it. Uncached and per request, so
|
||||
// turning guests off takes effect on the next request of every guest
|
||||
// session and every token derived from one. Guests are a small share of
|
||||
// traffic; the read is one primary-key lookup.
|
||||
// The workspace's guest switch is enforced here, once, for every guest request
|
||||
// — not per handler, where each guest-reachable route would have to remember
|
||||
// it. Uncached, so turning guests off takes effect on the next request of every
|
||||
// guest session and every token derived from one.
|
||||
if crate::scopes::has_guest_sentinel(opt_job_authed.authed.scopes.as_deref()) {
|
||||
let Some(w_id) = w_id else { return None };
|
||||
match windmill_common::workspaces::is_guest_access_enabled(&self.db, &w_id).await {
|
||||
|
||||
@@ -33223,7 +33223,7 @@ components:
|
||||
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 is
|
||||
checked when the session is minted and on every guest request after
|
||||
checked when the session is minted and again on every guest request
|
||||
on_behalf_of:
|
||||
type: string
|
||||
on_behalf_of_email:
|
||||
|
||||
@@ -292,9 +292,9 @@ pub enum ExecutionMode {
|
||||
/// Login required, workspace membership not: anyone the instance's identity
|
||||
/// provider authenticates may open the app, and the runnables execute as the
|
||||
/// publisher exactly as in [`ExecutionMode::Publisher`]. Such a viewer holds a
|
||||
/// guest session — no `usr` row, no `password` row, no seat — so it is only
|
||||
/// honored where `workspace_settings.guest_access_enabled` is on, checked
|
||||
/// where the session is minted rather than here.
|
||||
/// guest session — no `usr` row, no `password` row, no seat — honored only where
|
||||
/// `workspace_settings.guest_access_enabled` is on, which `AuthCache` re-reads on
|
||||
/// every guest request.
|
||||
Guest,
|
||||
/// Default for a policy that omits `execution_mode`. It MUST stay a mode
|
||||
/// that requires an authenticated viewer: an omitted field must never be
|
||||
@@ -1841,8 +1841,10 @@ async fn get_app_embed_token(
|
||||
|
||||
let authed_for_token = if policy.anonymous_execution {
|
||||
// Anonymous app: still mint a scoped token if the viewer happens to be
|
||||
// logged in (so the app sees their identity), otherwise stay anonymous.
|
||||
opt_authed
|
||||
// logged in (so the app sees their identity), otherwise stay anonymous. A
|
||||
// guest's session names another app and cannot contain this one's scopes,
|
||||
// so it renders anonymously here rather than being refused.
|
||||
opt_authed.filter(|a| !windmill_api_auth::scopes::has_guest_sentinel(a.scopes.as_deref()))
|
||||
} else {
|
||||
let mode = if policy.guest_execution {
|
||||
ExecutionMode::Guest
|
||||
@@ -3748,14 +3750,16 @@ 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.
|
||||
// A guest acts only through an app open to guests — or to everyone. A members-only
|
||||
// mode means the policy changed after the session was issued. Decided here, in the
|
||||
// one resolver every on-behalf path (runs, S3 reads, uploads) goes through.
|
||||
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)
|
||||
&& !matches!(
|
||||
policy.execution_mode(),
|
||||
ExecutionMode::Guest | ExecutionMode::Anonymous
|
||||
)
|
||||
{
|
||||
return Err(Error::PermissionDenied(
|
||||
"this app is not open to guests".to_string(),
|
||||
@@ -4094,10 +4098,13 @@ 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. The app's mode and the workspace's switch are
|
||||
// decided elsewhere: the on-behalf resolver and `AuthCache` respectively.
|
||||
// naming the one app it may run — except an anonymous app, open to a guest as to
|
||||
// anyone. Mode and workspace switch are decided by the on-behalf resolver and
|
||||
// `AuthCache` respectively.
|
||||
if let Some(authed) = opt_authed.as_ref().filter(|_| is_guest_caller) {
|
||||
check_scopes(authed, || format!("apps:run:{}", path))?;
|
||||
if !matches!(policy.execution_mode(), ExecutionMode::Anonymous) {
|
||||
check_scopes(authed, || format!("apps:run:{}", path))?;
|
||||
}
|
||||
}
|
||||
|
||||
// Execution is publisher and an user is authenticated: check if the user is authorized to
|
||||
|
||||
@@ -225,14 +225,18 @@
|
||||
)
|
||||
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.
|
||||
// The stale guest session must be gone 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. The card waits on
|
||||
// `staleGuestCleared`, and a failed logout fails closed rather than offering a
|
||||
// sign-in that could not complete.
|
||||
let staleGuestCleared = $state(false)
|
||||
let staleGuestLogoutFailed = $state(false)
|
||||
$effect(() => {
|
||||
if (deniedStatus === 403 && guestEntry === 'guest' && !staleGuestCleared) {
|
||||
staleGuestCleared = true
|
||||
UserService.logout().catch(() => {})
|
||||
UserService.logout()
|
||||
.then(() => (staleGuestCleared = true))
|
||||
.catch(() => (staleGuestLogoutFailed = true))
|
||||
}
|
||||
})
|
||||
let embedToken: string | null = $state(null)
|
||||
@@ -573,9 +577,9 @@
|
||||
onContinue={onSdkConsentContinue}
|
||||
onDecline={onSdkConsentDecline}
|
||||
/>
|
||||
{:else if offerSignIn && guestEntry === 'pending'}
|
||||
{:else if offerSignIn && (guestEntry === 'pending' || (deniedStatus === 403 && guestEntry === 'guest' && !staleGuestCleared && !staleGuestLogoutFailed))}
|
||||
<Skeleton layout={[[4], 0.5, [50]]} />
|
||||
{:else if offerSignIn && guestEntry === 'error'}
|
||||
{:else if offerSignIn && (guestEntry === 'error' || staleGuestLogoutFailed)}
|
||||
<div class="px-4 mt-20">
|
||||
<Alert type="error" title="Could not check access">
|
||||
The app could not be reached to find out who may open it. Reload to try again.
|
||||
|
||||
Reference in New Issue
Block a user