Skip to content
Snippets Groups Projects
Commit 534d4ea7 authored by Steven Masley's avatar Steven Masley Committed by Stephen Kirby
Browse files

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

(cherry picked from commit 27f26910)
parent 8ce87004
No related branches found
No related tags found
No related merge requests found
...@@ -1255,7 +1255,9 @@ type ExternalAuthConfigOptions struct { ...@@ -1255,7 +1255,9 @@ type ExternalAuthConfigOptions struct {
// ValidatePayload is the payload that is used when the user calls the // ValidatePayload is the payload that is used when the user calls the
// equivalent of "userinfo" for oauth2. This is not standardized, so is // equivalent of "userinfo" for oauth2. This is not standardized, so is
// different for each provider type. // different for each provider type.
ValidatePayload func(email string) interface{} //
// The int,error payload can control the response if set.
ValidatePayload func(email string) (interface{}, int, error)
// routes is more advanced usage. This allows the caller to // routes is more advanced usage. This allows the caller to
// completely customize the response. It captures all routes under the /external-auth-validate/* // 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 ...@@ -1292,7 +1294,20 @@ func (f *FakeIDP) ExternalAuthConfig(t testing.TB, id string, custom *ExternalAu
case "/user", "/", "": case "/user", "/", "":
var payload interface{} = "OK" var payload interface{} = "OK"
if custom.ValidatePayload != nil { if custom.ValidatePayload != nil {
payload = custom.ValidatePayload(email) var err error
var code int
payload, code, err = custom.ValidatePayload(email)
if code == 0 && err == nil {
code = http.StatusOK
}
if code == 0 && err != nil {
code = http.StatusUnauthorized
}
if err != nil {
http.Error(rw, fmt.Sprintf("failed validation via custom method: %s", err.Error()), code)
return
}
rw.WriteHeader(code)
} }
_ = json.NewEncoder(rw).Encode(payload) _ = json.NewEncoder(rw).Encode(payload)
default: default:
......
...@@ -202,7 +202,7 @@ func (c *Config) ValidateToken(ctx context.Context, link *oauth2.Token) (bool, * ...@@ -202,7 +202,7 @@ func (c *Config) ValidateToken(ctx context.Context, link *oauth2.Token) (bool, *
return false, nil, err return false, nil, err
} }
defer res.Body.Close() defer res.Body.Close()
if res.StatusCode == http.StatusUnauthorized { if res.StatusCode == http.StatusUnauthorized || res.StatusCode == http.StatusForbidden {
// The token is no longer valid! // The token is no longer valid!
return false, nil, nil return false, nil, nil
} }
......
...@@ -79,11 +79,11 @@ func TestExternalAuthByID(t *testing.T) { ...@@ -79,11 +79,11 @@ func TestExternalAuthByID(t *testing.T) {
client := coderdtest.New(t, &coderdtest.Options{ client := coderdtest.New(t, &coderdtest.Options{
ExternalAuthConfigs: []*externalauth.Config{ ExternalAuthConfigs: []*externalauth.Config{
fake.ExternalAuthConfig(t, providerID, &oidctest.ExternalAuthConfigOptions{ fake.ExternalAuthConfig(t, providerID, &oidctest.ExternalAuthConfigOptions{
ValidatePayload: func(_ string) interface{} { ValidatePayload: func(_ string) (interface{}, int, error) {
return github.User{ return github.User{
Login: github.String("kyle"), Login: github.String("kyle"),
AvatarURL: github.String("https://avatars.githubusercontent.com/u/12345678?v=4"), AvatarURL: github.String("https://avatars.githubusercontent.com/u/12345678?v=4"),
} }, 0, nil
}, },
}, func(cfg *externalauth.Config) { }, func(cfg *externalauth.Config) {
cfg.Type = codersdk.EnhancedExternalAuthProviderGitHub.String() cfg.Type = codersdk.EnhancedExternalAuthProviderGitHub.String()
...@@ -108,11 +108,11 @@ func TestExternalAuthByID(t *testing.T) { ...@@ -108,11 +108,11 @@ func TestExternalAuthByID(t *testing.T) {
// routes includes a route for /install that returns a list of installations // routes includes a route for /install that returns a list of installations
routes := (&oidctest.ExternalAuthConfigOptions{ routes := (&oidctest.ExternalAuthConfigOptions{
ValidatePayload: func(_ string) interface{} { ValidatePayload: func(_ string) (interface{}, int, error) {
return github.User{ return github.User{
Login: github.String("kyle"), Login: github.String("kyle"),
AvatarURL: github.String("https://avatars.githubusercontent.com/u/12345678?v=4"), AvatarURL: github.String("https://avatars.githubusercontent.com/u/12345678?v=4"),
} }, 0, nil
}, },
}).AddRoute("/installs", func(_ string, rw http.ResponseWriter, r *http.Request) { }).AddRoute("/installs", func(_ string, rw http.ResponseWriter, r *http.Request) {
httpapi.Write(r.Context(), rw, http.StatusOK, struct { httpapi.Write(r.Context(), rw, http.StatusOK, struct {
...@@ -556,7 +556,7 @@ func TestExternalAuthCallback(t *testing.T) { ...@@ -556,7 +556,7 @@ func TestExternalAuthCallback(t *testing.T) {
// If the validation URL gives a non-OK status code, this // If the validation URL gives a non-OK status code, this
// should be treated as an internal server error. // should be treated as an internal server error.
srv.Config.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { srv.Config.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusForbidden) w.WriteHeader(http.StatusBadRequest)
w.Write([]byte("Something went wrong!")) w.Write([]byte("Something went wrong!"))
}) })
_, err = agentClient.ExternalAuth(ctx, agentsdk.ExternalAuthRequest{ _, err = agentClient.ExternalAuth(ctx, agentsdk.ExternalAuthRequest{
...@@ -565,7 +565,7 @@ func TestExternalAuthCallback(t *testing.T) { ...@@ -565,7 +565,7 @@ func TestExternalAuthCallback(t *testing.T) {
var apiError *codersdk.Error var apiError *codersdk.Error
require.ErrorAs(t, err, &apiError) require.ErrorAs(t, err, &apiError)
require.Equal(t, http.StatusInternalServerError, apiError.StatusCode()) require.Equal(t, http.StatusInternalServerError, apiError.StatusCode())
require.Equal(t, "validate external auth token: status 403: body: Something went wrong!", apiError.Detail) require.Equal(t, "validate external auth token: status 400: body: Something went wrong!", apiError.Detail)
}) })
t.Run("ExpiredNoRefresh", func(t *testing.T) { t.Run("ExpiredNoRefresh", func(t *testing.T) {
......
...@@ -20,9 +20,11 @@ import ( ...@@ -20,9 +20,11 @@ import (
"cdr.dev/slog/sloggers/slogtest" "cdr.dev/slog/sloggers/slogtest"
"github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/audit"
"github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/coderdtest"
"github.com/coder/coder/v2/coderd/coderdtest/oidctest"
"github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbauthz"
"github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/database/dbtime"
"github.com/coder/coder/v2/coderd/externalauth"
"github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/coderd/rbac"
"github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/provisioner/echo" "github.com/coder/coder/v2/provisioner/echo"
...@@ -711,6 +713,78 @@ func TestWorkspaceBuildStatus(t *testing.T) { ...@@ -711,6 +713,78 @@ func TestWorkspaceBuildStatus(t *testing.T) {
require.EqualValues(t, codersdk.WorkspaceStatusDeleted, workspace.LatestBuild.Status) require.EqualValues(t, codersdk.WorkspaceStatusDeleted, workspace.LatestBuild.Status)
} }
func TestWorkspaceDeleteSuspendedUser(t *testing.T) {
t.Parallel()
const providerID = "fake-github"
fake := oidctest.NewFakeIDP(t, oidctest.WithServing())
validateCalls := 0
userSuspended := false
owner := coderdtest.New(t, &coderdtest.Options{
IncludeProvisionerDaemon: true,
ExternalAuthConfigs: []*externalauth.Config{
fake.ExternalAuthConfig(t, providerID, &oidctest.ExternalAuthConfigOptions{
ValidatePayload: func(email string) (interface{}, int, error) {
validateCalls++
if userSuspended {
// Simulate the user being suspended from the IDP too.
return "", http.StatusForbidden, fmt.Errorf("user is suspended")
}
return "OK", 0, nil
},
}),
},
})
first := coderdtest.CreateFirstUser(t, owner)
// New user that we will suspend when we try to delete the workspace.
client, user := coderdtest.CreateAnotherUser(t, owner, first.OrganizationID, rbac.RoleTemplateAdmin())
fake.ExternalLogin(t, client)
version := coderdtest.CreateTemplateVersion(t, client, first.OrganizationID, &echo.Responses{
Parse: echo.ParseComplete,
ProvisionApply: echo.ApplyComplete,
ProvisionPlan: []*proto.Response{{
Type: &proto.Response_Plan{
Plan: &proto.PlanComplete{
Error: "",
Resources: nil,
Parameters: nil,
ExternalAuthProviders: []*proto.ExternalAuthProviderResource{
{
Id: providerID,
Optional: false,
},
},
},
},
}},
})
validateCalls = 0 // Reset
coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
template := coderdtest.CreateTemplate(t, client, first.OrganizationID, version.ID)
workspace := coderdtest.CreateWorkspace(t, client, first.OrganizationID, template.ID)
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID)
require.Equal(t, 1, validateCalls) // Ensure the external link is working
// Suspend the user
ctx := testutil.Context(t, testutil.WaitLong)
_, err := owner.UpdateUserStatus(ctx, user.ID.String(), codersdk.UserStatusSuspended)
require.NoError(t, err, "suspend user")
// Now delete the workspace build
userSuspended = true
build, err := owner.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{
Transition: codersdk.WorkspaceTransitionDelete,
})
require.NoError(t, err)
build = coderdtest.AwaitWorkspaceBuildJobCompleted(t, owner, build.ID)
require.Equal(t, 2, validateCalls)
require.Equal(t, codersdk.WorkspaceStatusDeleted, build.Status)
}
func TestWorkspaceBuildDebugMode(t *testing.T) { func TestWorkspaceBuildDebugMode(t *testing.T) {
t.Parallel() t.Parallel()
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment