Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit9028717

Browse files
authored
fix: disable auto-create if external auth requirements aren't met (#12538)
1 parentef26ad9 commit9028717

File tree

6 files changed

+124
-26
lines changed

6 files changed

+124
-26
lines changed

‎site/jest.setup.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ jest.mock("contexts/useProxyLatency", () => ({
4040
global.scrollTo=jest.fn();
4141

4242
window.HTMLElement.prototype.scrollIntoView=jest.fn();
43+
window.open=jest.fn();
4344

4445
// Polyfill the getRandomValues that is used on utils/random.ts
4546
Object.defineProperty(global.self,"crypto",{

‎site/src/api/errors.ts‎

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,26 @@ export type ApiError = AxiosError<ApiErrorResponse> & {
2424
};
2525

2626
exportconstisApiError=(err:unknown):err isApiError=>{
27-
returnaxios.isAxiosError(err)&&err.response!==undefined;
27+
return(
28+
axios.isAxiosError(err)&&
29+
err.response!==undefined&&
30+
isApiErrorResponse(err.response.data)
31+
);
32+
};
33+
34+
exportconstisApiErrorResponse=(err:unknown):err isApiErrorResponse=>{
35+
return(
36+
typeoferr==="object"&&
37+
err!==null&&
38+
"message"inerr&&
39+
typeoferr.message==="string"&&
40+
(!("detail"inerr)||
41+
err.detail===undefined||
42+
typeoferr.detail==="string")&&
43+
(!("validations"inerr)||
44+
err.validations===undefined||
45+
Array.isArray(err.validations))
46+
);
2847
};
2948

3049
exportconsthasApiFieldErrors=(error:ApiError):boolean=>
@@ -67,6 +86,9 @@ export const getErrorMessage = (
6786
if(isApiError(error)&&error.response.data.message){
6887
returnerror.response.data.message;
6988
}
89+
if(isApiErrorResponse(error)){
90+
returnerror.message;
91+
}
7092
// if error is a non-empty string
7193
if(error&&typeoferror==="string"){
7294
returnerror;
@@ -88,9 +110,15 @@ export const getValidationErrorMessage = (error: unknown): string => {
88110
returnvalidationErrors.map((error)=>error.detail).join("\n");
89111
};
90112

91-
exportconstgetErrorDetail=(error:unknown):string|undefined|null=>
92-
isApiError(error)
93-
?error.response.data.detail
94-
:errorinstanceofError
95-
?`Please check the developer console for more details.`
96-
:null;
113+
exportconstgetErrorDetail=(error:unknown):string|undefined|null=>{
114+
if(errorinstanceofError){
115+
return"Please check the developer console for more details.";
116+
}
117+
if(isApiError(error)){
118+
returnerror.response.data.detail;
119+
}
120+
if(isApiErrorResponse(error)){
121+
returnerror.detail;
122+
}
123+
returnnull;
124+
};

‎site/src/components/Alert/ErrorAlert.stories.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,6 @@ export const WithActionAndDismiss: Story = {
5555

5656
exportconstWithNonApiError:Story={
5757
args:{
58-
error:newError("Non API error here"),
58+
error:newError("Everything has gone horribly, devastatingly wrong."),
5959
},
6060
};

‎site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx‎

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -251,15 +251,48 @@ describe("CreateWorkspacePage", () => {
251251
MockOrganization.id,
252252
"me",
253253
expect.objectContaining({
254-
template_id:MockTemplate.id,
254+
template_version_id:MockTemplate.active_version_id,
255255
rich_parameter_values:[
256-
expect.objectContaining({name:param,value:paramValue}),
256+
expect.objectContaining({
257+
name:param,
258+
source:"url",
259+
value:paramValue,
260+
}),
257261
],
258262
}),
259263
);
260264
});
261265
});
262266

267+
it("disables mode=auto if a required external auth provider is not connected",async()=>{
268+
constparam="first_parameter";
269+
constparamValue="It works!";
270+
constcreateWorkspaceSpy=jest.spyOn(API,"createWorkspace");
271+
272+
constexternalAuthSpy=jest
273+
.spyOn(API,"getTemplateVersionExternalAuth")
274+
.mockResolvedValue([MockTemplateVersionExternalAuthGithub]);
275+
276+
renderWithAuth(<CreateWorkspacePage/>,{
277+
route:
278+
"/templates/"+
279+
MockTemplate.name+
280+
`/workspace?param.${param}=${paramValue}&mode=auto`,
281+
path:"/templates/:template/workspace",
282+
});
283+
awaitwaitForLoaderToBeRemoved();
284+
285+
constwarning=
286+
"This template requires an external authentication provider that is not connected.";
287+
expect(awaitscreen.findByText(warning)).toBeInTheDocument();
288+
expect(createWorkspaceSpy).not.toBeCalled();
289+
290+
// We don't need to do this on any other tests out of hundreds of very, very,
291+
// very similar tests, and yet here, I find it to be absolutely necessary for
292+
// some reason that I certainly do not understand. - Kayla
293+
externalAuthSpy.mockReset();
294+
});
295+
263296
it("auto create a workspace if uses mode=auto and version=version-id",async()=>{
264297
constparam="first_parameter";
265298
constparamValue="It works!";

‎site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx‎

Lines changed: 46 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { Helmet } from "react-helmet-async";
33
import{useMutation,useQuery,useQueryClient}from"react-query";
44
import{useNavigate,useParams,useSearchParams}from"react-router-dom";
55
import{getUserParameters}from"api/api";
6+
importtype{ApiErrorResponse}from"api/errors";
67
import{checkAuthorization}from"api/queries/authCheck";
78
import{
89
richParameters,
@@ -37,11 +38,14 @@ const CreateWorkspacePage: FC = () => {
3738
const{user:me, organizationId}=useAuthenticated();
3839
constnavigate=useNavigate();
3940
const[searchParams,setSearchParams]=useSearchParams();
40-
constmode=getWorkspaceMode(searchParams);
41-
constcustomVersionId=searchParams.get("version")??undefined;
4241
const{ experiments}=useDashboard();
4342

43+
constcustomVersionId=searchParams.get("version")??undefined;
4444
constdefaultName=searchParams.get("name");
45+
constdisabledParams=searchParams.get("disable_params")?.split(",");
46+
const[mode,setMode]=useState(()=>getWorkspaceMode(searchParams));
47+
const[autoCreateError,setAutoCreateError]=
48+
useState<ApiErrorResponse|null>(null);
4549

4650
constqueryClient=useQueryClient();
4751
constautoCreateWorkspaceMutation=useMutation(
@@ -126,37 +130,69 @@ const CreateWorkspacePage: FC = () => {
126130
}
127131
});
128132

129-
constautoStartReady=
130-
mode==="auto"&&(!autofillEnabled||userParametersQuery.isSuccess);
133+
consthasAllRequiredExternalAuth=Boolean(
134+
!isLoadingExternalAuth&&
135+
externalAuth?.every((auth)=>auth.optional||auth.authenticated),
136+
);
137+
138+
letautoCreateReady=
139+
mode==="auto"&&
140+
(!autofillEnabled||userParametersQuery.isSuccess)&&
141+
hasAllRequiredExternalAuth;
142+
143+
// `mode=auto` was set, but a prerequisite has failed, and so auto-mode should be abandoned.
144+
if(
145+
mode==="auto"&&
146+
!isLoadingExternalAuth&&
147+
!hasAllRequiredExternalAuth
148+
){
149+
// Prevent suddenly resuming auto-mode if the user connects to all of the required
150+
// external auth providers.
151+
setMode("form");
152+
// Ensure this is always false, so that we don't ever let `automateWorkspaceCreation`
153+
// fire when we're trying to disable it.
154+
autoCreateReady=false;
155+
// Show an error message to explain _why_ the workspace was not created automatically.
156+
constsubject=
157+
externalAuth?.length===1
158+
?"an external authentication provider that is"
159+
:"external authentication providers that are";
160+
setAutoCreateError({
161+
message:`This template requires${subject} not connected.`,
162+
detail:
163+
"Auto-creation has been disabled. Please connect all required external authentication providers before continuing.",
164+
});
165+
}
166+
131167
useEffect(()=>{
132-
if(autoStartReady){
168+
if(autoCreateReady){
133169
voidautomateWorkspaceCreation();
134170
}
135-
},[automateWorkspaceCreation,autoStartReady]);
171+
},[automateWorkspaceCreation,autoCreateReady]);
136172

137173
return(
138174
<>
139175
<Helmet>
140176
<title>{pageTitle(title)}</title>
141177
</Helmet>
142178
{loadFormDataError&&<ErrorAlerterror={loadFormDataError}/>}
143-
{isLoadingFormData||
144-
isLoadingExternalAuth||
145-
autoCreateWorkspaceMutation.isLoading ?(
179+
{isLoadingFormData||isLoadingExternalAuth||autoCreateReady ?(
146180
<Loader/>
147181
) :(
148182
<CreateWorkspacePageView
149183
mode={mode}
150184
defaultName={defaultName}
185+
disabledParams={disabledParams}
151186
defaultOwner={me}
152187
autofillParameters={autofillParameters}
153-
error={createWorkspaceMutation.error}
188+
error={createWorkspaceMutation.error||autoCreateError}
154189
resetMutation={createWorkspaceMutation.reset}
155190
template={templateQuery.data!}
156191
versionId={realizedVersionId}
157192
externalAuth={externalAuth??[]}
158193
externalAuthPollingState={externalAuthPollingState}
159194
startPollingExternalAuth={startPollingExternalAuth}
195+
hasAllRequiredExternalAuth={hasAllRequiredExternalAuth}
160196
permissions={permissionsQuery.dataasCreateWSPermissions}
161197
parameters={realizedParametersasTemplateVersionParameter[]}
162198
creatingWorkspace={createWorkspaceMutation.isLoading}

‎site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import FormHelperText from "@mui/material/FormHelperText";
44
importTextFieldfrom"@mui/material/TextField";
55
import{typeFormikContextType,useFormik}from"formik";
66
import{typeFC,useEffect,useState,useMemo,useCallback}from"react";
7-
import{useSearchParams}from"react-router-dom";
87
import*asYupfrom"yup";
98
importtype*asTypesGenfrom"api/typesGenerated";
109
import{Alert}from"components/Alert/Alert";
@@ -51,15 +50,17 @@ export const Language = {
5150

5251
exportinterfaceCreateWorkspacePageViewProps{
5352
mode:CreateWorkspaceMode;
53+
defaultName?:string|null;
54+
disabledParams?:string[];
5455
error:unknown;
5556
resetMutation:()=>void;
56-
defaultName?:string|null;
5757
defaultOwner:TypesGen.User;
5858
template:TypesGen.Template;
5959
versionId?:string;
6060
externalAuth:TypesGen.TemplateVersionExternalAuth[];
6161
externalAuthPollingState:ExternalAuthPollingState;
6262
startPollingExternalAuth:()=>void;
63+
hasAllRequiredExternalAuth:boolean;
6364
parameters:TypesGen.TemplateVersionParameter[];
6465
autofillParameters:AutofillBuildParameter[];
6566
permissions:CreateWSPermissions;
@@ -73,9 +74,10 @@ export interface CreateWorkspacePageViewProps {
7374

7475
exportconstCreateWorkspacePageView:FC<CreateWorkspacePageViewProps>=({
7576
mode,
77+
defaultName,
78+
disabledParams,
7679
error,
7780
resetMutation,
78-
defaultName,
7981
defaultOwner,
8082
template,
8183
versionId,
@@ -90,8 +92,6 @@ export const CreateWorkspacePageView: FC<CreateWorkspacePageViewProps> = ({
9092
onCancel,
9193
})=>{
9294
const[owner,setOwner]=useState(defaultOwner);
93-
const[searchParams]=useSearchParams();
94-
constdisabledParamsList=searchParams?.get("disable_params")?.split(",");
9595
constrequiresExternalAuth=externalAuth.some((auth)=>!auth.authenticated);
9696
const[suggestedName,setSuggestedName]=useState(()=>
9797
generateWorkspaceName(),
@@ -285,7 +285,7 @@ export const CreateWorkspacePageView: FC<CreateWorkspacePageViewProps> = ({
285285
constparameterField=`rich_parameter_values.${index}`;
286286
constparameterInputName=`${parameterField}.value`;
287287
constisDisabled=
288-
disabledParamsList?.includes(
288+
disabledParams?.includes(
289289
parameter.name.toLowerCase().replace(//g,"_"),
290290
)||creatingWorkspace;
291291

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp