PAM: Build GDM support only on the library

We don't really use all the gdm-related code in the PAM Exec child that we use in normal PAM transactions, so no need to compile it at all by default while we need it in the PAM library that GDM will consume.
ubuntu · Oct 2, 2024 · 521bae5 · 521bae5
1 parent 6fd5788
commit 521bae5
Show file tree

Hide file tree

Showing 29 changed files with 106 additions and 46 deletions.
diff --git a/.github/workflows/qa.yaml b/.github/workflows/qa.yaml
@@ -201,7 +201,7 @@ jobs:
           # Overriding the default coverage directory is not an exported flag of go test (yet), so
           # we need to override it using the test.gocoverdir flag instead.
           #TODO: Update when https://go-review.googlesource.com/c/go/+/456595 is merged.
-          go test -timeout ${GO_TESTS_TIMEOUT} -cover -covermode=set ./... -coverpkg=./... -shuffle=on -args -test.gocoverdir="${raw_cov_dir}"
+          go test -tags withgdmmodel -timeout ${GO_TESTS_TIMEOUT} -cover -covermode=set ./... -coverpkg=./... -shuffle=on -args -test.gocoverdir="${raw_cov_dir}"
 
           # Convert the raw coverage data into textfmt so we can merge the Rust one into it
           go tool covdata textfmt -i="${raw_cov_dir}" -o="${cov_dir}/coverage.out"
@@ -220,7 +220,7 @@ jobs:
         env:
           GO_TESTS_TIMEOUT: 35m
         run: |
-          go test -timeout ${GO_TESTS_TIMEOUT} -race ./...
+          go test -tags withgdmmodel -timeout ${GO_TESTS_TIMEOUT} -race ./...
 
       - name: Run PAM tests (with Address Sanitizer)
         if: matrix.test == 'asan'
@@ -233,14 +233,14 @@ jobs:
           # Use these flags to give ASAN a better time to unwind the stack trace
           GO_GC_FLAGS: -N -l
         run: |
-          go test -C ./pam/internal -asan -gcflags=all="${GO_GC_FLAGS}" -timeout ${GO_TESTS_TIMEOUT} ./... || exit_code=$?
+          go test -C ./pam/internal -tags withgdmmodel -asan -gcflags=all="${GO_GC_FLAGS}" -timeout ${GO_TESTS_TIMEOUT} ./... || exit_code=$?
           if [ "${exit_code}" -ne 0 ]; then
             cat "${AUTHD_TEST_ARTIFACTS_PATH}"/asan.log* || true
             exit ${exit_code}
           fi
 
           pushd ./pam/integration-tests
-          go test -asan -gcflags=all="${GO_GC_FLAGS}" -c
+          go test -tags withgdmmodel -asan -gcflags=all="${GO_GC_FLAGS}" -c
           # FIXME: Suppression may be removed with newer libpam, as the one we ship in ubuntu as some leaks
           env LSAN_OPTIONS=suppressions=$(pwd)/lsan.supp \
             ./integration-tests.test \

diff --git a/pam/generate.go b/pam/generate.go
@@ -2,6 +2,6 @@
 
 //go:generate go generate -C internal/proto
 
-//go:generate ./generate.sh -tags "!pam_binary_cli && !pam_debug"
+//go:generate ./generate.sh -tags "!pam_binary_cli && !pam_debug" -build-tags withgdmmodel
 
 package main
diff --git a/pam/generate_debug.go b/pam/generate_debug.go
@@ -2,6 +2,6 @@
 
 //go:generate go generate -C internal/proto
 
-//go:generate env CFLAGS=-g3 CGO_CFLAGS=-g3 ./generate.sh -tags "!pam_binary_cli && pam_debug" -build-tags pam_gdm_debug -output pam_module_debug.go
+//go:generate env CFLAGS=-g3 CGO_CFLAGS=-g3 ./generate.sh -tags "!pam_binary_cli && pam_debug" -build-tags "pam_gdm_debug,withgdmmodel" -output pam_module_debug.go
 
 package main
diff --git a/pam/integration-tests/gdm-module-handler_test.go b/pam/integration-tests/gdm-module-handler_test.go
@@ -1,3 +1,5 @@
+//go:build withgdmmodel
+
 package main_test
 
 import (

diff --git a/pam/integration-tests/gdm_test.go b/pam/integration-tests/gdm_test.go
@@ -1,3 +1,5 @@
+//go:build withgdmmodel
+
 package main_test
 
 import (
@@ -14,12 +16,15 @@ import (
 	"github.com/stretchr/testify/require"
 	"github.com/ubuntu/authd"
 	"github.com/ubuntu/authd/internal/brokers"
+	"github.com/ubuntu/authd/internal/services/errmessages"
 	"github.com/ubuntu/authd/internal/testutils"
 	localgroupstestutils "github.com/ubuntu/authd/internal/users/localgroups/testutils"
 	"github.com/ubuntu/authd/pam/internal/gdm"
 	"github.com/ubuntu/authd/pam/internal/gdm_test"
 	"github.com/ubuntu/authd/pam/internal/pam_test"
 	"github.com/ubuntu/authd/pam/internal/proto"
+	"google.golang.org/grpc"
+	"google.golang.org/grpc/credentials/insecure"
 )
 
 func enableGdmExtension() {
@@ -888,7 +893,7 @@ func buildPAMModule(t *testing.T) string {
 	libPath := filepath.Join(t.TempDir(), "libpam_authd.so")
 	t.Logf("Compiling PAM library at %s", libPath)
 
-	cmd.Args = append(cmd.Args, "-tags=pam_debug,pam_gdm_debug", "-o", libPath)
+	cmd.Args = append(cmd.Args, "-tags=withgdmmodel,pam_debug,pam_gdm_debug", "-o", libPath)
 	out, err := cmd.CombinedOutput()
 	require.NoError(t, err, "Setup: could not compile PAM module: %s", out)
 	if string(out) != "" {
@@ -924,3 +929,24 @@ func testQrcodeUILayoutData(reqN int) *authd.UILayout {
 		Entry:   base.Entry,
 	}
 }
+
+func requirePreviousBrokerForUser(t *testing.T, socketPath string, brokerName string, user string) {
+	t.Helper()
+
+	conn, err := grpc.NewClient("unix://"+socketPath, grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithUnaryInterceptor(errmessages.FormatErrorMessage))
+	require.NoError(t, err, "Can't connect to authd socket")
+
+	t.Cleanup(func() { conn.Close() })
+	pamClient := authd.NewPAMClient(conn)
+	brokers, err := pamClient.AvailableBrokers(context.TODO(), nil)
+	require.NoError(t, err, "Can't get available brokers")
+	prevBroker, err := pamClient.GetPreviousBroker(context.TODO(), &authd.GPBRequest{Username: user})
+	require.NoError(t, err, "Can't get previous broker")
+	var prevBrokerID string
+	for _, b := range brokers.BrokersInfos {
+		if b.Name == brokerName {
+			prevBrokerID = b.Id
+		}
+	}
+	require.Equal(t, prevBroker.PreviousBroker, prevBrokerID)
+}
diff --git a/pam/integration-tests/helpers_test.go b/pam/integration-tests/helpers_test.go
@@ -1,7 +1,6 @@
 package main_test
 
 import (
-	"context"
 	"errors"
 	"io/fs"
 	"math"
@@ -11,11 +10,7 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/require"
-	"github.com/ubuntu/authd"
-	"github.com/ubuntu/authd/internal/services/errmessages"
 	"github.com/ubuntu/authd/internal/testutils"
-	"google.golang.org/grpc"
-	"google.golang.org/grpc/credentials/insecure"
 )
 
 func prepareFileLogging(t *testing.T, fileName string) string {
@@ -35,27 +30,6 @@ func prepareFileLogging(t *testing.T, fileName string) string {
 	return cliLog
 }
 
-func requirePreviousBrokerForUser(t *testing.T, socketPath string, brokerName string, user string) {
-	t.Helper()
-
-	conn, err := grpc.NewClient("unix://"+socketPath, grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithUnaryInterceptor(errmessages.FormatErrorMessage))
-	require.NoError(t, err, "Can't connect to authd socket")
-
-	t.Cleanup(func() { conn.Close() })
-	pamClient := authd.NewPAMClient(conn)
-	brokers, err := pamClient.AvailableBrokers(context.TODO(), nil)
-	require.NoError(t, err, "Can't get available brokers")
-	prevBroker, err := pamClient.GetPreviousBroker(context.TODO(), &authd.GPBRequest{Username: user})
-	require.NoError(t, err, "Can't get previous broker")
-	var prevBrokerID string
-	for _, b := range brokers.BrokersInfos {
-		if b.Name == brokerName {
-			prevBrokerID = b.Id
-		}
-	}
-	require.Equal(t, prevBroker.PreviousBroker, prevBrokerID)
-}
-
 func saveArtifactsForDebugOnCleanup(t *testing.T, artifacts []string) {
 	t.Helper()
 	t.Cleanup(func() { saveArtifactsForDebug(t, artifacts) })

diff --git a/pam/internal/adapter/authentication.go b/pam/internal/adapter/authentication.go
@@ -93,9 +93,7 @@ type isAuthenticatedResultReceived struct {
 }
 
 // isAuthenticatedCancelled is the event to cancel the auth request.
-type isAuthenticatedCancelled struct {
-	msg string
-}
+type isAuthenticatedCancelled struct{}
 
 // reselectAuthMode signals to restart auth mode selection with the same id (to resend sms or
 // reenable the broker).

diff --git a/pam/internal/adapter/gdmmodel.go b/pam/internal/adapter/gdmmodel.go
@@ -1,3 +1,5 @@
+//go:build withgdmmodel
+
 package adapter
 
 import (
@@ -286,7 +288,6 @@ func (m gdmModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 		return m, sendEvent(m.emitEventSync(&gdm.EventData_AuthEvent{
 			AuthEvent: &gdm.Events_AuthEvent{Response: &authd.IAResponse{
 				Access: brokers.AuthCancelled,
-				Msg:    msg.msg,
 			}},
 		}))
 	}

diff --git a/pam/internal/adapter/gdmmodel_convhandler_test.go b/pam/internal/adapter/gdmmodel_convhandler_test.go
@@ -1,3 +1,5 @@
+//go:build withgdmmodel
+
 package adapter
 
 import (

diff --git a/pam/internal/adapter/gdmmodel_default.go b/pam/internal/adapter/gdmmodel_default.go
@@ -0,0 +1,9 @@
+//go:build withgdmmodel
+
+package adapter
+
+import "github.com/msteinert/pam/v2"
+
+func getGdmModel(pamMTx pam.ModuleTransaction) gdmModeler {
+	return gdmModel{pamMTx: pamMTx}
+}
diff --git a/pam/internal/adapter/gdmmodel_disabled.go b/pam/internal/adapter/gdmmodel_disabled.go
@@ -0,0 +1,9 @@
+//go:build !withgdmmodel
+
+package adapter
+
+import "github.com/msteinert/pam/v2"
+
+func getGdmModel(pamMTx pam.ModuleTransaction) gdmModeler {
+	return nil
+}
diff --git a/pam/internal/adapter/gdmmodel_helpers_test.go b/pam/internal/adapter/gdmmodel_helpers_test.go
@@ -1,3 +1,5 @@
+//go:build withgdmmodel
+
 package adapter
 
 import (

diff --git a/pam/internal/adapter/gdmmodel_test.go b/pam/internal/adapter/gdmmodel_test.go
@@ -1,3 +1,5 @@
+//go:build withgdmmodel
+
 package adapter
 
 import (

diff --git a/pam/internal/adapter/gdmmodel_uimodel_test.go b/pam/internal/adapter/gdmmodel_uimodel_test.go
@@ -1,3 +1,5 @@
+//go:build withgdmmodel
+
 package adapter
 
 import (

diff --git a/pam/internal/adapter/model.go b/pam/internal/adapter/model.go
@@ -107,7 +107,7 @@ func (m *UIModel) Init() tea.Cmd {
 
 	switch m.ClientType {
 	case Gdm:
-		m.gdmModel = gdmModel{pamMTx: m.PamMTx}
+		m.gdmModel = getGdmModel(m.PamMTx)
 		cmds = append(cmds, m.gdmModel.Init())
 	case Native:
 		m.nativeModel = nativeModel{pamMTx: m.PamMTx, nssClient: m.NssClient}

diff --git a/pam/internal/gdm/conversation.go b/pam/internal/gdm/conversation.go
@@ -1,3 +1,5 @@
+//go:build withgdmmodel
+
 package gdm
 
 import "C"

diff --git a/pam/internal/gdm/conversation_test.go b/pam/internal/gdm/conversation_test.go
@@ -1,3 +1,5 @@
+//go:build withgdmmodel
+
 package gdm
 
 import (

diff --git a/pam/internal/gdm/export_test.go b/pam/internal/gdm/export_test.go
@@ -1,3 +1,5 @@
+//go:build withgdmmodel
+
 package gdm
 
 func init() {

diff --git a/pam/internal/gdm/extension.go b/pam/internal/gdm/extension.go
@@ -1,3 +1,5 @@
+//go:build withgdmmodel
+
 package gdm
 
 /*

diff --git a/pam/internal/gdm/extension_test.go b/pam/internal/gdm/extension_test.go
@@ -1,3 +1,5 @@
+//go:build withgdmmodel
+
 package gdm
 
 import (

diff --git a/pam/internal/gdm/extension_unsupported.go b/pam/internal/gdm/extension_unsupported.go
@@ -0,0 +1,12 @@
+//go:build !withgdmmodel
+
+// Package gdm is the package for the GDM pam module handing.
+package gdm
+
+// PamExtensionCustomJSON is the gdm PAM extension for passing string values.
+const PamExtensionCustomJSON = ""
+
+// IsPamExtensionSupported returns if the provided extension is supported.
+func IsPamExtensionSupported(extension string) bool {
+	return false
+}
diff --git a/pam/internal/gdm/gdm.pb.go b/pam/internal/gdm/gdm.pb.go
diff --git a/pam/internal/gdm/generate.go b/pam/internal/gdm/generate.go
@@ -0,0 +1,5 @@
+//go:build generate
+
+package gdm
+
+//go:generate ../../../tools/generate-proto.sh -I../../.. -I../proto --with-build-tag withgdmmodel gdm.proto
diff --git a/pam/internal/gdm/protocol.go b/pam/internal/gdm/protocol.go
@@ -1,4 +1,4 @@
-//go:generate ../../../tools/generate-proto.sh -I../../.. -I../proto gdm.proto
+//go:build withgdmmodel
 
 // Package gdm is the package for the GDM pam module handing.
 package gdm

diff --git a/pam/internal/gdm/protocol_test.go b/pam/internal/gdm/protocol_test.go
@@ -1,3 +1,5 @@
+//go:build withgdmmodel
+
 package gdm_test
 
 import (

diff --git a/pam/internal/gdm_test/gdm_utils.go b/pam/internal/gdm_test/gdm_utils.go
@@ -1,3 +1,5 @@
+//go:build withgdmmodel
+
 package gdm_test
 
 import (

diff --git a/pam/internal/proto/pam.pb.go b/pam/internal/proto/pam.pb.go
diff --git a/pam/pam_module.go b/pam/pam_module.go
diff --git a/pam/pam_module_debug.go b/pam/pam_module_debug.go