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

Commit27f2691

Browse files
authored
chore: external auth validate response "Forbidden" should return invalid, not an error (#13446)
* chore: add unit test to delete workspace from suspended user* chore: account for forbidden as well as unauthorized response codes
1 parent0b019ca commit27f2691

File tree

4 files changed

+98
-9
lines changed

4 files changed

+98
-9
lines changed

‎coderd/coderdtest/oidctest/idp.go‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1255,7 +1255,9 @@ type ExternalAuthConfigOptions struct {
12551255
// ValidatePayload is the payload that is used when the user calls the
12561256
// equivalent of "userinfo" for oauth2. This is not standardized, so is
12571257
// different for each provider type.
1258-
ValidatePayloadfunc(emailstring)interface{}
1258+
//
1259+
// The int,error payload can control the response if set.
1260+
ValidatePayloadfunc(emailstring) (interface{},int,error)
12591261

12601262
// routes is more advanced usage. This allows the caller to
12611263
// completely customize the response. It captures all routes under the /external-auth-validate/*
@@ -1292,7 +1294,20 @@ func (f *FakeIDP) ExternalAuthConfig(t testing.TB, id string, custom *ExternalAu
12921294
case"/user","/","":
12931295
varpayloadinterface{}="OK"
12941296
ifcustom.ValidatePayload!=nil {
1295-
payload=custom.ValidatePayload(email)
1297+
varerrerror
1298+
varcodeint
1299+
payload,code,err=custom.ValidatePayload(email)
1300+
ifcode==0&&err==nil {
1301+
code=http.StatusOK
1302+
}
1303+
ifcode==0&&err!=nil {
1304+
code=http.StatusUnauthorized
1305+
}
1306+
iferr!=nil {
1307+
http.Error(rw,fmt.Sprintf("failed validation via custom method: %s",err.Error()),code)
1308+
return
1309+
}
1310+
rw.WriteHeader(code)
12961311
}
12971312
_=json.NewEncoder(rw).Encode(payload)
12981313
default:

‎coderd/externalauth/externalauth.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ func (c *Config) ValidateToken(ctx context.Context, link *oauth2.Token) (bool, *
218218
returnfalse,nil,err
219219
}
220220
deferres.Body.Close()
221-
ifres.StatusCode==http.StatusUnauthorized {
221+
ifres.StatusCode==http.StatusUnauthorized||res.StatusCode==http.StatusForbidden{
222222
// The token is no longer valid!
223223
returnfalse,nil,nil
224224
}

‎coderd/externalauth_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,11 +79,11 @@ func TestExternalAuthByID(t *testing.T) {
7979
client:=coderdtest.New(t,&coderdtest.Options{
8080
ExternalAuthConfigs: []*externalauth.Config{
8181
fake.ExternalAuthConfig(t,providerID,&oidctest.ExternalAuthConfigOptions{
82-
ValidatePayload:func(_string)interface{} {
82+
ValidatePayload:func(_string)(interface{},int,error) {
8383
return github.User{
8484
Login:github.String("kyle"),
8585
AvatarURL:github.String("https://avatars.githubusercontent.com/u/12345678?v=4"),
86-
}
86+
},0,nil
8787
},
8888
},func(cfg*externalauth.Config) {
8989
cfg.Type=codersdk.EnhancedExternalAuthProviderGitHub.String()
@@ -108,11 +108,11 @@ func TestExternalAuthByID(t *testing.T) {
108108

109109
// routes includes a route for /install that returns a list of installations
110110
routes:= (&oidctest.ExternalAuthConfigOptions{
111-
ValidatePayload:func(_string)interface{} {
111+
ValidatePayload:func(_string)(interface{},int,error) {
112112
return github.User{
113113
Login:github.String("kyle"),
114114
AvatarURL:github.String("https://avatars.githubusercontent.com/u/12345678?v=4"),
115-
}
115+
},0,nil
116116
},
117117
}).AddRoute("/installs",func(_string,rw http.ResponseWriter,r*http.Request) {
118118
httpapi.Write(r.Context(),rw,http.StatusOK,struct {
@@ -556,7 +556,7 @@ func TestExternalAuthCallback(t *testing.T) {
556556
// If the validation URL gives a non-OK status code, this
557557
// should be treated as an internal server error.
558558
srv.Config.Handler=http.HandlerFunc(func(w http.ResponseWriter,r*http.Request) {
559-
w.WriteHeader(http.StatusForbidden)
559+
w.WriteHeader(http.StatusBadRequest)
560560
w.Write([]byte("Something went wrong!"))
561561
})
562562
_,err=agentClient.ExternalAuth(ctx, agentsdk.ExternalAuthRequest{
@@ -565,7 +565,7 @@ func TestExternalAuthCallback(t *testing.T) {
565565
varapiError*codersdk.Error
566566
require.ErrorAs(t,err,&apiError)
567567
require.Equal(t,http.StatusInternalServerError,apiError.StatusCode())
568-
require.Equal(t,"validate external auth token: status403: body: Something went wrong!",apiError.Detail)
568+
require.Equal(t,"validate external auth token: status400: body: Something went wrong!",apiError.Detail)
569569
})
570570

571571
t.Run("ExpiredNoRefresh",func(t*testing.T) {

‎coderd/workspacebuilds_test.go‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,11 @@ import (
2020
"cdr.dev/slog/sloggers/slogtest"
2121
"github.com/coder/coder/v2/coderd/audit"
2222
"github.com/coder/coder/v2/coderd/coderdtest"
23+
"github.com/coder/coder/v2/coderd/coderdtest/oidctest"
2324
"github.com/coder/coder/v2/coderd/database"
2425
"github.com/coder/coder/v2/coderd/database/dbauthz"
2526
"github.com/coder/coder/v2/coderd/database/dbtime"
27+
"github.com/coder/coder/v2/coderd/externalauth"
2628
"github.com/coder/coder/v2/coderd/rbac"
2729
"github.com/coder/coder/v2/codersdk"
2830
"github.com/coder/coder/v2/provisioner/echo"
@@ -711,6 +713,78 @@ func TestWorkspaceBuildStatus(t *testing.T) {
711713
require.EqualValues(t,codersdk.WorkspaceStatusDeleted,workspace.LatestBuild.Status)
712714
}
713715

716+
funcTestWorkspaceDeleteSuspendedUser(t*testing.T) {
717+
t.Parallel()
718+
constproviderID="fake-github"
719+
fake:=oidctest.NewFakeIDP(t,oidctest.WithServing())
720+
721+
validateCalls:=0
722+
userSuspended:=false
723+
owner:=coderdtest.New(t,&coderdtest.Options{
724+
IncludeProvisionerDaemon:true,
725+
ExternalAuthConfigs: []*externalauth.Config{
726+
fake.ExternalAuthConfig(t,providerID,&oidctest.ExternalAuthConfigOptions{
727+
ValidatePayload:func(emailstring) (interface{},int,error) {
728+
validateCalls++
729+
ifuserSuspended {
730+
// Simulate the user being suspended from the IDP too.
731+
return"",http.StatusForbidden,fmt.Errorf("user is suspended")
732+
}
733+
return"OK",0,nil
734+
},
735+
}),
736+
},
737+
})
738+
739+
first:=coderdtest.CreateFirstUser(t,owner)
740+
741+
// New user that we will suspend when we try to delete the workspace.
742+
client,user:=coderdtest.CreateAnotherUser(t,owner,first.OrganizationID,rbac.RoleTemplateAdmin())
743+
fake.ExternalLogin(t,client)
744+
745+
version:=coderdtest.CreateTemplateVersion(t,client,first.OrganizationID,&echo.Responses{
746+
Parse:echo.ParseComplete,
747+
ProvisionApply:echo.ApplyComplete,
748+
ProvisionPlan: []*proto.Response{{
749+
Type:&proto.Response_Plan{
750+
Plan:&proto.PlanComplete{
751+
Error:"",
752+
Resources:nil,
753+
Parameters:nil,
754+
ExternalAuthProviders: []*proto.ExternalAuthProviderResource{
755+
{
756+
Id:providerID,
757+
Optional:false,
758+
},
759+
},
760+
},
761+
},
762+
}},
763+
})
764+
765+
validateCalls=0// Reset
766+
coderdtest.AwaitTemplateVersionJobCompleted(t,client,version.ID)
767+
template:=coderdtest.CreateTemplate(t,client,first.OrganizationID,version.ID)
768+
workspace:=coderdtest.CreateWorkspace(t,client,first.OrganizationID,template.ID)
769+
coderdtest.AwaitWorkspaceBuildJobCompleted(t,client,workspace.LatestBuild.ID)
770+
require.Equal(t,1,validateCalls)// Ensure the external link is working
771+
772+
// Suspend the user
773+
ctx:=testutil.Context(t,testutil.WaitLong)
774+
_,err:=owner.UpdateUserStatus(ctx,user.ID.String(),codersdk.UserStatusSuspended)
775+
require.NoError(t,err,"suspend user")
776+
777+
// Now delete the workspace build
778+
userSuspended=true
779+
build,err:=owner.CreateWorkspaceBuild(ctx,workspace.ID, codersdk.CreateWorkspaceBuildRequest{
780+
Transition:codersdk.WorkspaceTransitionDelete,
781+
})
782+
require.NoError(t,err)
783+
build=coderdtest.AwaitWorkspaceBuildJobCompleted(t,owner,build.ID)
784+
require.Equal(t,2,validateCalls)
785+
require.Equal(t,codersdk.WorkspaceStatusDeleted,build.Status)
786+
}
787+
714788
funcTestWorkspaceBuildDebugMode(t*testing.T) {
715789
t.Parallel()
716790

0 commit comments

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