Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate unique UIDs and GIDs #663

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions cmd/authd/integrationtests.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"strings"

"github.com/ubuntu/authd/internal/services/permissions"
"github.com/ubuntu/authd/internal/users/localgroups"
"github.com/ubuntu/authd/internal/users/localentries"
)

// load any behaviour modifiers from env variable.
Expand All @@ -21,6 +21,6 @@ func init() {
if gpasswdArgs == "" || grpFilePath == "" {
panic("AUTHD_INTEGRATIONTESTS_GPASSWD_ARGS and AUTHD_INTEGRATIONTESTS_GPASSWD_GRP_FILE_PATH must be set")
}
localgroups.Z_ForTests_SetGpasswdCmd(strings.Split(gpasswdArgs, " "))
localgroups.Z_ForTests_SetGroupPath(grpFilePath)
localentries.Z_ForTests_SetGpasswdCmd(strings.Split(gpasswdArgs, " "))
localentries.Z_ForTests_SetGroupPath(grpFilePath)
}
11 changes: 5 additions & 6 deletions examplebroker/broker.go
Original file line number Diff line number Diff line change
Expand Up @@ -930,12 +930,11 @@ func userInfoFromName(name string) 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,
Name: name,
UUID: "uuid-" + name,
Home: "/home/" + name,
Shell: "/usr/bin/bash",
Gecos: "gecos for " + name,
}

switch name {
Expand Down
22 changes: 6 additions & 16 deletions internal/brokers/broker.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
"github.com/ubuntu/authd/internal/brokers/auth"
"github.com/ubuntu/authd/internal/brokers/layouts"
"github.com/ubuntu/authd/internal/log"
"github.com/ubuntu/authd/internal/users"
"github.com/ubuntu/authd/internal/users/types"
"github.com/ubuntu/decorate"
"golang.org/x/exp/slices"
)
Expand Down Expand Up @@ -184,7 +184,7 @@ func (b Broker) IsAuthenticated(ctx context.Context, sessionID, authenticationDa
return "", "", err
}

d, err := json.Marshal(info.UserInfo)
d, err := json.Marshal(info)
if err != nil {
return "", "", fmt.Errorf("can't marshal UserInfo: %v", err)
}
Expand Down Expand Up @@ -317,22 +317,17 @@ func (b Broker) parseSessionID(sessionID string) string {
return strings.TrimPrefix(sessionID, fmt.Sprintf("%s-", b.ID))
}

type userInfo struct {
users.UserInfo
UUID string
}

// unmarshalUserInfo tries to unmarshal the rawMsg into a userinfo.
func unmarshalUserInfo(rawMsg json.RawMessage) (userInfo, error) {
var u userInfo
func unmarshalUserInfo(rawMsg json.RawMessage) (types.UserInfo, error) {
var u types.UserInfo
if err := json.Unmarshal(rawMsg, &u); err != nil {
return userInfo{}, fmt.Errorf("message is not JSON formatted: %v", err)
return types.UserInfo{}, fmt.Errorf("message is not JSON formatted: %v", err)
}
return u, nil
}

// validateUserInfo checks if the specified userinfo is valid.
func validateUserInfo(uInfo userInfo) (err error) {
func validateUserInfo(uInfo types.UserInfo) (err error) {
defer decorate.OnError(&err, "provided userinfo is invalid")

// Validate username. We don't want to check here if it matches the username from the request, because it's the
Expand All @@ -350,11 +345,6 @@ func validateUserInfo(uInfo userInfo) (err error) {
return fmt.Errorf("value provided for shell is not an absolute path: %s", uInfo.Shell)
}

// Validate UUID
if uInfo.UUID == "" {
return errors.New("empty UUID")
}

// Validate groups
for _, g := range uInfo.Groups {
if g.Name == "" {
Expand Down
1 change: 0 additions & 1 deletion internal/brokers/broker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,6 @@ func TestIsAuthenticated(t *testing.T) {
"Error when broker returns invalid userinfo": {sessionID: "IA_invalid_userinfo"},
"Error when broker returns userinfo with empty username": {sessionID: "IA_info_empty_user_name"},
"Error when broker returns userinfo with empty group name": {sessionID: "IA_info_empty_group_name"},
"Error when broker returns userinfo with empty UUID": {sessionID: "IA_info_empty_uuid"},
"Error when broker returns userinfo with invalid homedir": {sessionID: "IA_info_invalid_home"},
"Error when broker returns userinfo with invalid shell": {sessionID: "IA_info_invalid_shell"},
"Error when broker returns data on auth.Next": {sessionID: "IA_next_with_data"},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FIRST CALL:
access:
data:
err: message is not JSON formatted: json: cannot unmarshal string into Go value of type brokers.userInfo
err: message is not JSON formatted: json: cannot unmarshal string into Go value of type types.UserInfo

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"Name":"success","UID":82162,"Gecos":"gecos for success","Dir":"/home/success","Shell":"/bin/sh/success","Groups":[{"Name":"success","GID":82162,"UGID":""},{"Name":"group-success","GID":81868,"UGID":""}],"UUID":""}
{"Name":"success","UID":82162,"Gecos":"gecos for success","Dir":"/home/success","Shell":"/bin/sh/success","Groups":[{"Name":"success","GID":82162,"UGID":""},{"Name":"group-success","GID":81868,"UGID":""}]}
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"Name":"success","UID":82162,"Gecos":"gecos for success","Dir":"/home/success","Shell":"/bin/sh/success","Groups":[{"Name":"success","GID":82162,"UGID":""},{"Name":"group-success","GID":81868,"UGID":""}],"UUID":""}
{"Name":"success","UID":82162,"Gecos":"gecos for success","Dir":"/home/success","Shell":"/bin/sh/success","Groups":[{"Name":"success","GID":82162,"UGID":""},{"Name":"group-success","GID":81868,"UGID":""}]}
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"Name":"","UID":82162,"Gecos":"gecos for success","Dir":"/home/success","Shell":"/bin/sh/success","Groups":[{"Name":"success","GID":82162,"UGID":""},{"Name":"group-success","GID":81868,"UGID":""}],"UUID":""}
{"Name":"","UID":82162,"Gecos":"gecos for success","Dir":"/home/success","Shell":"/bin/sh/success","Groups":[{"Name":"success","GID":82162,"UGID":""},{"Name":"group-success","GID":81868,"UGID":""}]}
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"Name":"","UID":82162,"Gecos":"gecos for success","Dir":"/home/success","Shell":"/bin/sh/success","Groups":[{"Name":"success","GID":82162,"UGID":""},{"Name":"group-success","GID":81868,"UGID":""}],"UUID":""}
{"Name":"","UID":82162,"Gecos":"gecos for success","Dir":"/home/success","Shell":"/bin/sh/success","Groups":[{"Name":"success","GID":82162,"UGID":""},{"Name":"group-success","GID":81868,"UGID":""}]}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "user-pre-check",
"uuid": "uuid-user-pre-check",
"uuid": "",
"gecos": "gecos for user-pre-check",
"dir": "/home/user-pre-check",
"shell": "/bin/sh/user-pre-check",
Expand Down
21 changes: 13 additions & 8 deletions internal/services/nss/nss.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/ubuntu/authd/internal/proto/authd"
"github.com/ubuntu/authd/internal/services/permissions"
"github.com/ubuntu/authd/internal/users"
"github.com/ubuntu/authd/internal/users/types"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
)
Expand Down Expand Up @@ -180,19 +181,23 @@ func (s Service) userPreCheck(ctx context.Context, username string) (pwent *auth
return nil, fmt.Errorf("user %q is not known by any broker", username)
}

var u users.UserEntry
var u types.UserEntry
if err := json.Unmarshal([]byte(userinfo), &u); err != nil {
return nil, fmt.Errorf("user data from broker invalid: %v", err)
}
// We need to generate the ID for the user, as its business logic is authd responsibility, not the broker's.
u.UID = s.userManager.GenerateUID(u.Name)
u.GID = s.userManager.GenerateGID(u.Name)

// Register a temporary user with a unique UID. If the user authenticates successfully, the user will be added to
// the database with the same UID.
u.UID, err = s.userManager.RegisterUserPreAuth(u.Name)
if err != nil {
return nil, fmt.Errorf("failed to add temporary record for user %q: %v", username, err)
}

return nssPasswdFromUsersPasswd(u), nil
}

// nssPasswdFromUsersPasswd returns a PasswdEntry from users.UserEntry.
func nssPasswdFromUsersPasswd(u users.UserEntry) *authd.PasswdEntry {
func nssPasswdFromUsersPasswd(u types.UserEntry) *authd.PasswdEntry {
return &authd.PasswdEntry{
Name: u.Name,
Passwd: "x",
Expand All @@ -205,17 +210,17 @@ func nssPasswdFromUsersPasswd(u users.UserEntry) *authd.PasswdEntry {
}

// nssGroupFromUsersGroup returns a GroupEntry from users.GroupEntry.
func nssGroupFromUsersGroup(g users.GroupEntry) *authd.GroupEntry {
func nssGroupFromUsersGroup(g types.GroupEntry) *authd.GroupEntry {
return &authd.GroupEntry{
Name: g.Name,
Passwd: "x",
Passwd: g.Passwd,
3v1n0 marked this conversation as resolved.
Show resolved Hide resolved
Gid: g.GID,
Members: g.Users,
}
}

// nssShadowFromUsersShadow returns a ShadowEntry from users.ShadowEntry.
func nssShadowFromUsersShadow(u users.ShadowEntry) *authd.ShadowEntry {
func nssShadowFromUsersShadow(u types.ShadowEntry) *authd.ShadowEntry {
return &authd.ShadowEntry{
Name: u.Name,
Passwd: "x",
Expand Down
15 changes: 13 additions & 2 deletions internal/services/nss/nss_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/stretchr/testify/require"
"github.com/ubuntu/authd/internal/brokers"
"github.com/ubuntu/authd/internal/log"
"github.com/ubuntu/authd/internal/proto/authd"
"github.com/ubuntu/authd/internal/services/errmessages"
"github.com/ubuntu/authd/internal/services/nss"
Expand All @@ -18,7 +19,8 @@ import (
"github.com/ubuntu/authd/internal/testutils/golden"
"github.com/ubuntu/authd/internal/users"
"github.com/ubuntu/authd/internal/users/cache"
localgroupstestutils "github.com/ubuntu/authd/internal/users/localgroups/testutils"
"github.com/ubuntu/authd/internal/users/idgenerator"
localgroupstestutils "github.com/ubuntu/authd/internal/users/localentries/testutils"
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/credentials/insecure"
Expand Down Expand Up @@ -331,7 +333,14 @@ func newUserManagerForTests(t *testing.T, sourceDB string) *users.Manager {
}
cache.Z_ForTests_CreateDBFromYAML(t, filepath.Join("testdata", sourceDB), cacheDir)

m, err := users.NewManager(users.DefaultConfig, cacheDir)
managerOpts := []users.Option{
users.WithIDGenerator(&idgenerator.IDGeneratorMock{
UIDsToGenerate: []uint32{1234},
GIDsToGenerate: []uint32{1234},
}),
}

m, err := users.NewManager(users.DefaultConfig, cacheDir, managerOpts...)
require.NoError(t, err, "Setup: could not create user manager")

t.Cleanup(func() { _ = m.Stop() })
Expand Down Expand Up @@ -416,6 +425,8 @@ func TestMain(m *testing.M) {
os.Exit(m.Run())
}

log.SetLevel(log.DebugLevel)

cleanup, err := testutils.StartSystemBusMock()
if err != nil {
fmt.Println("Error starting system bus mock:", err)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: group1
passwd: x
passwd: ""
gid: 11111
members:
- user1
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: group1
passwd: x
passwd: ""
gid: 11111
members:
- user1
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
- name: group1
passwd: x
passwd: ""
gid: 11111
members:
- user1
- name: group2
passwd: x
passwd: ""
gid: 22222
members:
- user2
- name: group3
passwd: x
passwd: ""
gid: 33333
members:
- user3
- name: commongroup
passwd: x
passwd: ""
gid: 99999
members:
- user2
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name: user-pre-check
passwd: x
uid: 1053432963
gid: 1053432963
uid: 1234
gid: 0
gecos: gecos for user-pre-check
homedir: /home/user-pre-check
shell: /bin/sh/user-pre-check
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name: User-Pre-Check
passwd: x
uid: 1053432963
gid: 1053432963
uid: 1234
gid: 0
gecos: gecos for User-Pre-Check
homedir: /home/User-Pre-Check
shell: /bin/sh/User-Pre-Check
3 changes: 2 additions & 1 deletion internal/services/pam/pam.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/ubuntu/authd/internal/proto/authd"
"github.com/ubuntu/authd/internal/services/permissions"
"github.com/ubuntu/authd/internal/users"
"github.com/ubuntu/authd/internal/users/types"
"github.com/ubuntu/decorate"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
Expand Down Expand Up @@ -262,7 +263,7 @@ func (s Service) IsAuthenticated(ctx context.Context, req *authd.IARequest) (res
}, nil
}

var uInfo users.UserInfo
var uInfo types.UserInfo
if err := json.Unmarshal([]byte(data), &uInfo); err != nil {
return nil, fmt.Errorf("user data from broker invalid: %v", err)
}
Expand Down
Loading
Loading