mirror of
https://github.com/hanzoai/team.git
synced 2026-09-30 01:57:35 +00:00
bbc9b152e6
* MM-68417, MM-68420: API support for PAT expiry and admin policy settings
POST /users/{id}/tokens now accepts a client-supplied expires_at
(previously stripped per the TODO in api4/user.go), and the create app
method enforces two new ServiceSettings:
- EnforcePersonalAccessTokenExpiry (bool, default false): when on,
rejects creates with expires_at == 0
- MaximumPersonalAccessTokenLifetimeDays (int, default 0 = unlimited):
caps how far in the future expires_at may be
Rejections return distinct app error ids so clients can disambiguate:
expires_at_required, expires_at_in_past, expires_at_too_far.
GET /users/{id}/tokens already serializes expires_at via the model's
JSON tag added in MM-68419; clients derive token status (active /
expired / inactive) from is_active + expires_at without a separate
server-side field, keeping the response shape minimal.
The Client4 helper CreateUserAccessToken gained an optional variadic
expiresAt parameter (and the mmctl Client interface + mock match)
rather than introducing a parallel WithExpiry method.
Refs: MM-68417, MM-68420
* MM-68417: exempt bot accounts from PAT expiry enforcement
Mirrors the existing EnableUserAccessTokens bypass at session.go:453,
where bot tokens are allowed even when human PATs are disabled. Bots
are programmatic clients that typically need long-lived credentials,
and integrations that provision them would otherwise break the moment
an admin enables EnforcePersonalAccessTokenExpiry — turning a settings
toggle into a footgun. The expiry policy now applies only to human
users; bots can still be given a future expires_at by callers that
want it, but the server won't require one.
Locked in by a new TestCreateUserAccessToken/bot_tokens_are_exempt
subtest that enables enforcement plus a 30-day cap, creates a bot,
and asserts a non-expiring token is accepted.
Refs: MM-68417
* MM-68420: bound MaximumPersonalAccessTokenLifetimeDays in isValid
Negative values silently meant "unlimited" (the runtime check is `> 0`),
and very large values overflow int64 when computing
`now + days*86_400_000` at token-creation time, producing a wrap-around
that either rejects all reasonable expiries or accepts past timestamps
as valid.
Bound the setting in ServiceSettings.isValid to [0, MaxPersonalAccess
TokenLifetimeDays] where the cap is 36500 (100 years) — past any
realistic operational use and well clear of int64 overflow. Surfaces
as a config validation error rather than a silently-broken runtime
check. New TestServiceSettingsIsValid cases lock in zero, negative,
upper-bound, and above-upper-bound behavior.
Refs: MM-68420
* MM-68417: reject multiple expiresAt values in Client4 helper
CodeRabbit caught that the variadic CreateUserAccessToken silently used
expiresAt[0] when callers passed more than one value, masking a
mis-call instead of failing fast. Return an error in that case so the
misuse is visible at the call site rather than producing a token with
the wrong (or right-but-coincidental) expiry.
Refs: MM-68417
* MM-68417: make expiresAt a required parameter on Client4.CreateUserAccessToken
Drop the variadic in favor of a regular int64 parameter. The variadic
form silently dropped extra values and made a misuse undetectable at
the call site (per CodeRabbit review on PR 36706); the previous fix
guarded against >1 values at runtime, but a required parameter is
strictly better — the compiler now refuses the misuse and every caller
is forced to make a deliberate decision about expiry. Existing callers
that want the old behavior pass 0 (== never expires).
Refs: MM-68417
* MM-68417: add --expires-in flag to mmctl user token generate
Adds an --expires-in duration flag so operators can create expiring
PATs via the CLI. Accepts the standard Go duration syntax plus a
trailing 'd' for days (the common case for token lifetimes), e.g.
--expires-in 90d, --expires-in 12h, --expires-in 1h30m. Empty (the
default) means no expiry — matching prior behavior. Without this
flag the command was unusable once an admin enables
EnforcePersonalAccessTokenExpiry, since every create would fail with
app.user_access_token.expires_at_required.app_error.
Flag parsing now happens before the user-lookup API call so the
command fails fast on invalid input. Regenerated mmctl docs reflect
the new flag.
Refs: MM-68417
* MM-68417: cap --expires-in day count to prevent time.Duration overflow
parseExpiresIn returns time.Duration(days) * 24 * time.Hour, which is
int64 nanoseconds and overflows past ~106751 days (CodeRabbit caught
this). Cap at model.MaxPersonalAccessTokenLifetimeDays (36500) so the
CLI rejects values the server would reject anyway, well below the
int64-overflow point. Adds two test cases (at-cap and beyond-cap) to
TestParseExpiresIn.
Refs: MM-68417
* MM-68420: collapse PAT expiry settings into a single max-lifetime setting
Remove ServiceSettings.EnforcePersonalAccessTokenExpiry and fold the
policy onto MaximumPersonalAccessTokenLifetimeDays: 0 means no policy
(never-expiring tokens allowed, no cap), while a value > 0 requires every
new token to expire within that many days. The two-setting design let an
admin set a maximum but leave enforcement off, silently allowing
never-expiring tokens to sidestep the cap; the only combination the
boolean added (require expiry, no upper bound) has little practical
value. The removed field was introduced on this branch and never
released, so this is not a breaking change.
---------
Co-authored-by: Hanzo Dev <[email protected]>
363 lines
12 KiB
Go
363 lines
12 KiB
Go
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
|
|
// See LICENSE.txt for license information.
|
|
|
|
package commands
|
|
|
|
import (
|
|
"context"
|
|
"fmt"
|
|
"net/http"
|
|
"testing"
|
|
"time"
|
|
|
|
gomock "github.com/golang/mock/gomock"
|
|
"github.com/mattermost/mattermost/server/public/model"
|
|
"github.com/pkg/errors"
|
|
"github.com/stretchr/testify/require"
|
|
|
|
"github.com/mattermost/mattermost/server/v8/cmd/mmctl/printer"
|
|
|
|
"github.com/spf13/cobra"
|
|
)
|
|
|
|
func TestParseExpiresIn(t *testing.T) {
|
|
for name, tc := range map[string]struct {
|
|
input string
|
|
want time.Duration
|
|
wantErr bool
|
|
}{
|
|
"days": {input: "30d", want: 30 * 24 * time.Hour},
|
|
"single day": {input: "1d", want: 24 * time.Hour},
|
|
"hours": {input: "12h", want: 12 * time.Hour},
|
|
"compound stdlib": {input: "1h30m", want: 90 * time.Minute},
|
|
"minutes": {input: "45m", want: 45 * time.Minute},
|
|
"non-numeric days": {input: "xd", wantErr: true},
|
|
"non-numeric stdlib": {input: "abc", wantErr: true},
|
|
"empty": {input: "", wantErr: true},
|
|
"days beyond cap": {input: "36501d", wantErr: true},
|
|
"days at cap accepted": {input: "36500d", want: 36500 * 24 * time.Hour},
|
|
} {
|
|
t.Run(name, func(t *testing.T) {
|
|
got, err := parseExpiresIn(tc.input)
|
|
if tc.wantErr {
|
|
require.Error(t, err)
|
|
return
|
|
}
|
|
require.NoError(t, err)
|
|
require.Equal(t, tc.want, got)
|
|
})
|
|
}
|
|
}
|
|
|
|
func (s *MmctlUnitTestSuite) TestGenerateTokenForAUserCmd() {
|
|
s.Run("Should generate a token for a user", func() {
|
|
printer.Clean()
|
|
|
|
userArg := "userId1"
|
|
mockUser := model.User{Id: "userId1", Email: "[email protected]", Username: "user1"}
|
|
mockToken := model.UserAccessToken{Token: "token-id", Description: "token-desc"}
|
|
|
|
s.client.
|
|
EXPECT().
|
|
GetUserByUsername(context.TODO(), userArg, "").
|
|
Return(nil, &model.Response{}, errors.New("no user found with the given username")).
|
|
Times(1)
|
|
|
|
s.client.
|
|
EXPECT().
|
|
GetUser(context.TODO(), userArg, "").
|
|
Return(&mockUser, &model.Response{}, nil).
|
|
Times(1)
|
|
|
|
s.client.
|
|
EXPECT().
|
|
CreateUserAccessToken(context.TODO(), mockUser.Id, mockToken.Description, int64(0)).
|
|
Return(&mockToken, &model.Response{}, nil).
|
|
Times(1)
|
|
|
|
err := generateTokenForAUserCmdF(s.client, &cobra.Command{}, []string{mockUser.Id, mockToken.Description})
|
|
s.Require().Nil(err)
|
|
s.Require().Len(printer.GetLines(), 1)
|
|
s.Require().Equal(&mockToken, printer.GetLines()[0])
|
|
})
|
|
|
|
s.Run("Should fail on an invalid username", func() {
|
|
printer.Clean()
|
|
|
|
userArg := "some-text"
|
|
s.client.
|
|
EXPECT().
|
|
GetUserByUsername(context.TODO(), userArg, "").
|
|
Return(nil, &model.Response{}, errors.New("no user found with the given username")).
|
|
Times(1)
|
|
|
|
s.client.
|
|
EXPECT().
|
|
GetUser(context.TODO(), userArg, "").
|
|
Return(nil, &model.Response{}, errors.New("no user found with the given ID")).
|
|
Times(1)
|
|
|
|
err := generateTokenForAUserCmdF(s.client, &cobra.Command{}, []string{userArg, "description"})
|
|
s.Require().NotNil(err)
|
|
s.Require().Contains(err.Error(), fmt.Sprintf("could not retrieve user information of %q", userArg))
|
|
})
|
|
|
|
s.Run("Should fail if can't create tokens for a valid user", func() {
|
|
printer.Clean()
|
|
|
|
userArg := "user1"
|
|
mockUser := model.User{Id: "userId1", Email: "[email protected]", Username: "user1"}
|
|
|
|
s.client.
|
|
EXPECT().
|
|
GetUserByUsername(context.TODO(), userArg, "").
|
|
Return(&mockUser, &model.Response{}, nil).
|
|
Times(1)
|
|
|
|
s.client.
|
|
EXPECT().
|
|
CreateUserAccessToken(context.TODO(), mockUser.Id, "description", int64(0)).
|
|
Return(nil, &model.Response{}, errors.New("error-message")).
|
|
Times(1)
|
|
|
|
err := generateTokenForAUserCmdF(s.client, &cobra.Command{}, []string{"user1", "description"})
|
|
s.Require().NotNil(err)
|
|
s.Require().Contains(err.Error(), fmt.Sprintf("could not create token for %q:", "user1"))
|
|
})
|
|
|
|
s.Run("Should pass a positive expires_at when --expires-in is set", func() {
|
|
printer.Clean()
|
|
|
|
userArg := "userId1"
|
|
mockUser := model.User{Id: "userId1", Email: "[email protected]", Username: "user1"}
|
|
mockToken := model.UserAccessToken{Token: "token-id", Description: "ci-token"}
|
|
|
|
s.client.
|
|
EXPECT().
|
|
GetUserByUsername(context.TODO(), userArg, "").
|
|
Return(nil, &model.Response{}, errors.New("no user found with the given username")).
|
|
Times(1)
|
|
s.client.
|
|
EXPECT().
|
|
GetUser(context.TODO(), userArg, "").
|
|
Return(&mockUser, &model.Response{}, nil).
|
|
Times(1)
|
|
|
|
now := time.Now()
|
|
s.client.
|
|
EXPECT().
|
|
CreateUserAccessToken(context.TODO(), mockUser.Id, mockToken.Description, gomock.AssignableToTypeOf(int64(0))).
|
|
DoAndReturn(func(_ context.Context, _, _ string, expiresAt int64) (*model.UserAccessToken, *model.Response, error) {
|
|
// 90 days = 7_776_000_000 ms; allow generous slack for clock between Now() calls.
|
|
expected := now.Add(90 * 24 * time.Hour).UnixMilli()
|
|
s.Require().InDelta(expected, expiresAt, float64(time.Minute.Milliseconds()))
|
|
return &mockToken, &model.Response{}, nil
|
|
}).
|
|
Times(1)
|
|
|
|
cmd := &cobra.Command{}
|
|
cmd.Flags().String("expires-in", "", "")
|
|
s.Require().NoError(cmd.Flags().Set("expires-in", "90d"))
|
|
|
|
err := generateTokenForAUserCmdF(s.client, cmd, []string{mockUser.Id, mockToken.Description})
|
|
s.Require().NoError(err)
|
|
})
|
|
|
|
s.Run("Should reject invalid --expires-in", func() {
|
|
printer.Clean()
|
|
|
|
cmd := &cobra.Command{}
|
|
cmd.Flags().String("expires-in", "", "")
|
|
s.Require().NoError(cmd.Flags().Set("expires-in", "not-a-duration"))
|
|
|
|
err := generateTokenForAUserCmdF(s.client, cmd, []string{"userId1", "desc"})
|
|
s.Require().Error(err)
|
|
s.Require().Contains(err.Error(), "invalid --expires-in")
|
|
})
|
|
|
|
s.Run("Should reject non-positive --expires-in", func() {
|
|
printer.Clean()
|
|
|
|
cmd := &cobra.Command{}
|
|
cmd.Flags().String("expires-in", "", "")
|
|
s.Require().NoError(cmd.Flags().Set("expires-in", "-5h"))
|
|
|
|
err := generateTokenForAUserCmdF(s.client, cmd, []string{"userId1", "desc"})
|
|
s.Require().Error(err)
|
|
s.Require().Contains(err.Error(), "must be positive")
|
|
})
|
|
}
|
|
|
|
func (s *MmctlUnitTestSuite) TestListTokensOfAUserCmdF() {
|
|
s.Run("Should list access tokens of a user", func() {
|
|
printer.Clean()
|
|
|
|
command := cobra.Command{}
|
|
command.Flags().Int("page", 0, "")
|
|
command.Flags().Int("per-page", 2, "")
|
|
command.Flags().Bool("all", true, "")
|
|
command.Flags().Bool("active", false, "")
|
|
command.Flags().Bool("inactive", false, "")
|
|
|
|
mockUser := model.User{Id: "userId1", Email: "[email protected]", Username: "user1"}
|
|
mockToken1 := model.UserAccessToken{IsActive: true, Id: "token-1-id", Description: "token-1-desc"}
|
|
mockToken2 := model.UserAccessToken{IsActive: false, Id: "token-2-id", Description: "token-2-desc"}
|
|
|
|
s.client.
|
|
EXPECT().
|
|
GetUserByUsername(context.TODO(), mockUser.Id, "").
|
|
Return(nil, &model.Response{}, errors.New("no user found with the given username")).
|
|
Times(1)
|
|
|
|
s.client.
|
|
EXPECT().
|
|
GetUser(context.TODO(), mockUser.Id, "").
|
|
Return(&mockUser, &model.Response{}, nil).
|
|
Times(1)
|
|
|
|
s.client.
|
|
EXPECT().
|
|
GetUserAccessTokensForUser(context.TODO(), mockUser.Id, 0, 9999).
|
|
Return(
|
|
[]*model.UserAccessToken{&mockToken1, &mockToken2},
|
|
&model.Response{}, nil,
|
|
).Times(1)
|
|
|
|
err := listTokensOfAUserCmdF(s.client, &command, []string{mockUser.Id})
|
|
s.Require().Nil(err)
|
|
s.Require().Len(printer.GetLines(), 2)
|
|
s.Require().Equal(&mockToken1, printer.GetLines()[0])
|
|
s.Require().Equal(&mockToken2, printer.GetLines()[1])
|
|
})
|
|
|
|
s.Run("Should list only active user access tokens of a user", func() {
|
|
printer.Clean()
|
|
|
|
command := cobra.Command{}
|
|
command.Flags().Int("page", 0, "")
|
|
command.Flags().Int("per-page", 2, "")
|
|
command.Flags().Bool("all", false, "")
|
|
command.Flags().Bool("active", true, "")
|
|
command.Flags().Bool("inactive", false, "")
|
|
|
|
mockUser := model.User{Id: "userId1", Email: "[email protected]", Username: "user1"}
|
|
mockToken1 := model.UserAccessToken{IsActive: true, Id: "token-1-id", Description: "token-1-desc"}
|
|
mockToken2 := model.UserAccessToken{IsActive: false, Id: "token-2-id", Description: "token-2-desc"}
|
|
|
|
s.client.
|
|
EXPECT().
|
|
GetUserByEmail(context.TODO(), mockUser.Email, "").
|
|
Return(&mockUser, &model.Response{}, errors.New("no user found with the given email")).
|
|
Times(1)
|
|
|
|
s.client.
|
|
EXPECT().
|
|
GetUserAccessTokensForUser(context.TODO(), mockUser.Id, 0, 2).
|
|
Return(
|
|
[]*model.UserAccessToken{&mockToken1, &mockToken2},
|
|
&model.Response{}, nil,
|
|
).Times(1)
|
|
|
|
err := listTokensOfAUserCmdF(s.client, &command, []string{mockUser.Email})
|
|
s.Require().Nil(err)
|
|
s.Require().Len(printer.GetLines(), 1)
|
|
s.Require().Equal(&mockToken1, printer.GetLines()[0])
|
|
})
|
|
|
|
s.Run("Should err on a absent user", func() {
|
|
printer.Clean()
|
|
|
|
userArg := "test-user"
|
|
command := cobra.Command{}
|
|
command.Flags().Int("page", 0, "")
|
|
command.Flags().Int("per-page", 2, "")
|
|
command.Flags().Bool("all", false, "")
|
|
command.Flags().Bool("active", false, "")
|
|
command.Flags().Bool("inactive", false, "")
|
|
|
|
s.client.
|
|
EXPECT().
|
|
GetUserByUsername(context.TODO(), userArg, "").
|
|
Return(nil, &model.Response{}, errors.New("no user found with the given username")).
|
|
Times(1)
|
|
|
|
s.client.
|
|
EXPECT().
|
|
GetUser(context.TODO(), userArg, "").
|
|
Return(nil, &model.Response{}, errors.New("no user found with the given user ID")).
|
|
Times(1)
|
|
|
|
err := listTokensOfAUserCmdF(s.client, &command, []string{userArg})
|
|
s.Require().NotNil(err)
|
|
s.Require().Contains(err.Error(), fmt.Sprintf("could not retrieve user information of %q", userArg))
|
|
})
|
|
|
|
s.Run("Should error if there are no user access tokens for a valid user", func() {
|
|
printer.Clean()
|
|
|
|
command := cobra.Command{}
|
|
command.Flags().Int("page", 0, "")
|
|
command.Flags().Int("per-page", 2, "")
|
|
command.Flags().Bool("all", false, "")
|
|
command.Flags().Bool("active", true, "")
|
|
command.Flags().Bool("inactive", false, "")
|
|
|
|
mockUser := model.User{Id: "userId1", Email: "[email protected]", Username: "user1"}
|
|
|
|
s.client.
|
|
EXPECT().
|
|
GetUserByEmail(context.TODO(), mockUser.Email, "").
|
|
Return(&mockUser, &model.Response{}, errors.New("no user found with the given email")).
|
|
Times(1)
|
|
|
|
s.client.
|
|
EXPECT().
|
|
GetUserAccessTokensForUser(context.TODO(), mockUser.Id, 0, 2).
|
|
Return(
|
|
[]*model.UserAccessToken{},
|
|
&model.Response{}, nil,
|
|
).Times(1)
|
|
|
|
err := listTokensOfAUserCmdF(s.client, &command, []string{mockUser.Email})
|
|
s.Require().NotNil(err)
|
|
s.Require().Equal(err.Error(), fmt.Sprintf("there are no tokens for the %q", mockUser.Email))
|
|
})
|
|
}
|
|
|
|
func (s *MmctlUnitTestSuite) TestRevokeTokenForAUserCmdF() {
|
|
s.Run("Should revoke user access tokens", func() {
|
|
printer.Clean()
|
|
|
|
mockToken1 := model.UserAccessToken{Id: "123456"}
|
|
mockToken2 := model.UserAccessToken{Id: "234567"}
|
|
|
|
s.client.
|
|
EXPECT().
|
|
RevokeUserAccessToken(context.TODO(), mockToken1.Id).
|
|
Return(&model.Response{StatusCode: http.StatusOK}, nil).
|
|
Times(1)
|
|
|
|
s.client.
|
|
EXPECT().
|
|
RevokeUserAccessToken(context.TODO(), mockToken2.Id).
|
|
Return(&model.Response{StatusCode: http.StatusOK}, nil).
|
|
Times(1)
|
|
|
|
err := revokeTokenForAUserCmdF(s.client, &cobra.Command{}, []string{mockToken1.Id, mockToken2.Id})
|
|
s.Require().Nil(err)
|
|
s.Require().Len(printer.GetLines(), 0)
|
|
})
|
|
|
|
s.Run("Should fail if can't revoke user access token", func() {
|
|
s.client.
|
|
EXPECT().
|
|
RevokeUserAccessToken(context.TODO(), "token-id").
|
|
Return(&model.Response{StatusCode: http.StatusBadRequest}, errors.New("some-error")).
|
|
Times(1)
|
|
|
|
err := revokeTokenForAUserCmdF(s.client, &cobra.Command{}, []string{"token-id"})
|
|
s.Require().NotNil(err)
|
|
s.Require().Contains(err.Error(), fmt.Sprintf("could not revoke token %q", "token-id"))
|
|
})
|
|
}
|