tests(pam/testutils): Integration tests for the PAM CLI (#109)

This adds integration tests for the PAM module and moves some code
around to avoid copying and maintain consistency between our integration
tests.
The golden files are `ASCII` representations of terminal screenshots, so
don't get scared by the number of lines changed (I promise it will be
okay).

UDENG-1456

Author: denisonbarbosa

.github/workflows/qa.yaml

@@ -10,6 +10,9 @@ on:
 env:
   apt_deps: >-
     libpam0g-dev
+
+  test_apt_deps: >-
+    ffmpeg
   # In Rust the grpc stubs are generated at build time
   # so we always need to install the protobuf compilers
   # when building the NSS crate.
@@ -31,6 +34,11 @@ jobs:
         with:
           golangci-lint-configfile: ".golangci.yaml"
           tools-directory: "tools"
+        env:
+          # The PAM module generator relies on this environment variable to define the output directory for the modules.
+          # If it does not exist, it uses a default value but emits a warning which fails the action so we need to
+          # define the variable for the action to pass successfully.
+          AUTHD_PAM_MODULES_PATH: "/tmp/authd_pam_modules"
       - name: Build cmd/authd with withexamplebroker tag
         run: |
           set -eu
@@ -89,11 +97,21 @@ jobs:
           sudo DEBIAN_FRONTEND=noninteractive apt update
 
           # The integration tests build the NSS crate, so we need the cargo build dependencies in order to run them.
-          sudo DEBIAN_FRONTEND=noninteractive apt install -y ${{ env.apt_deps }} ${{ env.protobuf_compilers}}
+          sudo DEBIAN_FRONTEND=noninteractive apt install -y ${{ env.apt_deps }} ${{ env.protobuf_compilers}} ${{ env.test_apt_deps }}
       - uses: actions/checkout@v4
       - uses: actions/setup-go@v5
         with:
           go-version-file: go.mod
+      - name: Install VHS and ttyd for integration tests
+        run: |
+          set -eu
+          go install github.com/charmbracelet/vhs@latest
+
+          # VHS requires ttyd >= 1.7.2 to work properly.
+          wget https://github.com/tsl0922/ttyd/releases/download/1.7.4/ttyd.x86_64
+          chmod +x ttyd.x86_64
+          sudo mv ttyd.x86_64 /usr/bin/ttyd
+
       - uses: actions-rs/toolchain@v1
         with:
           profile: minimal
@@ -137,7 +155,7 @@ jobs:
           CGO_CFLAGS: "-O0 -g3 -fno-omit-frame-pointer"
         run: |
           # Use `-dwarflocationlists` to give ASAN a better time to unwind the stack trace
-          go test -C pam -asan -gcflags="-dwarflocationlists=true" ./...
+          go test -asan -gcflags="-dwarflocationlists=true" ./pam/pam_test ./pam/gdm/
       - name: Upload coverage to Codecov
         uses: codecov/codecov-action@v3
         with:

CONTRIBUTING.md

@@ -8,13 +8,20 @@ These are mostly guidelines, not rules. Use your best judgment, and feel free to
 
 ## Quicklinks
 
-* [Code of Conduct](#code-of-conduct)
-* [Getting Started](#getting-started)
-* [Issues](#issues)
-* [Pull Requests](#pull-requests)
-* [Contributing to the code](#contributing-to-the-code)
-* [Contributor License Agreement](#contributor-license-agreement)
-* [Getting Help](#getting-help)
+- [Contributing to authd](#contributing-to-authd)
+  - [Quicklinks](#quicklinks)
+  - [Code of Conduct](#code-of-conduct)
+  - [Getting Started](#getting-started)
+    - [Issues](#issues)
+    - [Pull Requests](#pull-requests)
+  - [Contributing to the code](#contributing-to-the-code)
+    - [Required dependencies](#required-dependencies)
+    - [Building and running the binaries](#building-and-running-the-binaries)
+    - [About the testsuite](#about-the-testsuite)
+      - [Tests with dependencies](#tests-with-dependencies)
+    - [Code style](#code-style)
+  - [Contributor License Agreement](#contributor-license-agreement)
+  - [Getting Help](#getting-help)
 
 ## Code of Conduct
 
@@ -79,6 +86,15 @@ TODO
 
 The test suite must pass before merging the PR to our main branch. Any new feature, change or fix must be covered by corresponding tests.
 
+#### Tests with dependencies
+
+Some tests, e.g. the [PAM CLI tests](https://github.com/ubuntu/authd/blob/5ba54c0a573f34e99782fe624b090ab229798fc3/pam/integration-tests/integration_test.go#L21), use external tools such as [vhs](https://github.com/charmbracelet/vhs)
+to record and run the tape files needed for the tests. Those tools are not included in the project dependencies and must be installed manually.
+
+For more information about their usage, refer to the following documentations:
+
+- [vhs](https://github.com/charmbracelet/vhs?tab=readme-ov-file#tutorial)
+
 ### Code style
 
 This project follow the Go code-style. For more informative information about the code style in use, please check <https://google.github.io/styleguide/go/>.

debian/rules

@@ -29,6 +29,10 @@ export CARGO_HOME = $(CURDIR)/debian/cargo_home
 # Needed by the pam module loader
 export AUTHD_PAM_MODULES_PATH = /usr/lib/$(DEB_TARGET_GNU_TYPE)/security
 
+# Skip some tests that rely on external dependencies when building package:
+# they need external commands (e.g. `vhs`) that are not available in the build environment.
+export AUTHD_SKIP_EXTERNAL_DEPENDENT_TESTS=1
+
 %:
 	dh $@ --buildsystem=golang --with=golang,apport
 

debian/tests/control

@@ -1,3 +1,3 @@
-Test-Command: go test -v -mod=vendor ./...
+Test-Command: AUTHD_SKIP_EXTERNAL_DEPENDENT_TESTS=1 go test -v -mod=vendor ./...
 Restrictions: allow-stderr
 Depends: @builddeps@

internal/brokers/examplebroker/broker.go

@@ -10,7 +10,6 @@ import (
 	"errors"
 	"fmt"
 	"html/template"
-	"math/rand"
 	"sort"
 	"strings"
 	"sync"
@@ -62,11 +61,12 @@ type Broker struct {
 
 var (
 	exampleUsers = map[string]struct{}{
-		"user1":            {},
-		"user2":            {},
-		"user-mfa":         {},
-		"user-needs-reset": {},
-		"user-can-reset":   {},
+		"user1":             {},
+		"user2":             {},
+		"user-mfa":          {},
+		"user-needs-reset":  {},
+		"user-can-reset":    {},
+		"user-local-groups": {},
 	}
 )
 
@@ -109,6 +109,8 @@ func (b *Broker) NewSession(ctx context.Context, username, lang string) (session
 	case "user-mfa-with-reset":
 		info.neededAuthSteps = 3
 		info.pwdChange = canReset
+	case "user-unexistent":
+		return "", "", fmt.Errorf("user %q does not exist", username)
 	}
 
 	b.currentSessionsMu.Lock()
@@ -370,10 +372,7 @@ func (b *Broker) SelectAuthenticationMode(ctx context.Context, sessionID, authen
 		// start transaction with fideo device
 	case "qrcodewithtypo":
 		// generate the url and finish the prompt on the fly.
-		//nolint:gosec // this is some example code not shipped in production
-		i := rand.Intn(3)
-		contents := []string{"https://ubuntu.com", "https://ubuntu-fr.org", "https://canonical.com"}
-		uiLayoutInfo["content"] = contents[i]
+		uiLayoutInfo["content"] = "https://ubuntu.com"
 		uiLayoutInfo["label"] = uiLayoutInfo["label"] + "1337"
 	}
 
@@ -477,7 +476,7 @@ func (b *Broker) handleIsAuthenticated(ctx context.Context, sessionInfo sessionI
 		}
 		// Send notification to phone1 and wait on server signal to return if OK or not
 		select {
-		case <-time.After(5 * time.Second):
+		case <-time.After(2 * time.Second):
 		case <-ctx.Done():
 			return responses.AuthCancelled, "", nil
 		}
@@ -502,7 +501,7 @@ func (b *Broker) handleIsAuthenticated(ctx context.Context, sessionInfo sessionI
 
 		// simulate direct exchange with the FIDO device
 		select {
-		case <-time.After(5 * time.Second):
+		case <-time.After(2 * time.Second):
 		case <-ctx.Done():
 			return responses.AuthCancelled, "", nil
 		}
@@ -513,7 +512,7 @@ func (b *Broker) handleIsAuthenticated(ctx context.Context, sessionInfo sessionI
 		}
 		// Simulate connexion with remote server to check that the correct code was entered
 		select {
-		case <-time.After(4 * time.Second):
+		case <-time.After(2 * time.Second):
 		case <-ctx.Done():
 			return responses.AuthCancelled, "", nil
 		}
@@ -540,7 +539,7 @@ func (b *Broker) handleIsAuthenticated(ctx context.Context, sessionInfo sessionI
 		}
 	}
 
-	if _, exists := exampleUsers[sessionInfo.username]; !exists {
+	if _, exists := exampleUsers[sessionInfo.username]; !exists && !strings.HasPrefix(sessionInfo.username, "user-integration") {
 		return responses.AuthDenied, `{"message": "user not found"}`, nil
 	}
 	return responses.AuthGranted, fmt.Sprintf(`{"userinfo": %s}`, userInfoFromName(sessionInfo.username)), nil
@@ -660,21 +659,43 @@ func (b *Broker) updateSession(sessionID string, info sessionInfo) error {
 
 // userInfoFromName transform a given name to the strinfigy userinfo string.
 func userInfoFromName(name string) string {
-	user := struct {
+	type groupJSONInfo struct {
 		Name string
-	}{Name: name}
+		UGID string
+	}
 
+	user := struct {
+		Name   string
+		UUID   string
+		Home   string
+		Shell  string
+		Groups []groupJSONInfo
+		Gecos  string
+	}{
+		Name:   name,
+		UUID:   "uuid-" + name,
+		Home:   "/home/" + name,
+		Shell:  "/usr/bin/bash",
+		Groups: []groupJSONInfo{{Name: "group-" + name, UGID: "ugid-" + name}},
+		Gecos:  "gecos for " + name,
+	}
+
+	if name == "user-local-groups" {
+		user.Groups = append(user.Groups, groupJSONInfo{Name: "localgroup", UGID: ""})
+	}
+
+	// only used for tests, we can ignore the template execution error as the returned data will be failing.
 	var buf bytes.Buffer
-
-	// only used for the example, we can ignore the template execution error as the returned data will be failing.
 	_ = template.Must(template.New("").Parse(`{
 		"name": "{{.Name}}",
-		"uuid": "uuid-{{.Name}}",
-		"gecos": "gecos for {{.Name}}",
-		"dir": "/home/{{.Name}}",
-		"shell": "/usr/bin/bash",
-		"avatar": "avatar for {{.Name}}",
-		"groups": [ {"name": "group-{{.Name}}", "ugid": "group-{{.Name}}"}, {"name": "sudo"} ]
+		"uuid": "{{.UUID}}",
+		"gecos": "{{.Gecos}}",
+		"dir": "{{.Home}}",
+		"shell": "{{.Shell}}",
+		"groups": [ {{range $index, $g := .Groups}}
+			{{- if $index}}, {{end -}}
+			{"name": "{{.Name}}", "ugid": "{{.UGID}}"}
+		{{- end}} ]
 	}`)).Execute(&buf, user)
 
 	return buf.String()