[RFC] PAM: Build GDM support only on the library

.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 \

.gitignore

@@ -29,3 +29,6 @@ vendor_rust/
 
 # Go workspace file
 go.work
+
+# vscode loca files
+!.vscode/*.json.default

.vscode/settings.json.default

@@ -0,0 +1,5 @@
+{
+    "go.buildTags": "withgdmmodel",
+    "go.lintTool": "golangci-lint"
+}
+

CONTRIBUTING.md

@@ -144,7 +144,7 @@ Then build the PAM module:
 
 ```shell
 go generate ./pam/
-go build -tags pam_binary_exec -o ./pam/authd-pam ./pam
+go build -o ./pam/authd-pam ./pam
 ```
 
 This last command will produce two libraries (`./pam/pam_authd.so` and `./pam/go-exec/pam_authd_exec.so`) and an executable (`./pam/authd-pam`).
@@ -170,7 +170,7 @@ The library resulting from the build is located in `./target/debug/libnss_authd.
 
 The project includes a comprehensive test suite made of unit and integration tests. All the tests must pass before the review is considered. If you have troubles with the test suite, feel free to mention it in your PR description.
 
-You can run all tests with: `go test ./...` (add the `-race` flag for race detection).
+You can run all tests with: `go test -tags withgdmmodel ./...` (add the `-race` flag for race detection).
 
 Every package has a suite of at least package-level tests. They may integrate more granular unit tests for complex functionalities. Integration tests are located in `./pam/integration-tests` for the PAM module and `./nss/integration-tests` for the NSS module.
 

debian/rules

@@ -91,6 +91,9 @@ override_dh_auto_build:
 	# Build the daemon
 	dh_auto_build -- $(AUTHD_GO_PACKAGE)/cmd/authd
 
+override_dh_auto_test:
+	dh_auto_test -- -tags withgdmmodel
+
 override_dh_auto_install:
 	dh_auto_install --destdir=debian/tmp -- --no-source
 

pam/generate.go

@@ -2,6 +2,6 @@
 
 //go:generate go generate -C internal/proto
 
-//go:generate ./generate.sh -tags !pam_debug
+//go:generate ./generate.sh -tags "!pam_debug && withgdmmodel" -build-tags withgdmmodel
 
 package main

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_debug -build-tags pam_gdm_debug -build-flags "\"-gcflags=all=-N -l\"" -output pam_module_debug.go
+//go:generate env CFLAGS=-g3 CGO_CFLAGS=-g3 ./generate.sh -tags "pam_debug && withgdmmodel" -build-tags "pam_gdm_debug,withgdmmodel" -build-flags "\"-gcflags=all=-N -l\"" -output pam_module_debug.go
 
 package main

pam/integration-tests/gdm-module-handler_test.go

@@ -1,3 +1,5 @@
+//go:build withgdmmodel
+
 package main_test
 
 import (

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() {
@@ -889,7 +894,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) != "" {
@@ -925,3 +930,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)
+}

pam/integration-tests/helpers_test.go

@@ -14,15 +14,13 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/require"
-	"github.com/ubuntu/authd"
-	"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/pam_test"
-	"google.golang.org/grpc"
-	"google.golang.org/grpc/credentials/insecure"
 )
 
+const authdCurrentUserRootEnvVariableContent = "AUTHD_INTEGRATIONTESTS_CURRENT_USER_AS_ROOT=1"
+
 func runAuthd(t *testing.T, gpasswdOutput, groupsFile string, currentUserAsRoot bool) string {
 	t.Helper()
 
@@ -131,27 +129,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)
-}
-
 // saveArtifactsForDebug saves the specified artifacts to a temporary directory if the test failed.
 func saveArtifactsForDebug(t *testing.T, artifacts []string) {
 	t.Helper()

pam/integration-tests/cli-integration_test.go → pam/integration-tests/main-integration_test.go

@@ -1,3 +1,5 @@
+//go:build withgdmmodel
+
 package main_test
 
 import (
@@ -8,8 +10,6 @@ import (
 	"github.com/ubuntu/authd/internal/testutils"
 )
 
-const authdCurrentUserRootEnvVariableContent = "AUTHD_INTEGRATIONTESTS_CURRENT_USER_AS_ROOT=1"
-
 func TestMain(m *testing.M) {
 	// Needed to skip the test setup when running the gpasswd mock.
 	if os.Getenv("GO_WANT_HELPER_PROCESS") != "" {

pam/integration-tests/main-invalid_test.go

@@ -0,0 +1,12 @@
+//go:build !withgdmmodel
+
+package main_test
+
+import (
+	"log"
+	"testing"
+)
+
+func TestMain(m *testing.M) {
+	log.Fatal("Setup: Tests must be compiled with `withgdmmodel` tag")
+}

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).

pam/internal/adapter/gdmmodel.go

@@ -1,3 +1,5 @@
+//go:build withgdmmodel
+
 package adapter
 
 import (
@@ -40,7 +42,7 @@ type gdmPollDone struct{}
 type gdmIsAuthenticatedResultReceived isAuthenticatedResultReceived
 
 // Init initializes the main model orchestrator.
-func (m *gdmModel) Init() tea.Cmd {
+func (m gdmModel) Init() tea.Cmd {
 	return tea.Sequence(m.protoHello(),
 		requestUICapabilities(m.pamMTx),
 		m.pollGdm())
@@ -186,7 +188,7 @@ func (m *gdmModel) emitEventSync(event gdm.Event) tea.Msg {
 	return nil
 }
 
-func (m gdmModel) Update(msg tea.Msg) (gdmModel, tea.Cmd) {
+func (m gdmModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 	if m.conversationsStopped {
 		return m, nil
 	}
@@ -282,7 +284,6 @@ func (m gdmModel) Update(msg tea.Msg) (gdmModel, tea.Cmd) {
 		return m, sendEvent(m.emitEventSync(&gdm.EventData_AuthEvent{
 			AuthEvent: &gdm.Events_AuthEvent{Response: &authd.IAResponse{
 				Access: brokers.AuthCancelled,
-				Msg:    msg.msg,
 			}},
 		}))
 	}
@@ -310,7 +311,7 @@ func (m gdmModel) changeStage(s proto.Stage) tea.Cmd {
 	}
 }
 
-func (m gdmModel) stopConversations() gdmModel {
+func (m gdmModel) stopConversations() gdmModeler {
 	// We're about to exit: let's ensure that all the messages have been processed.
 
 	wait := make(chan struct{})
@@ -333,3 +334,11 @@ func (m gdmModel) stopConversations() gdmModel {
 	m.conversationsStopped = true
 	return m
 }
+
+func (m gdmModel) View() string {
+	return ""
+}
+
+func (m gdmModel) conversationsActive() bool {
+	return !m.conversationsStopped
+}

pam/internal/adapter/gdmmodel_convhandler_test.go

@@ -1,3 +1,5 @@
+//go:build withgdmmodel
+
 package adapter
 
 import (

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}
+}

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
+}

pam/internal/adapter/gdmmodel_helpers_test.go

@@ -1,3 +1,5 @@
+//go:build withgdmmodel
+
 package adapter
 
 import (

pam/internal/adapter/gdmmodel_test.go

@@ -1,3 +1,5 @@
+//go:build withgdmmodel
+
 package adapter
 
 import (
@@ -2477,7 +2479,7 @@ func TestGdmModel(t *testing.T) {
 				require.Equal(t, tc.wantExitStatus, appState.ExitStatus())
 			}
 
-			require.True(t, appState.gdmModel.conversationsStopped)
+			require.False(t, appState.gdmModel.conversationsActive())
 
 			for _, req := range tc.wantNoGdmRequests {
 				require.NotContains(t, gdmHandler.handledRequests, req)

pam/internal/adapter/gdmmodel_uimodel_test.go

@@ -1,3 +1,5 @@
+//go:build withgdmmodel
+
 package adapter
 
 import (

pam/internal/adapter/gdmmodeler.go

@@ -0,0 +1,13 @@
+package adapter
+
+import (
+	tea "github.com/charmbracelet/bubbletea"
+	"github.com/ubuntu/authd/pam/internal/proto"
+)
+
+type gdmModeler interface {
+	tea.Model
+	changeStage(proto.Stage) tea.Cmd
+	stopConversations() gdmModeler
+	conversationsActive() bool
+}

pam/internal/adapter/model.go

@@ -57,7 +57,7 @@ type UIModel struct {
 	brokerSelectionModel   brokerSelectionModel
 	authModeSelectionModel authModeSelectionModel
 	authenticationModel    authenticationModel
-	gdmModel               gdmModel
+	gdmModel               gdmModeler
 	nativeModel            nativeModel
 
 	exitStatus PamReturnStatus
@@ -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}
@@ -264,7 +264,9 @@ func (m *UIModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 		var modelCmd tea.Cmd
 		switch m.ClientType {
 		case Gdm:
-			m.gdmModel, modelCmd = m.gdmModel.Update(msg)
+			var gdmModel tea.Model
+			gdmModel, modelCmd = m.gdmModel.Update(msg)
+			m.gdmModel, _ = gdmModel.(gdmModeler)
 		case Native:
 			m.nativeModel, modelCmd = m.nativeModel.Update(msg)
 		}
@@ -299,7 +301,8 @@ func (m *UIModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 
 	switch m.ClientType {
 	case Gdm:
-		m.gdmModel, cmd = m.gdmModel.Update(msg)
+		gdmModel, cmd := m.gdmModel.Update(msg)
+		m.gdmModel, _ = gdmModel.(gdmModeler)
 		cmds = append(cmds, cmd)
 	case Native:
 		m.nativeModel, cmd = m.nativeModel.Update(msg)