Skip to content

Remove deprecated upsert password endpoint #15855

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.

Sign up for GitHub

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

Merged
strideynet merged 8 commits into from
Aug 30, 2022
Merged

Remove deprecated upsert password endpoint #15855

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9498968
remove UpsertPassword endpoint from Auth HTTP API server
strideynet May 30, 2022
3d53f2b
Merge branch 'master' into strideynet/remove-upsert-password-endpoint
strideynet Aug 25, 2022
ef43ee1
fix merge
strideynet Aug 25, 2022
ca0ea1b
Merge branch 'master' into strideynet/remove-upsert-password-endpoint
strideynet Aug 25, 2022
43c8413
Fix TestUsersCRUD
strideynet Aug 25, 2022
5a1e41d
Merge branch 'master' into strideynet/remove-upsert-password-endpoint
strideynet Aug 25, 2022
ce77088
Merge branch 'master' into strideynet/remove-upsert-password-endpoint
strideynet Aug 25, 2022
95045fa
Merge branch 'master' into strideynet/remove-upsert-password-endpoint
strideynet Aug 30, 2022
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
20 changes: 0 additions & 20 deletions lib/auth/apiserver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ func NewAPIServer(config *APIConfig) (http.Handler, error) {
// Passwords and sessions
srv.POST("/:version/users", srv.withAuth(srv.upsertUser))
srv.PUT("/:version/users/:user/web/password", srv.withAuth(srv.changePassword))
srv.POST("/:version/users/:user/web/password", srv.withAuth(srv.upsertPassword))
srv.POST("/:version/users/:user/web/password/check", srv.withRate(srv.withAuth(srv.checkPassword)))
srv.POST("/:version/users/:user/web/sessions", srv.withAuth(srv.createWebSession))
srv.POST("/:version/users/:user/web/authenticate", srv.withAuth(srv.authenticateWebUser))
Expand Down Expand Up @@ -566,25 +565,6 @@ func (s *APIServer) changePassword(auth ClientI, w http.ResponseWriter, r *http.
return message(fmt.Sprintf("password has been changed for user %q", req.User)), nil
}

type upsertPasswordReq struct {
Password string `json:"password"`
}

func (s *APIServer) upsertPassword(auth ClientI, w http.ResponseWriter, r *http.Request, p httprouter.Params, version string) (interface{}, error) {
var req *upsertPasswordReq
if err := httplib.ReadJSON(r, &req); err != nil {
return nil, trace.Wrap(err)
}

user := p.ByName("user")
err := auth.UpsertPassword(user, []byte(req.Password))
if err != nil {
return nil, trace.Wrap(err)
}

return message(fmt.Sprintf("password for for user %q upserted", user)), nil
}

type upsertUserRawReq struct {
User json.RawMessage `json:"user"`
}
Expand Down
7 changes: 0 additions & 7 deletions lib/auth/auth_with_roles.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1594,13 +1594,6 @@ func (a *ServerWithRoles) CreateToken(ctx context.Context, token types.Provision
return a.authServer.CreateToken(ctx, token)
}

func (a *ServerWithRoles) UpsertPassword(user string, password []byte) error {
if err := a.currentUserAction(user); err != nil {
return trace.Wrap(err)
}
return a.authServer.UpsertPassword(user, password)
}

// ChangePassword updates users password based on the old password.
func (a *ServerWithRoles) ChangePassword(req services.ChangePasswordReq) error {
if err := a.currentUserAction(req.User); err != nil {
Expand Down
17 changes: 0 additions & 17 deletions lib/auth/clt.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -754,21 +754,6 @@ func (c *Client) DeleteProxy(name string) error {
return nil
}

// UpsertPassword updates web access password for the user
func (c *Client) UpsertPassword(user string, password []byte) error {
_, err := c.PostJSON(
context.TODO(),
c.Endpoint("users", user, "web", "password"),
upsertPasswordReq{
Password: string(password),
})
if err != nil {
return trace.Wrap(err)
}

return nil
}

// UpsertUser user updates user entry.
func (c *Client) UpsertUser(user types.User) error {
data, err := services.MarshalUser(user)
Expand Down Expand Up @@ -1399,8 +1384,6 @@ type WebService interface {

// IdentityService manages identities and users
type IdentityService interface {
// UpsertPassword updates web access password for the user
UpsertPassword(user string, password []byte) error
// UpsertOIDCConnector updates or creates OIDC connector
UpsertOIDCConnector(ctx context.Context, connector types.OIDCConnector) error
// GetOIDCConnector returns OIDC connector information by id
Expand Down
19 changes: 10 additions & 9 deletions lib/auth/tls_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1115,15 +1115,16 @@ func TestUsersCRUD(t *testing.T) {
clt, err := tt.server.NewClient(TestAdmin())
require.NoError(t, err)

err = clt.UpsertPassword("user1", []byte("some pass"))
usr, err := types.NewUser("user1")
require.NoError(t, err)
require.NoError(t, clt.CreateUser(ctx, usr))

users, err := clt.GetUsers(false)
require.NoError(t, err)
require.Equal(t, len(users), 1)
require.Equal(t, users[0].GetName(), "user1")

require.NoError(t, clt.DeleteUser(context.TODO(), "user1"))
require.NoError(t, clt.DeleteUser(ctx, "user1"))

users, err = clt.GetUsers(false)
require.NoError(t, err)
Expand Down Expand Up @@ -1165,7 +1166,7 @@ func TestPasswordCRUD(t *testing.T) {
err = clt.CheckPassword("user1", pass, "123456")
require.Error(t, err)

err = clt.UpsertPassword("user1", pass)
err = tt.server.Auth().UpsertPassword("user1", pass)
Copy link
Copy Markdown
Contributor

@rosstimothy rosstimothy Aug 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How come clt.UpsertPassword was replaced with tt.server.Auth().UpsertPassword instead of clt.ChangePassword?

Copy link
Copy Markdown
Contributor Author

@strideynet strideynet Aug 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this test is relying on some ✨ additional and unexpected behaviour ✨ of the UpsertPassword method on the auth server which means if you upsert the password for a user that does not exist, the user is created.

require.NoError(t, err)

dev, err := services.NewTOTPDevice("otp", otpSecret, tt.clock.Now())
Expand Down Expand Up @@ -1210,7 +1211,7 @@ func TestOTPCRUD(t *testing.T) {
otpSecret := base32.StdEncoding.EncodeToString([]byte(rawSecret))

// upsert a password and totp secret
err = clt.UpsertPassword("user1", pass)
err = tt.server.Auth().UpsertPassword("user1", pass)
require.NoError(t, err)
dev, err := services.NewTOTPDevice("otp", otpSecret, tt.clock.Now())
require.NoError(t, err)
Expand Down Expand Up @@ -1277,7 +1278,7 @@ func TestWebSessionWithoutAccessRequest(t *testing.T) {
_, err = proxy.AuthenticateWebUser(ctx, req)
require.True(t, trace.IsAccessDenied(err))

err = clt.UpsertPassword(user, pass)
err = tt.server.Auth().UpsertPassword(user, pass)
require.NoError(t, err)

// success with password set up
Expand Down Expand Up @@ -1357,7 +1358,7 @@ func TestWebSessionMultiAccessRequests(t *testing.T) {
requestableRoleName := "requestable"
user, err := CreateUserRoleAndRequestable(clt, username, requestableRoleName)
require.NoError(t, err)
err = clt.UpsertPassword(username, password)
err = tt.server.Auth().UpsertPassword(username, password)
require.NoError(t, err)

// Set search_as_roles, user can request this role only with a resource
Expand Down Expand Up @@ -1557,7 +1558,7 @@ func TestWebSessionWithApprovedAccessRequestAndSwitchback(t *testing.T) {
},
}

err = clt.UpsertPassword(user, pass)
err = tt.server.Auth().UpsertPassword(user, pass)
require.NoError(t, err)

ws, err := proxy.AuthenticateWebUser(ctx, req)
Expand Down Expand Up @@ -2470,7 +2471,7 @@ func TestLoginAttempts(t *testing.T) {
proxy, err := tt.server.NewClient(TestBuiltin(types.RoleProxy))
require.NoError(t, err)

err = clt.UpsertPassword(user, pass)
err = tt.server.Auth().UpsertPassword(user, pass)
require.NoError(t, err)

req := AuthenticateUserRequest{
Expand Down Expand Up @@ -2571,7 +2572,7 @@ func TestLoginNoLocalAuth(t *testing.T) {
require.NoError(t, err)
_, _, err = CreateUserAndRole(clt, user, []string{user})
require.NoError(t, err)
err = clt.UpsertPassword(user, pass)
err = tt.server.Auth().UpsertPassword(user, pass)
require.NoError(t, err)

// Set auth preference to disallow local auth.
Expand Down