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

Commit d29b7b2

Browse files
committed
[feat] Add access token scopes authorization based on graphql directives
This only works on token based authorization, when using non-internal tokens. Existing functionality was kept backwards compatible on this POC, so if you have a token with "user:all" or "site-admin:sudo" everything works like before. Similarly session based auth still works the same. Added a directive `@authz(scopes: ["some_scope"])` which controls scopes that are required when calling the graphql API with a token based authentication. When this directive is present on a field, query, mutation or type, it is required that the token has scopes that are listed. The beauty of this approach is, that we can add different scopes to different queries/fields/mutations and single source of truth is our graphql schema. - unit tests - failing authorization on queries/mutations/fields without the directive - adding scopes to all needed graphql entities - UI changes to create tokens with more scopes - making backwards incompatible changes - authorizing internal API access Tested locally. To test locally, you need to modify `go.mod` to point to your own local fork of graph-gophers/graphql-go#446 It is also needed to apply the patch suggested in this comment: https://github.com/graph-gophers/graphql-go/pull/446/files#r914374506 To create a token with different scopes, create a normal token as you would usually by going to Settings -> Access tokens. You need to modify the scopes directly in the database (yikes!). Search the `schema.graphql` file for `@authz` directive scope definitions that are required with these changes. You then need to use the token directly with curl or similar. When using the token without proper scopes, you should see graphql errors instead of data.
1 parent 7836dc2 commit d29b7b2

File tree

7 files changed

+98
-50
lines changed

7 files changed

+98
-50
lines changed

cmd/frontend/graphqlbackend/graphqlbackend.go

+31
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/graph-gophers/graphql-go/introspection"
1616
"github.com/graph-gophers/graphql-go/relay"
1717
"github.com/graph-gophers/graphql-go/trace"
18+
gqltypes "github.com/graph-gophers/graphql-go/types"
1819
"github.com/inconshreveable/log15"
1920
"github.com/prometheus/client_golang/prometheus"
2021
"github.com/prometheus/client_golang/prometheus/promauto"
@@ -490,6 +491,9 @@ func NewSchema(
490491
resolver,
491492
graphql.Tracer(&prometheusTracer{}),
492493
graphql.UseStringDescriptions(),
494+
graphql.DirectiveVisitors(map[string]gqltypes.DirectiveVisitor{
495+
"authz": &authzDirectiveVisitor{},
496+
}),
493497
)
494498
}
495499

@@ -848,3 +852,30 @@ func (r *schemaResolver) CodeHostSyncDue(ctx context.Context, args *struct {
848852
}
849853
return r.db.ExternalServices().SyncDue(ctx, ids, time.Duration(args.Seconds)*time.Second)
850854
}
855+
856+
type authzDirectiveVisitor struct{}
857+
858+
func (v *authzDirectiveVisitor) Before(ctx context.Context, directive *gqltypes.Directive, input interface{}) error {
859+
if scopesAttr, ok := directive.Arguments.Get("scopes"); ok {
860+
a := actor.FromContext(ctx)
861+
// only care about token based auth and non-internal tokens for now
862+
if a.FromToken && !a.Internal {
863+
requiredScopes := scopesAttr.Deserialize(nil).([]interface{})
864+
if len(requiredScopes) < 1 {
865+
return errors.Errorf("Authorization required, but no scopes are given in graphql schema")
866+
}
867+
// for now all scopes are required, but this can be changed in the future
868+
// to be more flexible
869+
for _, scope := range requiredScopes {
870+
if ok := a.Scopes[scope.(string)]; !ok {
871+
return errors.Errorf("Missing token scope %s", scope)
872+
}
873+
}
874+
}
875+
}
876+
return nil
877+
}
878+
879+
func (v *authzDirectiveVisitor) After(ctx context.Context, directive *gqltypes.Directive, output interface{}) (interface{}, error) {
880+
return output, nil
881+
}

cmd/frontend/graphqlbackend/schema.graphql

+8-5
Original file line numberDiff line numberDiff line change
@@ -1141,6 +1141,8 @@ input SurveySubmissionInput {
11411141
better: String
11421142
}
11431143

1144+
directive @authz(scopes: [String]!) on QUERY | FIELD_DEFINITION | OBJECT
1145+
11441146
"""
11451147
Input for a happiness feedback submission.
11461148
"""
@@ -1217,7 +1219,7 @@ type Query {
12171219
): RepositoryRedirect
12181220
"""
12191221
Lists external services under given namespace.
1220-
If no namespace is given, it returns all external services.
1222+
If no namespace is given, it returns all external services
12211223
"""
12221224
externalServices(
12231225
"""
@@ -1233,7 +1235,8 @@ type Query {
12331235
Opaque pagination cursor.
12341236
"""
12351237
after: String
1236-
): ExternalServiceConnection!
1238+
): ExternalServiceConnection! @authz(scopes: ["external_services:read"])
1239+
12371240
"""
12381241
List all repositories.
12391242
"""
@@ -1501,7 +1504,7 @@ type Query {
15011504
"""
15021505
Retrieve the list of defined feature flags
15031506
"""
1504-
featureFlags: [FeatureFlag!]!
1507+
featureFlags: [FeatureFlag!]! @authz(scopes: ["feature_flags:read"])
15051508

15061509
"""
15071510
Retrieve a feature flag
@@ -2330,7 +2333,7 @@ type Highlight {
23302333
"""
23312334
A list of external services.
23322335
"""
2333-
type ExternalServiceConnection {
2336+
type ExternalServiceConnection @authz(scopes: ["external_services:read"]) {
23342337
"""
23352338
A list of external services.
23362339
"""
@@ -2391,7 +2394,7 @@ type ExternalService implements Node {
23912394
"""
23922395
The JSON configuration of the external service.
23932396
"""
2394-
config: JSONCString!
2397+
config: JSONCString! @authz(scopes: ["external_services:admin"])
23952398

23962399
"""
23972400
When the external service was created.

cmd/frontend/internal/httpapi/auth.go

+21-10
Original file line numberDiff line numberDiff line change
@@ -59,18 +59,29 @@ func AccessTokenAuthMiddleware(db database.DB, next http.Handler) http.Handler {
5959
return
6060
}
6161

62+
accessToken, err := db.AccessTokens().Lookup(r.Context(), token)
63+
// convert scopes to a map for faster lookup
64+
scopes := make(map[string]bool)
65+
for _, scope := range accessToken.Scopes {
66+
scopes[scope] = true
67+
}
6268
// Validate access token.
6369
//
6470
// 🚨 SECURITY: It's important we check for the correct scopes to know what this token
6571
// is allowed to do.
66-
var requiredScope string
72+
var hasRequiredScope bool
6773
if sudoUser == "" {
68-
requiredScope = authz.ScopeUserAll
74+
ok := scopes[authz.ScopeUserAll]
75+
// we require either user:all scope or any other scope to be present
76+
hasRequiredScope = ok || len(accessToken.Scopes) > 0
6977
} else {
70-
requiredScope = authz.ScopeSiteAdminSudo
78+
hasRequiredScope = scopes[authz.ScopeSiteAdminSudo]
7179
}
72-
subjectUserID, err := db.AccessTokens().Lookup(r.Context(), token, requiredScope)
73-
if err != nil {
80+
if !hasRequiredScope {
81+
err = database.ErrAccessTokenNotFound
82+
}
83+
84+
if err != nil || !hasRequiredScope {
7485
if err == database.ErrAccessTokenNotFound || errors.HasType(err, database.InvalidTokenError{}) {
7586
log15.Error("AccessTokenAuthMiddleware.invalidAccessToken", "token", token, "error", err)
7687
http.Error(w, "Invalid access token.", http.StatusUnauthorized)
@@ -85,12 +96,12 @@ func AccessTokenAuthMiddleware(db database.DB, next http.Handler) http.Handler {
8596
// Determine the actor's user ID.
8697
var actorUserID int32
8798
if sudoUser == "" {
88-
actorUserID = subjectUserID
99+
actorUserID = accessToken.SubjectUserID
89100
} else {
90101
// 🚨 SECURITY: Confirm that the sudo token's subject is still a site admin, to
91102
// prevent users from retaining site admin privileges after being demoted.
92-
if err := auth.CheckUserIsSiteAdmin(r.Context(), db, subjectUserID); err != nil {
93-
log15.Error("Sudo access token's subject is not a site admin.", "subjectUserID", subjectUserID, "err", err)
103+
if err := auth.CheckUserIsSiteAdmin(r.Context(), db, accessToken.SubjectUserID); err != nil {
104+
log15.Error("Sudo access token's subject is not a site admin.", "subjectUserID", accessToken.SubjectUserID, "err", err)
94105
http.Error(w, "The subject user of a sudo access token must be a site admin.", http.StatusForbidden)
95106
return
96107
}
@@ -110,10 +121,10 @@ func AccessTokenAuthMiddleware(db database.DB, next http.Handler) http.Handler {
110121
return
111122
}
112123
actorUserID = user.ID
113-
log15.Debug("HTTP request used sudo token.", "requestURI", r.URL.RequestURI(), "tokenSubjectUserID", subjectUserID, "actorUserID", actorUserID, "actorUsername", user.Username)
124+
log15.Debug("HTTP request used sudo token.", "requestURI", r.URL.RequestURI(), "tokenSubjectUserID", accessToken.SubjectUserID, "actorUserID", actorUserID, "actorUsername", user.Username)
114125
}
115126

116-
r = r.WithContext(actor.WithActor(r.Context(), &actor.Actor{UID: actorUserID}))
127+
r = r.WithContext(actor.WithActor(r.Context(), &actor.Actor{UID: actorUserID, Scopes: scopes, FromToken: true}))
117128
}
118129

119130
next.ServeHTTP(w, r)

go.mod

+4
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@ module github.com/sourcegraph/sourcegraph
22

33
go 1.18
44

5+
// TODO: do not merge this local hack
6+
// source is from this PR: https://github.com/graph-gophers/graphql-go/pull/446
7+
replace github.com/graph-gophers/graphql-go => /Users/milan/work/graphql-go
8+
59
require (
610
cloud.google.com/go/kms v1.4.0
711
cloud.google.com/go/monitoring v1.2.0

internal/actor/actor.go

+5
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ type Actor struct {
4848

4949
// mockUser indicates this user was created in the context of a test.
5050
mockUser bool
51+
52+
// true if actor was created from a token
53+
FromToken bool
54+
// list of scopes of a token in case token auth is used
55+
Scopes map[string]bool
5156
}
5257

5358
// FromUser returns an actor corresponding to the user with the given ID

internal/database/access_tokens.go

+13-16
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,9 @@ type AccessTokenStore interface {
114114
//
115115
// Calling Lookup also updates the access token's last-used-at date.
116116
//
117-
// 🚨 SECURITY: This returns a user ID if and only if the tokenHexEncoded corresponds to a valid,
117+
// 🚨 SECURITY: This returns an access token if and only if the tokenHexEncoded corresponds to a valid,
118118
// non-deleted access token.
119-
Lookup(ctx context.Context, tokenHexEncoded, requiredScope string) (subjectUserID int32, err error)
119+
Lookup(ctx context.Context, tokenHexEncoded string) (*AccessToken, error)
120120

121121
Transact(context.Context) (AccessTokenStore, error)
122122
With(basestore.ShareableStore) AccessTokenStore
@@ -187,16 +187,14 @@ INSERT INTO access_tokens(subject_user_id, scopes, value_sha256, note, creator_u
187187
return id, token, nil
188188
}
189189

190-
func (s *accessTokenStore) Lookup(ctx context.Context, tokenHexEncoded, requiredScope string) (subjectUserID int32, err error) {
191-
if requiredScope == "" {
192-
return 0, errors.New("no scope provided in access token lookup")
193-
}
194-
190+
func (s *accessTokenStore) Lookup(ctx context.Context, tokenHexEncoded string) (*AccessToken, error) {
195191
token, err := decodeToken(tokenHexEncoded)
196192
if err != nil {
197-
return 0, errors.Wrap(err, "AccessTokens.Lookup")
193+
return nil, errors.Wrap(err, "AccessTokens.Lookup")
198194
}
199195

196+
t := &AccessToken{}
197+
200198
if err := s.Handle().QueryRowContext(ctx,
201199
// Ensure that subject and creator users still exist.
202200
`
@@ -205,19 +203,18 @@ WHERE t.id IN (
205203
SELECT t2.id FROM access_tokens t2
206204
JOIN users subject_user ON t2.subject_user_id=subject_user.id AND subject_user.deleted_at IS NULL
207205
JOIN users creator_user ON t2.creator_user_id=creator_user.id AND creator_user.deleted_at IS NULL
208-
WHERE t2.value_sha256=$1 AND t2.deleted_at IS NULL AND
209-
$2 = ANY (t2.scopes)
206+
WHERE t2.value_sha256=$1 AND t2.deleted_at IS NULL
210207
)
211-
RETURNING t.subject_user_id
208+
RETURNING t.id, t.subject_user_id, t.scopes, t.creator_user_id, t.internal, t.created_at
212209
`,
213-
toSHA256Bytes(token), requiredScope,
214-
).Scan(&subjectUserID); err != nil {
210+
toSHA256Bytes(token),
211+
).Scan(&t.ID, &t.SubjectUserID, pq.Array(&t.Scopes), &t.CreatorUserID, &t.Internal, &t.CreatedAt); err != nil {
215212
if err == sql.ErrNoRows {
216-
return 0, ErrAccessTokenNotFound
213+
return nil, ErrAccessTokenNotFound
217214
}
218-
return 0, err
215+
return nil, err
219216
}
220-
return subjectUserID, nil
217+
return t, nil
221218
}
222219

223220
func (s *accessTokenStore) GetByID(ctx context.Context, id int64) (*AccessToken, error) {

internal/database/mocks_temp.go

+16-19
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)