mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-08 08:04:25 +00:00
fix(frontend): block creation when the username policy is unknown
There is no safe default. `create_workspace` refuses a username on an instance that automates them and requires one on an instance that does not (`workspaces.rs:5820`), so a client that cannot read the setting has two request shapes available and the server rejects both. Last round's "ask for one" was as wrong as the "automated" guess it replaced. So the loader reports the failure instead of inventing an answer, and the form says so: Create stays disabled, with a line explaining why and a link to try again. Verified in the browser both ways — unreadable policy disables Create and shows the message, a healthy load prefills the name and enables it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012fRjnaHLwjpHN84gNNxah9
This commit is contained in:
co-authored by
Claude Opus 5
parent
a978b1dbed
commit
4303538686
@@ -41,19 +41,21 @@
|
||||
let automateUsername = $state(true)
|
||||
let suggestedUsername = $state<string | undefined>(undefined)
|
||||
/**
|
||||
* Whether the policy is settled, which is what this form may not submit without:
|
||||
* `automateUsername` starts at the common case, and posting that guess to an instance
|
||||
* that derives no usernames sends none where one is required. `loadUsernamePolicy`
|
||||
* always answers — "ask for one" when it cannot read the setting — so this turns true
|
||||
* on a known answer rather than on the attempt finishing.
|
||||
* Whether the username policy is known, which is what this form may not submit without.
|
||||
* `create_workspace` refuses a username on an instance that automates them and requires
|
||||
* one on an instance that does not, so a client that has not read the setting cannot
|
||||
* pick a request shape — there is no safe default to fall back to, only two shapes the
|
||||
* server rejects. Unknown therefore blocks Create and says why, with a retry.
|
||||
*/
|
||||
let policyLoaded = $state(false)
|
||||
let policyFailed = $state(false)
|
||||
/** Someone typed while the prefill was in flight; their name wins over the suggestion. */
|
||||
let nameEdited = false
|
||||
|
||||
async function load() {
|
||||
// Settled apart: the policy decides whether this form may submit at all, the suggested
|
||||
// name is cosmetic, and neither failure should decide the other.
|
||||
policyFailed = false
|
||||
const [me, policy] = await Promise.allSettled([
|
||||
UserService.globalWhoami(),
|
||||
loadUsernamePolicy()
|
||||
@@ -64,12 +66,14 @@
|
||||
? defaultWorkspaceName(me.value.name, me.value.email)
|
||||
: 'My workspace'
|
||||
}
|
||||
// `loadUsernamePolicy` answers "ask for one" rather than rejecting when the setting
|
||||
// cannot be read, so the fallback here is the same answer by another route.
|
||||
const answer = policy.status === 'fulfilled' ? policy.value : { automate: false }
|
||||
automateUsername = answer.automate
|
||||
suggestedUsername = answer.suggested
|
||||
if (!answer.automate && !answer.suggested) advanced = true
|
||||
if (policy.status === 'rejected') {
|
||||
console.error('Could not read the username policy:', policy.reason)
|
||||
policyFailed = true
|
||||
return
|
||||
}
|
||||
automateUsername = policy.value.automate
|
||||
suggestedUsername = policy.value.suggested
|
||||
if (!policy.value.automate && !policy.value.suggested) advanced = true
|
||||
policyLoaded = true
|
||||
}
|
||||
void load()
|
||||
@@ -175,6 +179,12 @@
|
||||
{#if problem && name.trim()}
|
||||
<span class="text-2xs font-normal text-red-500">{problem}</span>
|
||||
{/if}
|
||||
{#if policyFailed}
|
||||
<span class="mt-1 text-2xs font-normal text-red-500">
|
||||
This instance's settings could not be read, so a workspace cannot be created yet.
|
||||
<button class="text-accent hover:underline" onclick={() => void load()}>Try again</button>
|
||||
</span>
|
||||
{/if}
|
||||
|
||||
<div class="mt-6 flex items-center justify-between gap-4">
|
||||
<div class="flex items-center gap-3">
|
||||
|
||||
@@ -71,11 +71,12 @@ describe('usernameFromName', () => {
|
||||
})
|
||||
|
||||
describe('loadUsernamePolicy', () => {
|
||||
// Fail-closed matters because a caller told "automated" hides its username field and
|
||||
// posts none: an instance that derives none refuses that, with nowhere to supply one.
|
||||
it('asks for a username when the setting cannot be read', async () => {
|
||||
// Neither default is safe — `create_workspace` refuses a username on an automating
|
||||
// instance and requires one otherwise — so an unreadable setting has to reach the caller
|
||||
// as a failure rather than as a guess it cannot tell apart from an answer.
|
||||
it('rejects rather than guessing when the setting cannot be read', async () => {
|
||||
getGlobal.mockRejectedValueOnce(new Error('502'))
|
||||
expect(await loadUsernamePolicy()).toEqual({ automate: false })
|
||||
await expect(loadUsernamePolicy()).rejects.toThrow('502')
|
||||
})
|
||||
|
||||
it('automates when the setting says so, and when it is unset', async () => {
|
||||
|
||||
@@ -68,20 +68,15 @@ export function usernameFromName(name: string): string | undefined {
|
||||
* requires one when it does not, so the field only exists in the second case.
|
||||
*/
|
||||
export async function loadUsernamePolicy(): Promise<UsernamePolicy> {
|
||||
let automate: boolean
|
||||
try {
|
||||
automate =
|
||||
((await SettingService.getGlobal({
|
||||
key: 'automate_username_creation'
|
||||
})) as boolean | null) ?? true
|
||||
} catch (error) {
|
||||
// Unreadable is not "automated". A caller told yes hides its username field and posts
|
||||
// none, which an instance that derives none then refuses — with nowhere on screen to
|
||||
// supply what it wanted. Answering no asks for one, which is right either way: an
|
||||
// instance that does automate ignores a username it was sent.
|
||||
console.error('Could not read the username policy; asking for one instead:', error)
|
||||
return { automate: false }
|
||||
}
|
||||
// Rejects rather than defaulting when the setting cannot be read, because neither
|
||||
// default is safe: `create_workspace` refuses a username on an instance that automates
|
||||
// them and requires one on an instance that does not (`workspaces.rs:5820`). A caller
|
||||
// that cannot read this cannot pick a request shape, and must say so instead of posting
|
||||
// one of the two the server rejects.
|
||||
const automate =
|
||||
((await SettingService.getGlobal({
|
||||
key: 'automate_username_creation'
|
||||
})) as boolean | null) ?? true
|
||||
if (automate) return { automate: true }
|
||||
try {
|
||||
const me = await UserService.globalWhoami()
|
||||
|
||||
Reference in New Issue
Block a user