From b9779af5b2df6d93daee518a2be9972f59a12caf Mon Sep 17 00:00:00 2001 From: Stephen Kirby <58410745+stirby@users.noreply.github.com> Date: Mon, 24 Jun 2024 11:27:13 -0500 Subject: [PATCH 1/3] fixed script ref (#13647) (cherry picked from commit 3d6c9799e3fcbc99d7cd6c727369f59866eb19b7) --- scripts/release.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/release.sh b/scripts/release.sh index ec887cd9c3dc1..f7babf2809464 100755 --- a/scripts/release.sh +++ b/scripts/release.sh @@ -384,7 +384,7 @@ You can follow the release progress [here](https://github.com/coder/coder/action fi # Push the branch so it's available for gh to create the PR. - maybedryrun "${dry_run}" git push -u "{remote}" "${pr_branch}" + maybedryrun "${dry_run}" git push -u "${remote}" "${pr_branch}" log "Creating pull request..." maybedryrun "${dry_run}" gh pr create \ From 8ce87004240ea7949b3d7d66499ecf4107615f1d Mon Sep 17 00:00:00 2001 From: Stephen Kirby <58410745+stirby@users.noreply.github.com> Date: Mon, 24 Jun 2024 11:50:24 -0500 Subject: [PATCH 2/3] fixed changelog script release channel flag (#13649) (cherry picked from commit b9d83c75de79205ee7c67b87a86b7e328112421d) --- scripts/release.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/release.sh b/scripts/release.sh index f7babf2809464..347c38642613f 100755 --- a/scripts/release.sh +++ b/scripts/release.sh @@ -217,7 +217,7 @@ release_branch=${tag_version[0]} new_version=${tag_version[1]} new_version="${new_version%$'\n'}" # Remove the trailing newline. -release_notes="$(execrelative ./release/generate_release_notes.sh --old-version "$old_version" --new-version "$new_version" --ref "$ref")" +release_notes="$(execrelative ./release/generate_release_notes.sh --old-version "$old_version" --new-version "$new_version" --ref "$ref" --$channel)" mkdir -p build release_notes_file="build/RELEASE-${new_version}.md" From 534d4ea752bf2e72a5c3b46710ee7e56c33411c3 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 3 Jun 2024 13:16:51 -0500 Subject: [PATCH 3/3] 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 27f26910b6350cd97a25564822857371cb81a9bd) --- coderd/coderdtest/oidctest/idp.go | 19 +++++++- coderd/externalauth/externalauth.go | 2 +- coderd/externalauth_test.go | 12 ++--- coderd/workspacebuilds_test.go | 74 +++++++++++++++++++++++++++++ 4 files changed, 98 insertions(+), 9 deletions(-) diff --git a/coderd/coderdtest/oidctest/idp.go b/coderd/coderdtest/oidctest/idp.go index c0b95619d46b7..844c4df1d2664 100644 --- a/coderd/coderdtest/oidctest/idp.go +++ b/coderd/coderdtest/oidctest/idp.go @@ -1255,7 +1255,9 @@ type ExternalAuthConfigOptions struct { // ValidatePayload is the payload that is used when the user calls the // equivalent of "userinfo" for oauth2. This is not standardized, so is // 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 // 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 case "/user", "/", "": var payload interface{} = "OK" 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) default: diff --git a/coderd/externalauth/externalauth.go b/coderd/externalauth/externalauth.go index 85e53f2e91f33..f5447ebeecd25 100644 --- a/coderd/externalauth/externalauth.go +++ b/coderd/externalauth/externalauth.go @@ -202,7 +202,7 @@ func (c *Config) ValidateToken(ctx context.Context, link *oauth2.Token) (bool, * return false, nil, err } defer res.Body.Close() - if res.StatusCode == http.StatusUnauthorized { + if res.StatusCode == http.StatusUnauthorized || res.StatusCode == http.StatusForbidden { // The token is no longer valid! return false, nil, nil } diff --git a/coderd/externalauth_test.go b/coderd/externalauth_test.go index db40ccf38a554..916a88460d53c 100644 --- a/coderd/externalauth_test.go +++ b/coderd/externalauth_test.go @@ -79,11 +79,11 @@ func TestExternalAuthByID(t *testing.T) { client := coderdtest.New(t, &coderdtest.Options{ ExternalAuthConfigs: []*externalauth.Config{ fake.ExternalAuthConfig(t, providerID, &oidctest.ExternalAuthConfigOptions{ - ValidatePayload: func(_ string) interface{} { + ValidatePayload: func(_ string) (interface{}, int, error) { return github.User{ Login: github.String("kyle"), AvatarURL: github.String("https://avatars.githubusercontent.com/u/12345678?v=4"), - } + }, 0, nil }, }, func(cfg *externalauth.Config) { cfg.Type = codersdk.EnhancedExternalAuthProviderGitHub.String() @@ -108,11 +108,11 @@ func TestExternalAuthByID(t *testing.T) { // routes includes a route for /install that returns a list of installations routes := (&oidctest.ExternalAuthConfigOptions{ - ValidatePayload: func(_ string) interface{} { + ValidatePayload: func(_ string) (interface{}, int, error) { return github.User{ Login: github.String("kyle"), AvatarURL: github.String("https://avatars.githubusercontent.com/u/12345678?v=4"), - } + }, 0, nil }, }).AddRoute("/installs", func(_ string, rw http.ResponseWriter, r *http.Request) { httpapi.Write(r.Context(), rw, http.StatusOK, struct { @@ -556,7 +556,7 @@ func TestExternalAuthCallback(t *testing.T) { // If the validation URL gives a non-OK status code, this // should be treated as an internal server error. 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!")) }) _, err = agentClient.ExternalAuth(ctx, agentsdk.ExternalAuthRequest{ @@ -565,7 +565,7 @@ func TestExternalAuthCallback(t *testing.T) { var apiError *codersdk.Error require.ErrorAs(t, err, &apiError) 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) { diff --git a/coderd/workspacebuilds_test.go b/coderd/workspacebuilds_test.go index f8560ff911925..eb76239b84658 100644 --- a/coderd/workspacebuilds_test.go +++ b/coderd/workspacebuilds_test.go @@ -20,9 +20,11 @@ import ( "cdr.dev/slog/sloggers/slogtest" "github.com/coder/coder/v2/coderd/audit" "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/dbauthz" "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/codersdk" "github.com/coder/coder/v2/provisioner/echo" @@ -711,6 +713,78 @@ func TestWorkspaceBuildStatus(t *testing.T) { 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) { t.Parallel()