chore: account for forbidden as well as unauthorized response codes

Emyrk · Emyrk · commit 36340e6fa62b · 2024-06-03T12:17:14.000-05:00
diff --git 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{}, error)
+	//
+	// 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/*
@@ -1293,10 +1295,19 @@ func (f *FakeIDP) ExternalAuthConfig(t testing.TB, id string, custom *ExternalAu
 			var payload interface{} = "OK"
 			if custom.ValidatePayload != nil {
 				var err error
-				payload, err = custom.ValidatePayload(email)
+				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()), http.StatusBadRequest)
+					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
@@ -218,7 +218,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
@@ -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{}, error) {
+					ValidatePayload: func(_ string) (interface{}, int, error) {
 						return github.User{
 							Login:     github.String("kyle"),
 							AvatarURL: github.String("https://avatars.githubusercontent.com/u/12345678?v=4"),
-						}, nil
+						}, 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{}, error) {
+			ValidatePayload: func(_ string) (interface{}, int, error) {
 				return github.User{
 					Login:     github.String("kyle"),
 					AvatarURL: github.String("https://avatars.githubusercontent.com/u/12345678?v=4"),
-				}, nil
+				}, 0, nil
 			},
 		}).AddRoute("/installs", func(_ string, rw http.ResponseWriter, r *http.Request) {
 			httpapi.Write(r.Context(), rw, http.StatusOK, struct {
diff --git a/coderd/workspacebuilds_test.go b/coderd/workspacebuilds_test.go
@@ -724,13 +724,13 @@ func TestWorkspaceDeleteSuspendedUser(t *testing.T) {
 		IncludeProvisionerDaemon: true,
 		ExternalAuthConfigs: []*externalauth.Config{
 			fake.ExternalAuthConfig(t, providerID, &oidctest.ExternalAuthConfigOptions{
-				ValidatePayload: func(email string) (interface{}, error) {
+				ValidatePayload: func(email string) (interface{}, int, error) {
 					validateCalls++
 					if userSuspended {
 						// Simulate the user being suspended from the IDP too.
-						return "", fmt.Errorf("user is suspended")
+						return "", http.StatusForbidden, fmt.Errorf("user is suspended")
 					}
-					return "OK", nil
+					return "OK", 0, nil
 				},
 			}),
 		},
@@ -782,7 +782,7 @@ func TestWorkspaceDeleteSuspendedUser(t *testing.T) {
 		Transition: codersdk.WorkspaceTransitionDelete,
 	})
 	require.NoError(t, err)
-	build = coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, build.ID)
+	build = coderdtest.AwaitWorkspaceBuildJobCompleted(t, owner, build.ID)
 	require.Equal(t, 2, validateCalls)
 	require.Equal(t, codersdk.WorkspaceStatusDeleted, build.Status)
 }

Original file line number	Diff line number	Diff line change
`@@ -218,7 +218,7 @@ func (c Config) ValidateToken(ctx context.Context, link oauth2.Token) (bool, *`
`218`	`218`	`return false, nil, err`
`219`	`219`	`}`
`220`	`220`	`defer res.Body.Close()`
`221`		`- if res.StatusCode == http.StatusUnauthorized {`
	`221`	`+ if res.StatusCode == http.StatusUnauthorized \|\| res.StatusCode == http.StatusForbidden {`
`222`	`222`	`// The token is no longer valid!`
`223`	`223`	`return false, nil, nil`
`224`	`224`	`}`