Return HTTP 400 status for inactive OIDC auth methods (#5048)

This commit modifies the OIDC authentication start method to return a 400 (Bad Request) error
if a client attempts to start the authentication flow against an OIDC auth method that is set to
inactive. This is something we noticed in our error logs in HCP Boundary that is impacting our
SLOs.

Previously this returned its own error code which is mapped to a 500 response. Tests have been updated
to catch this new scenario.

Signed-off-by: David Bond <davidsbond93@gmail.com>

internal/daemon/controller/handlers/authmethods/oidc.go

@@ -178,7 +178,14 @@ func (s Service) authenticateOidcStart(ctx context.Context, req *pbs.Authenticat
 	}
 
 	authUrl, tokenId, err := oidc.StartAuth(ctx, s.oidcRepoFn, req.GetAuthMethodId(), opts...)
-	if err != nil {
+	switch {
+	case errors.Match(errors.T(errors.AuthMethodInactive), err):
+		return nil, handlers.ApiErrorWithCodeAndMessage(codes.FailedPrecondition, "Cannot start authentication against an inactive OIDC auth method")
+	case errors.Match(errors.T(errors.RecordNotFound), err):
+		return nil, handlers.ApiErrorWithCodeAndMessage(codes.NotFound, "Auth method %s was not found", req.GetAuthMethodId())
+	case errors.Match(errors.T(errors.InvalidParameter), err):
+		return nil, handlers.ApiErrorWithCodeAndMessage(codes.InvalidArgument, err.Error())
+	case err != nil:
 		event.WriteError(ctx, op, err, event.WithInfoMsg("error starting the oidc authentication flow"))
 		return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Error generating parameters for starting the OIDC flow. See the controller's log for more information.")
 	}

internal/daemon/controller/handlers/authmethods/oidc_test.go

@@ -1624,9 +1624,10 @@ func TestAuthenticate_OIDC_Start(t *testing.T) {
 	s := getSetup(t)
 
 	cases := []struct {
-		name    string
-		request *pbs.AuthenticateRequest
-		wantErr error
+		name       string
+		request    *pbs.AuthenticateRequest
+		beforeTest func(t *testing.T)
+		wantErr    error
 	}{
 		{
 			name: "no command",
@@ -1657,6 +1658,57 @@ func TestAuthenticate_OIDC_Start(t *testing.T) {
 				AuthMethodId: s.authMethod.GetPublicId(),
 			},
 		},
+		{
+			name: "auth method does not exist",
+			request: &pbs.AuthenticateRequest{
+				Command:      "start",
+				AuthMethodId: "amoidc_1234",
+				Attrs: &pbs.AuthenticateRequest_OidcStartAttributes{
+					OidcStartAttributes: &pbs.OidcStartAttributes{
+						RoundtripPayload: func() *structpb.Struct {
+							ret, err := structpb.NewStruct(map[string]any{
+								"foo": "bar",
+								"baz": true,
+							})
+							require.NoError(t, err)
+							return ret
+						}(),
+					},
+				},
+			},
+			wantErr: handlers.ApiErrorWithCode(codes.NotFound),
+		},
+		{
+			name: "auth method is inactive",
+			beforeTest: func(t *testing.T) {
+				r, err := s.oidcRepoFn()
+				require.NoError(t, err)
+				_, err = r.MakeInactive(s.ctx, s.authMethod.GetPublicId(), 2)
+				require.NoError(t, err)
+
+				t.Cleanup(func() {
+					_, err = r.MakePublic(s.ctx, s.authMethod.GetPublicId(), 3)
+					require.NoError(t, err)
+				})
+			},
+			request: &pbs.AuthenticateRequest{
+				Command:      "start",
+				AuthMethodId: s.authMethod.GetPublicId(),
+				Attrs: &pbs.AuthenticateRequest_OidcStartAttributes{
+					OidcStartAttributes: &pbs.OidcStartAttributes{
+						RoundtripPayload: func() *structpb.Struct {
+							ret, err := structpb.NewStruct(map[string]any{
+								"foo": "bar",
+								"baz": true,
+							})
+							require.NoError(t, err)
+							return ret
+						}(),
+					},
+				},
+			},
+			wantErr: handlers.ApiErrorWithCode(codes.FailedPrecondition),
+		},
 		// NOTE: We can't really test bad roundtrip payload attributes because
 		// any type structpb lets us use in creation will be valid for JSON, and
 		// attempting to force with e.g. json.RawMessage causes structpb to
@@ -1692,6 +1744,10 @@ func TestAuthenticate_OIDC_Start(t *testing.T) {
 
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
+			if tc.beforeTest != nil {
+				tc.beforeTest(t)
+			}
+
 			assert, require := assert.New(t), require.New(t)
 			got, err := s.authMethodService.Authenticate(auth.DisabledAuthTestContext(s.iamRepoFn, s.org.GetPublicId()), tc.request)
 			if tc.wantErr != nil {