From 14aa19f7aa96ffa0f2591a6977ac52348c1eaf7d Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 28 Jul 2016 19:14:36 +0200 Subject: [PATCH 1/6] Store original email and openid identifier in the GECOS field LP: #1607121 --- daemon/api.go | 3 ++- daemon/api_test.go | 8 +++++--- osutil/user.go | 4 +--- osutil/user_test.go | 4 ++-- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/daemon/api.go b/daemon/api.go index 60a68d0cfa3..9520fcccd4d 100644 --- a/daemon/api.go +++ b/daemon/api.go @@ -1414,7 +1414,8 @@ func postCreateUser(c *Command, r *http.Request, user *auth.UserState) Response return BadRequest("cannot create user %q: %s", createData.EMail, err) } - if err := osutilAddExtraUser(v.Username, v.SSHKeys); err != nil { + gecos := fmt.Sprintf("%s,%s", createData.EMail, v.OpenIDIdentifier) + if err := osutilAddExtraUser(v.Username, v.SSHKeys, gecos); err != nil { return BadRequest("cannot create user %s: %s", v.Username, err) } diff --git a/daemon/api_test.go b/daemon/api_test.go index 2d78b7b5779..a743d057ac6 100644 --- a/daemon/api_test.go +++ b/daemon/api_test.go @@ -3079,13 +3079,15 @@ func (s *apiSuite) TestPostCreateUser(c *check.C) { storeUserInfo = func(user string) (*store.User, error) { c.Check(user, check.Equals, "popper@lse.ac.uk") return &store.User{ - Username: "karl", - SSHKeys: []string{"ssh1", "ssh2"}, + Username: "karl", + SSHKeys: []string{"ssh1", "ssh2"}, + OpenIDIdentifier: "xxyyzz", }, nil } - osutilAddExtraUser = func(username string, sshKeys []string) error { + osutilAddExtraUser = func(username string, sshKeys []string, gecos string) error { c.Check(username, check.Equals, "karl") c.Check(sshKeys, check.DeepEquals, []string{"ssh1", "ssh2"}) + c.Check(gecos, check.Equals, "popper@lse.ac.uk,xxyyzz") return nil } diff --git a/osutil/user.go b/osutil/user.go index 0ea0cdba809..7c61ea91bb6 100644 --- a/osutil/user.go +++ b/osutil/user.go @@ -31,9 +31,7 @@ import ( var userLookup = user.Lookup -func AddExtraUser(name string, sshKeys []string) error { - // FIXME: put date/time/openid in there - gecos := "created by snapd" +func AddExtraUser(name string, sshKeys []string, gecos string) error { cmd := exec.Command("adduser", "--gecos", gecos, "--extrausers", "--disabled-password", name) if output, err := cmd.CombinedOutput(); err != nil { return fmt.Errorf("adduser failed with %s: %s", err, output) diff --git a/osutil/user_test.go b/osutil/user_test.go index ca6241da8d9..670989ea14f 100644 --- a/osutil/user_test.go +++ b/osutil/user_test.go @@ -49,10 +49,10 @@ func (s *createUserSuite) TestAddExtraUser(c *check.C) { mc := testutil.MockCommand(c, "adduser", "true") defer mc.Restore() - err := osutil.AddExtraUser("karl", []string{"ssh-key1", "ssh-key2"}) + err := osutil.AddExtraUser("karl", []string{"ssh-key1", "ssh-key2"}, "my gecos") c.Assert(err, check.IsNil) c.Check(mc.Calls(), check.DeepEquals, [][]string{ - {"adduser", "--gecos", "created by snapd", "--extrausers", "--disabled-password", "karl"}, + {"adduser", "--gecos", "my gecos", "--extrausers", "--disabled-password", "karl"}, }) sshKeys, err := ioutil.ReadFile(filepath.Join(mockHome, ".ssh", "authorized_keys")) c.Assert(err, check.IsNil) From 0c40ad77e46a7efd279a6a9a22c2d04edcb7c339 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 28 Jul 2016 19:21:48 +0200 Subject: [PATCH 2/6] Error if no ssh keys can be found for a given user in create-user LP: #1607129 --- daemon/api.go | 3 +++ daemon/api_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/daemon/api.go b/daemon/api.go index 9520fcccd4d..a7dc8bbb1a8 100644 --- a/daemon/api.go +++ b/daemon/api.go @@ -1413,6 +1413,9 @@ func postCreateUser(c *Command, r *http.Request, user *auth.UserState) Response if err != nil { return BadRequest("cannot create user %q: %s", createData.EMail, err) } + if len(v.SSHKeys) == 0 { + return BadRequest("cannot create user for %s: no ssh keys found", createData.EMail) + } gecos := fmt.Sprintf("%s,%s", createData.EMail, v.OpenIDIdentifier) if err := osutilAddExtraUser(v.Username, v.SSHKeys, gecos); err != nil { diff --git a/daemon/api_test.go b/daemon/api_test.go index a743d057ac6..0b047507316 100644 --- a/daemon/api_test.go +++ b/daemon/api_test.go @@ -3075,6 +3075,31 @@ func (s *apiSuite) TestStateChangeAbortIsReady(c *check.C) { }) } +func (s *apiSuite) TestPostCreateUserNoSSHKeys(c *check.C) { + storeUserInfo = func(user string) (*store.User, error) { + c.Check(user, check.Equals, "popper@lse.ac.uk") + return &store.User{ + Username: "karl", + OpenIDIdentifier: "xxyyzz", + }, nil + } + postCreateUserUcrednetGetUID = func(string) (uint32, error) { + return 0, nil + } + defer func() { + postCreateUserUcrednetGetUID = ucrednetGetUID + }() + + buf := bytes.NewBufferString(`{"email": "popper@lse.ac.uk"}`) + req, err := http.NewRequest("POST", "/v2/create-user", buf) + c.Assert(err, check.IsNil) + + rsp := postCreateUser(createUserCmd, req, nil).(*resp) + + c.Check(rsp.Type, check.Equals, ResponseTypeError) + c.Check(rsp.Result.(*errorResult).Message, check.Matches, "cannot create user for popper@lse.ac.uk: no ssh keys found") +} + func (s *apiSuite) TestPostCreateUser(c *check.C) { storeUserInfo = func(user string) (*store.User, error) { c.Check(user, check.Equals, "popper@lse.ac.uk") From 0d03ce4160324bdb80cc69f05b0181da9d43f172 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 28 Jul 2016 19:41:08 +0200 Subject: [PATCH 3/6] Allow `.` in the usernames of create-user LP: #1603018 --- osutil/user.go | 10 +++++++++- osutil/user_test.go | 7 ++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/osutil/user.go b/osutil/user.go index 7c61ea91bb6..89d00103531 100644 --- a/osutil/user.go +++ b/osutil/user.go @@ -26,13 +26,21 @@ import ( "os/exec" "os/user" "path/filepath" + "regexp" "strings" ) var userLookup = user.Lookup func AddExtraUser(name string, sshKeys []string, gecos string) error { - cmd := exec.Command("adduser", "--gecos", gecos, "--extrausers", "--disabled-password", name) + // we check the (user)name ourselfs, adduser is a bit too + // strict (i.e. no `.`) + validNames := regexp.MustCompile(`^[a-z][-a-z0-9_.]*$`) + if !validNames.MatchString(name) { + return fmt.Errorf("cannot add user %q: name contains invalid charackters", name) + } + + cmd := exec.Command("adduser", "--force-badname", "--gecos", gecos, "--extrausers", "--disabled-password", name) if output, err := cmd.CombinedOutput(); err != nil { return fmt.Errorf("adduser failed with %s: %s", err, output) } diff --git a/osutil/user_test.go b/osutil/user_test.go index 670989ea14f..6779dd3206e 100644 --- a/osutil/user_test.go +++ b/osutil/user_test.go @@ -52,9 +52,14 @@ func (s *createUserSuite) TestAddExtraUser(c *check.C) { err := osutil.AddExtraUser("karl", []string{"ssh-key1", "ssh-key2"}, "my gecos") c.Assert(err, check.IsNil) c.Check(mc.Calls(), check.DeepEquals, [][]string{ - {"adduser", "--gecos", "my gecos", "--extrausers", "--disabled-password", "karl"}, + {"adduser", "--force-badname", "--gecos", "my gecos", "--extrausers", "--disabled-password", "karl"}, }) sshKeys, err := ioutil.ReadFile(filepath.Join(mockHome, ".ssh", "authorized_keys")) c.Assert(err, check.IsNil) c.Check(string(sshKeys), check.Equals, "ssh-key1\nssh-key2") } + +func (s *createUserSuite) TestAddExtraUserInvalid(c *check.C) { + err := osutil.AddExtraUser("k!", nil, "my gecos") + c.Assert(err, check.ErrorMatches, `cannot add user "k!": name contains invalid charackters`) +} From bb7d040a0830e5041056e8327185e54ceec61658 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Sun, 31 Jul 2016 17:06:58 +0200 Subject: [PATCH 4/6] add user created via create-user to the sudo group --- daemon/api.go | 4 ++-- daemon/api_test.go | 6 +++--- osutil/user.go | 10 ++++++++-- osutil/user_test.go | 10 +++++----- 4 files changed, 18 insertions(+), 12 deletions(-) diff --git a/daemon/api.go b/daemon/api.go index a7dc8bbb1a8..6a024c14123 100644 --- a/daemon/api.go +++ b/daemon/api.go @@ -1384,7 +1384,7 @@ func abortChange(c *Command, r *http.Request, user *auth.UserState) Response { var ( postCreateUserUcrednetGetUID = ucrednetGetUID storeUserInfo = store.UserInfo - osutilAddExtraUser = osutil.AddExtraUser + osutilAddExtraSudoUser = osutil.AddExtraSudoUser ) func postCreateUser(c *Command, r *http.Request, user *auth.UserState) Response { @@ -1418,7 +1418,7 @@ func postCreateUser(c *Command, r *http.Request, user *auth.UserState) Response } gecos := fmt.Sprintf("%s,%s", createData.EMail, v.OpenIDIdentifier) - if err := osutilAddExtraUser(v.Username, v.SSHKeys, gecos); err != nil { + if err := osutilAddExtraSudoUser(v.Username, v.SSHKeys, gecos); err != nil { return BadRequest("cannot create user %s: %s", v.Username, err) } diff --git a/daemon/api_test.go b/daemon/api_test.go index 0b047507316..ff3201ac836 100644 --- a/daemon/api_test.go +++ b/daemon/api_test.go @@ -387,7 +387,7 @@ func (s *apiSuite) TestListIncludesAll(c *check.C) { "snapstateTryPath", "snapstateGet", "readSnapInfo", - "osutilAddExtraUser", + "osutilAddExtraSudoUser", "storeUserInfo", "postCreateUserUcrednetGetUID", "ensureStateSoon", @@ -3109,7 +3109,7 @@ func (s *apiSuite) TestPostCreateUser(c *check.C) { OpenIDIdentifier: "xxyyzz", }, nil } - osutilAddExtraUser = func(username string, sshKeys []string, gecos string) error { + osutilAddExtraSudoUser = func(username string, sshKeys []string, gecos string) error { c.Check(username, check.Equals, "karl") c.Check(sshKeys, check.DeepEquals, []string{"ssh1", "ssh2"}) c.Check(gecos, check.Equals, "popper@lse.ac.uk,xxyyzz") @@ -3120,7 +3120,7 @@ func (s *apiSuite) TestPostCreateUser(c *check.C) { return 0, nil } defer func() { - osutilAddExtraUser = osutil.AddExtraUser + osutilAddExtraSudoUser = osutil.AddExtraSudoUser postCreateUserUcrednetGetUID = ucrednetGetUID }() diff --git a/osutil/user.go b/osutil/user.go index 89d00103531..f473d0d873d 100644 --- a/osutil/user.go +++ b/osutil/user.go @@ -32,7 +32,7 @@ import ( var userLookup = user.Lookup -func AddExtraUser(name string, sshKeys []string, gecos string) error { +func AddExtraSudoUser(name string, sshKeys []string, gecos string) error { // we check the (user)name ourselfs, adduser is a bit too // strict (i.e. no `.`) validNames := regexp.MustCompile(`^[a-z][-a-z0-9_.]*$`) @@ -40,7 +40,13 @@ func AddExtraUser(name string, sshKeys []string, gecos string) error { return fmt.Errorf("cannot add user %q: name contains invalid charackters", name) } - cmd := exec.Command("adduser", "--force-badname", "--gecos", gecos, "--extrausers", "--disabled-password", name) + cmd := exec.Command("adduser", + "--force-badname", + "--gecos", gecos, + "--extrausers", + "--disabled-password", + "--add_extra_groups", "sudo", + name) if output, err := cmd.CombinedOutput(); err != nil { return fmt.Errorf("adduser failed with %s: %s", err, output) } diff --git a/osutil/user_test.go b/osutil/user_test.go index 6779dd3206e..278ec71bb09 100644 --- a/osutil/user_test.go +++ b/osutil/user_test.go @@ -37,7 +37,7 @@ type createUserSuite struct { var _ = check.Suite(&createUserSuite{}) -func (s *createUserSuite) TestAddExtraUser(c *check.C) { +func (s *createUserSuite) TestAddExtraSudoUser(c *check.C) { mockHome := c.MkDir() restorer := osutil.MockUserLookup(func(string) (*user.User, error) { return &user.User{ @@ -49,17 +49,17 @@ func (s *createUserSuite) TestAddExtraUser(c *check.C) { mc := testutil.MockCommand(c, "adduser", "true") defer mc.Restore() - err := osutil.AddExtraUser("karl", []string{"ssh-key1", "ssh-key2"}, "my gecos") + err := osutil.AddExtraSudoUser("karl", []string{"ssh-key1", "ssh-key2"}, "my gecos") c.Assert(err, check.IsNil) c.Check(mc.Calls(), check.DeepEquals, [][]string{ - {"adduser", "--force-badname", "--gecos", "my gecos", "--extrausers", "--disabled-password", "karl"}, + {"adduser", "--force-badname", "--gecos", "my gecos", "--extrausers", "--disabled-password", "--add_extra_groups", "sudo", "karl"}, }) sshKeys, err := ioutil.ReadFile(filepath.Join(mockHome, ".ssh", "authorized_keys")) c.Assert(err, check.IsNil) c.Check(string(sshKeys), check.Equals, "ssh-key1\nssh-key2") } -func (s *createUserSuite) TestAddExtraUserInvalid(c *check.C) { - err := osutil.AddExtraUser("k!", nil, "my gecos") +func (s *createUserSuite) TestAddExtraSudoUserInvalid(c *check.C) { + err := osutil.AddExtraSudoUser("k!", nil, "my gecos") c.Assert(err, check.ErrorMatches, `cannot add user "k!": name contains invalid charackters`) } From ee25c15d8a46ab1921aa693b09043fe5eea0fc1e Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 4 Aug 2016 09:05:11 +0200 Subject: [PATCH 5/6] make the adduser regexp a bit more liberal and make it match what SSO allows --- osutil/user.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/osutil/user.go b/osutil/user.go index f473d0d873d..cde2c7f9632 100644 --- a/osutil/user.go +++ b/osutil/user.go @@ -34,8 +34,9 @@ var userLookup = user.Lookup func AddExtraSudoUser(name string, sshKeys []string, gecos string) error { // we check the (user)name ourselfs, adduser is a bit too - // strict (i.e. no `.`) - validNames := regexp.MustCompile(`^[a-z][-a-z0-9_.]*$`) + // strict (i.e. no `.`) - this regexp is in sync with that SSO + // allows as valid usernames + validNames := regexp.MustCompile(`^[a-z0-9][-a-z0-9+.-_]*$`) if !validNames.MatchString(name) { return fmt.Errorf("cannot add user %q: name contains invalid charackters", name) } From de107027ebc8bb96ffb660242c9acfb47b80033b Mon Sep 17 00:00:00 2001 From: Gustavo Niemeyer Date: Sat, 6 Aug 2016 23:26:02 -0300 Subject: [PATCH 6/6] Fix typos. --- osutil/user.go | 4 ++-- osutil/user_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/osutil/user.go b/osutil/user.go index cde2c7f9632..45e5aba82b6 100644 --- a/osutil/user.go +++ b/osutil/user.go @@ -33,12 +33,12 @@ import ( var userLookup = user.Lookup func AddExtraSudoUser(name string, sshKeys []string, gecos string) error { - // we check the (user)name ourselfs, adduser is a bit too + // we check the (user)name ourselves, adduser is a bit too // strict (i.e. no `.`) - this regexp is in sync with that SSO // allows as valid usernames validNames := regexp.MustCompile(`^[a-z0-9][-a-z0-9+.-_]*$`) if !validNames.MatchString(name) { - return fmt.Errorf("cannot add user %q: name contains invalid charackters", name) + return fmt.Errorf("cannot add user %q: name contains invalid characters", name) } cmd := exec.Command("adduser", diff --git a/osutil/user_test.go b/osutil/user_test.go index 278ec71bb09..573d81b001f 100644 --- a/osutil/user_test.go +++ b/osutil/user_test.go @@ -61,5 +61,5 @@ func (s *createUserSuite) TestAddExtraSudoUser(c *check.C) { func (s *createUserSuite) TestAddExtraSudoUserInvalid(c *check.C) { err := osutil.AddExtraSudoUser("k!", nil, "my gecos") - c.Assert(err, check.ErrorMatches, `cannot add user "k!": name contains invalid charackters`) + c.Assert(err, check.ErrorMatches, `cannot add user "k!": name contains invalid characters`) }