Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Commit dd32e5f

Browse files
authored
clearer access tokens allow check on dotcom (#63368)
Previously, the code would prevent us from using the AccessTokensAdmin config setting on dotcom entirely, instead of just restricting it when site admins create an access token for a different user, which was the intent. ## Test plan CI
1 parent 13cba57 commit dd32e5f

File tree

2 files changed

+17
-10
lines changed

2 files changed

+17
-10
lines changed

cmd/frontend/graphqlbackend/access_tokens.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,6 @@ type createAccessTokenInput struct {
2929
}
3030

3131
func (r *schemaResolver) CreateAccessToken(ctx context.Context, args *createAccessTokenInput) (*createAccessTokenResult, error) {
32-
// 🚨 SECURITY: Creating access tokens for any user by site admins is not
33-
// allowed on Sourcegraph.com. This check is mostly the defense for a
34-
// misconfiguration of the site configuration.
35-
if dotcom.SourcegraphDotComMode() && conf.AccessTokensAllow() == conf.AccessTokensAdmin {
36-
return nil, errors.Errorf("access token configuration value %q is disabled on Sourcegraph.com", conf.AccessTokensAllow())
37-
}
38-
3932
userID, err := UnmarshalUserID(args.User)
4033
if err != nil {
4134
return nil, err
@@ -56,6 +49,16 @@ func (r *schemaResolver) CreateAccessToken(ctx context.Context, args *createAcce
5649
if err := auth.CheckCurrentUserIsSiteAdmin(ctx, r.db); err != nil {
5750
return nil, errors.New("Access token creation has been restricted to admin users. Contact an admin user to create a new access token.")
5851
}
52+
53+
// 🚨 SECURITY: Creating access tokens for other users by site admins is not allowed on
54+
// Sourcegraph.com. This check is mostly the defense for a misconfiguration of the site
55+
// configuration.
56+
if dotcom.SourcegraphDotComMode() {
57+
if err := auth.CheckSameUser(ctx, userID); err != nil {
58+
return nil, errors.New("access token creation for other users is disabled on Sourcegraph.com")
59+
}
60+
}
61+
5962
case conf.AccessTokensNone:
6063
default:
6164
return nil, errors.New("Access token creation is disabled. Contact an admin user to enable.")

cmd/frontend/graphqlbackend/access_tokens_test.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -375,8 +375,12 @@ func TestMutation_CreateAccessToken(t *testing.T) {
375375
assert.Equal(t, want, got)
376376
})
377377

378-
t.Run("disable create access token for any user on Sourcegraph.com", func(t *testing.T) {
378+
t.Run("prevent create access token for other user on Sourcegraph.com", func(t *testing.T) {
379+
users := dbmocks.NewMockUserStore()
380+
users.GetByCurrentAuthUserFunc.SetDefaultReturn(&types.User{ID: 1, SiteAdmin: true}, nil)
381+
379382
db := dbmocks.NewMockDB()
383+
db.UsersFunc.SetDefaultReturn(users)
380384

381385
conf.Get().AuthAccessTokens = &schema.AuthAccessTokens{Allow: string(conf.AccessTokensAdmin)}
382386
defer func() { conf.Get().AuthAccessTokens = nil }()
@@ -386,13 +390,13 @@ func TestMutation_CreateAccessToken(t *testing.T) {
386390
ctx := actor.WithActor(context.Background(), &actor.Actor{UID: 1})
387391
_, err := newSchemaResolver(db, gitserver.NewTestClient(t)).CreateAccessToken(ctx,
388392
&createAccessTokenInput{
389-
User: MarshalUserID(1),
393+
User: MarshalUserID(2),
390394
Scopes: []string{authz.ScopeUserAll},
391395
DurationSeconds: &defaultTokenDuration,
392396
},
393397
)
394398
got := fmt.Sprintf("%v", err)
395-
want := `access token configuration value "site-admin-create" is disabled on Sourcegraph.com`
399+
want := `access token creation for other users is disabled on Sourcegraph.com`
396400
assert.Equal(t, want, got)
397401
})
398402
}

0 commit comments

Comments
 (0)