Skip to content

Commit 7ef62ba

Browse files
easyCZroboquat
authored andcommitted
[pat] Validate and enforce scopes for PATs
1 parent 7c6766c commit 7ef62ba

File tree

3 files changed

+150
-7
lines changed

3 files changed

+150
-7
lines changed

components/gitpod-db/go/dbtest/personal_access_token.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ func NewPersonalAccessToken(t *testing.T, record db.PersonalAccessToken) db.Pers
2626
UserID: uuid.New(),
2727
Hash: "some-secure-hash",
2828
Name: "some-name",
29-
Scopes: []string{"read", "write"},
29+
Scopes: []string{"resource:default", "function:*"},
3030
ExpirationTime: now.Add(5 * time.Hour),
3131
CreatedAt: now,
3232
LastModified: now,

components/public-api-server/pkg/apiv1/tokens.go

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"errors"
1010
"fmt"
1111
"regexp"
12+
"sort"
1213
"strings"
1314

1415
connect "github.com/bufbuild/connect-go"
@@ -20,6 +21,7 @@ import (
2021
protocol "github.com/gitpod-io/gitpod/gitpod-protocol"
2122
"github.com/gitpod-io/gitpod/public-api-server/pkg/auth"
2223
"github.com/gitpod-io/gitpod/public-api-server/pkg/proxy"
24+
"github.com/google/go-cmp/cmp"
2325
"github.com/google/uuid"
2426
"google.golang.org/protobuf/types/known/timestamppb"
2527
"gorm.io/gorm"
@@ -56,9 +58,10 @@ func (s *TokensService) CreatePersonalAccessToken(ctx context.Context, req *conn
5658
return nil, connect.NewError(connect.CodeInvalidArgument, errors.New("Received invalid Expiration Time, it is a required parameter."))
5759
}
5860

59-
// TODO: Parse and validate scopes before storing
60-
// Until we do that, we store empty scopes.
61-
var scopes []string
61+
scopes, err := validateScopes(tokenReq.GetScopes())
62+
if err != nil {
63+
return nil, err
64+
}
6265

6366
conn, err := getConnection(ctx, s.connectionPool)
6467
if err != nil {
@@ -347,3 +350,27 @@ func validatePersonalAccessTokenName(name string) (string, error) {
347350

348351
return trimmed, nil
349352
}
353+
354+
const (
355+
allFunctionsScope = "function:*"
356+
defaultResourceScope = "resource:default"
357+
)
358+
359+
func validateScopes(scopes []string) ([]string, error) {
360+
// Currently we do not have support for fine grained permissions, and therefore do not support fine-grained scopes.
361+
// Therefore, for now we operate in one of the following modes:
362+
// * Token has no scopes - represented as the empty list of scopes
363+
// * Token explicitly has access to everything the user has access to, represented as ["function:*", "resource:default"]
364+
if len(scopes) == 0 {
365+
return nil, nil
366+
}
367+
368+
sort.Strings(scopes)
369+
allScopesSorted := []string{allFunctionsScope, defaultResourceScope}
370+
371+
if cmp.Equal(scopes, allScopesSorted) {
372+
return scopes, nil
373+
}
374+
375+
return nil, connect.NewError(connect.CodeInvalidArgument, fmt.Errorf("Tokens can currently only have no scopes (empty), or all scopes represented as [%s, %s]", allFunctionsScope, defaultResourceScope))
376+
}

components/public-api-server/pkg/apiv1/tokens_test.go

Lines changed: 119 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ func TestTokensService_CreatePersonalAccessTokenWithoutFeatureFlag(t *testing.T)
6767
})
6868

6969
t.Run("invalid argument when name is not specified", func(t *testing.T) {
70-
_, _, client := setupTokensService(t, withTokenFeatureDisabled)
70+
_, _, client := setupTokensService(t, withTokenFeatureEnabled)
7171

7272
_, err := client.CreatePersonalAccessToken(context.Background(), connect.NewRequest(&v1.CreatePersonalAccessTokenRequest{
7373
Token: &v1.PersonalAccessToken{},
@@ -91,7 +91,7 @@ func TestTokensService_CreatePersonalAccessTokenWithoutFeatureFlag(t *testing.T)
9191
})
9292

9393
t.Run("invalid argument when expiration time is unspecified", func(t *testing.T) {
94-
_, _, client := setupTokensService(t, withTokenFeatureDisabled)
94+
_, _, client := setupTokensService(t, withTokenFeatureEnabled)
9595

9696
_, err := client.CreatePersonalAccessToken(context.Background(), connect.NewRequest(&v1.CreatePersonalAccessTokenRequest{
9797
Token: &v1.PersonalAccessToken{
@@ -102,7 +102,7 @@ func TestTokensService_CreatePersonalAccessTokenWithoutFeatureFlag(t *testing.T)
102102
})
103103

104104
t.Run("invalid argument when expiration time is invalid", func(t *testing.T) {
105-
_, _, client := setupTokensService(t, withTokenFeatureDisabled)
105+
_, _, client := setupTokensService(t, withTokenFeatureEnabled)
106106

107107
_, err := client.CreatePersonalAccessToken(context.Background(), connect.NewRequest(&v1.CreatePersonalAccessTokenRequest{
108108
Token: &v1.PersonalAccessToken{
@@ -115,6 +115,19 @@ func TestTokensService_CreatePersonalAccessTokenWithoutFeatureFlag(t *testing.T)
115115
require.Equal(t, connect.CodeInvalidArgument, connect.CodeOf(err))
116116
})
117117

118+
t.Run("invalid argument when disallowed scopes used", func(t *testing.T) {
119+
_, _, client := setupTokensService(t, withTokenFeatureEnabled)
120+
121+
_, err := client.CreatePersonalAccessToken(context.Background(), connect.NewRequest(&v1.CreatePersonalAccessTokenRequest{
122+
Token: &v1.PersonalAccessToken{
123+
Name: "my-token",
124+
ExpirationTime: timestamppb.Now(),
125+
Scopes: []string{"random:scope"},
126+
},
127+
}))
128+
require.Equal(t, connect.CodeInvalidArgument, connect.CodeOf(err))
129+
})
130+
118131
t.Run("crates personal access token", func(t *testing.T) {
119132
serverMock, dbConn, client := setupTokensService(t, withTokenFeatureEnabled)
120133

@@ -149,6 +162,53 @@ func TestTokensService_CreatePersonalAccessTokenWithoutFeatureFlag(t *testing.T)
149162
require.NoError(t, err)
150163
require.Equal(t, user.ID, storedInDB.UserID.String())
151164
})
165+
166+
t.Run("crates personal access token with no scopes when none provided", func(t *testing.T) {
167+
serverMock, dbConn, client := setupTokensService(t, withTokenFeatureEnabled)
168+
169+
serverMock.EXPECT().GetLoggedInUser(gomock.Any()).Return(user, nil)
170+
171+
token := &v1.PersonalAccessToken{
172+
Name: "my-token",
173+
ExpirationTime: timestamppb.Now(),
174+
}
175+
176+
response, err := client.CreatePersonalAccessToken(context.Background(), connect.NewRequest(&v1.CreatePersonalAccessTokenRequest{
177+
Token: token,
178+
}))
179+
require.NoError(t, err)
180+
181+
created := response.Msg.GetToken()
182+
t.Cleanup(func() {
183+
require.NoError(t, dbConn.Where("id = ?", created.GetId()).Delete(&db.PersonalAccessToken{}).Error)
184+
})
185+
186+
require.Len(t, created.GetScopes(), 0, "must have no scopes, none were provided in the request")
187+
})
188+
189+
t.Run("crates personal access token with full access when correct scopes provided", func(t *testing.T) {
190+
serverMock, dbConn, client := setupTokensService(t, withTokenFeatureEnabled)
191+
192+
serverMock.EXPECT().GetLoggedInUser(gomock.Any()).Return(user, nil)
193+
194+
token := &v1.PersonalAccessToken{
195+
Name: "my-token",
196+
ExpirationTime: timestamppb.Now(),
197+
Scopes: []string{"resource:default", "function:*"},
198+
}
199+
200+
response, err := client.CreatePersonalAccessToken(context.Background(), connect.NewRequest(&v1.CreatePersonalAccessTokenRequest{
201+
Token: token,
202+
}))
203+
require.NoError(t, err)
204+
205+
created := response.Msg.GetToken()
206+
t.Cleanup(func() {
207+
require.NoError(t, dbConn.Where("id = ?", created.GetId()).Delete(&db.PersonalAccessToken{}).Error)
208+
})
209+
210+
require.Equal(t, []string{allFunctionsScope, defaultResourceScope}, created.GetScopes())
211+
})
152212
}
153213

154214
func TestTokensService_GetPersonalAccessToken(t *testing.T) {
@@ -617,6 +677,62 @@ func TestTokensService_Workflow(t *testing.T) {
617677
require.NoError(t, err)
618678
}
619679

680+
func TestValidateScopes(t *testing.T) {
681+
for _, s := range []struct {
682+
Name string
683+
RequestedScopes []string
684+
Error bool
685+
}{
686+
{
687+
Name: "no scopes are permitted",
688+
RequestedScopes: nil,
689+
},
690+
{
691+
Name: "empty scopes are permitted",
692+
RequestedScopes: []string{},
693+
},
694+
{
695+
Name: "all scopes are permitted",
696+
RequestedScopes: []string{"function:*", "resource:default"},
697+
},
698+
{
699+
Name: "all scopes (unsorted) are permitted",
700+
RequestedScopes: []string{"resource:default", "function:*"},
701+
},
702+
{
703+
Name: "only all function scope is not permitted",
704+
RequestedScopes: []string{"function:*"},
705+
Error: true,
706+
},
707+
{
708+
Name: "only all default resource scope is not permitted",
709+
RequestedScopes: []string{"resource:default"},
710+
Error: true,
711+
},
712+
{
713+
Name: "unknown scope is rejected",
714+
RequestedScopes: []string{"unknown"},
715+
Error: true,
716+
},
717+
{
718+
Name: "unknown scope, with all scopes, is rejected",
719+
RequestedScopes: []string{"unknown", "function:*", "resource:default"},
720+
Error: true,
721+
},
722+
} {
723+
t.Run(s.Name, func(t *testing.T) {
724+
_, err := validateScopes(s.RequestedScopes)
725+
726+
if s.Error {
727+
require.Error(t, err)
728+
} else {
729+
require.NoError(t, err)
730+
}
731+
})
732+
733+
}
734+
}
735+
620736
func setupTokensService(t *testing.T, expClient experiments.Client) (*protocol.MockAPIInterface, *gorm.DB, v1connect.TokensServiceClient) {
621737
t.Helper()
622738

0 commit comments

Comments
 (0)