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

Commit

Permalink
[feat] Add access token scopes authorization based on graphql directives
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
kopancek committed Oct 12, 2022
1 parent 7836dc2 commit d29b7b2
Show file tree
Hide file tree
Showing 7 changed files with 98 additions and 50 deletions.
31 changes: 31 additions & 0 deletions cmd/frontend/graphqlbackend/graphqlbackend.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/graph-gophers/graphql-go/introspection"
"github.com/graph-gophers/graphql-go/relay"
"github.com/graph-gophers/graphql-go/trace"
gqltypes "github.com/graph-gophers/graphql-go/types"
"github.com/inconshreveable/log15"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promauto"
Expand Down Expand Up @@ -490,6 +491,9 @@ func NewSchema(
resolver,
graphql.Tracer(&prometheusTracer{}),
graphql.UseStringDescriptions(),
graphql.DirectiveVisitors(map[string]gqltypes.DirectiveVisitor{
"authz": &authzDirectiveVisitor{},
}),
)
}

Expand Down Expand Up @@ -848,3 +852,30 @@ func (r *schemaResolver) CodeHostSyncDue(ctx context.Context, args *struct {
}
return r.db.ExternalServices().SyncDue(ctx, ids, time.Duration(args.Seconds)*time.Second)
}

type authzDirectiveVisitor struct{}

func (v *authzDirectiveVisitor) Before(ctx context.Context, directive *gqltypes.Directive, input interface{}) error {
if scopesAttr, ok := directive.Arguments.Get("scopes"); ok {
a := actor.FromContext(ctx)
// only care about token based auth and non-internal tokens for now
if a.FromToken && !a.Internal {
requiredScopes := scopesAttr.Deserialize(nil).([]interface{})
if len(requiredScopes) < 1 {
return errors.Errorf("Authorization required, but no scopes are given in graphql schema")
}
// for now all scopes are required, but this can be changed in the future
// to be more flexible
for _, scope := range requiredScopes {
if ok := a.Scopes[scope.(string)]; !ok {
return errors.Errorf("Missing token scope %s", scope)
}
}
}
}
return nil
}

func (v *authzDirectiveVisitor) After(ctx context.Context, directive *gqltypes.Directive, output interface{}) (interface{}, error) {
return output, nil
}
13 changes: 8 additions & 5 deletions cmd/frontend/graphqlbackend/schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -1141,6 +1141,8 @@ input SurveySubmissionInput {
better: String
}

directive @authz(scopes: [String]!) on QUERY | FIELD_DEFINITION | OBJECT

"""
Input for a happiness feedback submission.
"""
Expand Down Expand Up @@ -1217,7 +1219,7 @@ type Query {
): RepositoryRedirect
"""
Lists external services under given namespace.
If no namespace is given, it returns all external services.
If no namespace is given, it returns all external services
"""
externalServices(
"""
Expand All @@ -1233,7 +1235,8 @@ type Query {
Opaque pagination cursor.
"""
after: String
): ExternalServiceConnection!
): ExternalServiceConnection! @authz(scopes: ["external_services:read"])

"""
List all repositories.
"""
Expand Down Expand Up @@ -1501,7 +1504,7 @@ type Query {
"""
Retrieve the list of defined feature flags
"""
featureFlags: [FeatureFlag!]!
featureFlags: [FeatureFlag!]! @authz(scopes: ["feature_flags:read"])

"""
Retrieve a feature flag
Expand Down Expand Up @@ -2330,7 +2333,7 @@ type Highlight {
"""
A list of external services.
"""
type ExternalServiceConnection {
type ExternalServiceConnection @authz(scopes: ["external_services:read"]) {
"""
A list of external services.
"""
Expand Down Expand Up @@ -2391,7 +2394,7 @@ type ExternalService implements Node {
"""
The JSON configuration of the external service.
"""
config: JSONCString!
config: JSONCString! @authz(scopes: ["external_services:admin"])

"""
When the external service was created.
Expand Down
31 changes: 21 additions & 10 deletions cmd/frontend/internal/httpapi/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,18 +59,29 @@ func AccessTokenAuthMiddleware(db database.DB, next http.Handler) http.Handler {
return
}

accessToken, err := db.AccessTokens().Lookup(r.Context(), token)
// convert scopes to a map for faster lookup
scopes := make(map[string]bool)
for _, scope := range accessToken.Scopes {
scopes[scope] = true
}
// Validate access token.
//
// 🚨 SECURITY: It's important we check for the correct scopes to know what this token
// is allowed to do.
var requiredScope string
var hasRequiredScope bool
if sudoUser == "" {
requiredScope = authz.ScopeUserAll
ok := scopes[authz.ScopeUserAll]
// we require either user:all scope or any other scope to be present
hasRequiredScope = ok || len(accessToken.Scopes) > 0
} else {
requiredScope = authz.ScopeSiteAdminSudo
hasRequiredScope = scopes[authz.ScopeSiteAdminSudo]
}
subjectUserID, err := db.AccessTokens().Lookup(r.Context(), token, requiredScope)
if err != nil {
if !hasRequiredScope {
err = database.ErrAccessTokenNotFound
}

if err != nil || !hasRequiredScope {
if err == database.ErrAccessTokenNotFound || errors.HasType(err, database.InvalidTokenError{}) {
log15.Error("AccessTokenAuthMiddleware.invalidAccessToken", "token", token, "error", err)
http.Error(w, "Invalid access token.", http.StatusUnauthorized)
Expand All @@ -85,12 +96,12 @@ func AccessTokenAuthMiddleware(db database.DB, next http.Handler) http.Handler {
// Determine the actor's user ID.
var actorUserID int32
if sudoUser == "" {
actorUserID = subjectUserID
actorUserID = accessToken.SubjectUserID
} else {
// 🚨 SECURITY: Confirm that the sudo token's subject is still a site admin, to
// prevent users from retaining site admin privileges after being demoted.
if err := auth.CheckUserIsSiteAdmin(r.Context(), db, subjectUserID); err != nil {
log15.Error("Sudo access token's subject is not a site admin.", "subjectUserID", subjectUserID, "err", err)
if err := auth.CheckUserIsSiteAdmin(r.Context(), db, accessToken.SubjectUserID); err != nil {
log15.Error("Sudo access token's subject is not a site admin.", "subjectUserID", accessToken.SubjectUserID, "err", err)
http.Error(w, "The subject user of a sudo access token must be a site admin.", http.StatusForbidden)
return
}
Expand All @@ -110,10 +121,10 @@ func AccessTokenAuthMiddleware(db database.DB, next http.Handler) http.Handler {
return
}
actorUserID = user.ID
log15.Debug("HTTP request used sudo token.", "requestURI", r.URL.RequestURI(), "tokenSubjectUserID", subjectUserID, "actorUserID", actorUserID, "actorUsername", user.Username)
log15.Debug("HTTP request used sudo token.", "requestURI", r.URL.RequestURI(), "tokenSubjectUserID", accessToken.SubjectUserID, "actorUserID", actorUserID, "actorUsername", user.Username)
}

r = r.WithContext(actor.WithActor(r.Context(), &actor.Actor{UID: actorUserID}))
r = r.WithContext(actor.WithActor(r.Context(), &actor.Actor{UID: actorUserID, Scopes: scopes, FromToken: true}))
}

next.ServeHTTP(w, r)
Expand Down
4 changes: 4 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ module github.com/sourcegraph/sourcegraph

go 1.18

// TODO: do not merge this local hack
// source is from this PR: https://github.com/graph-gophers/graphql-go/pull/446
replace github.com/graph-gophers/graphql-go => /Users/milan/work/graphql-go

require (
cloud.google.com/go/kms v1.4.0
cloud.google.com/go/monitoring v1.2.0
Expand Down
5 changes: 5 additions & 0 deletions internal/actor/actor.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ type Actor struct {

// mockUser indicates this user was created in the context of a test.
mockUser bool

// true if actor was created from a token
FromToken bool
// list of scopes of a token in case token auth is used
Scopes map[string]bool
}

// FromUser returns an actor corresponding to the user with the given ID
Expand Down
29 changes: 13 additions & 16 deletions internal/database/access_tokens.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,9 @@ type AccessTokenStore interface {
//
// Calling Lookup also updates the access token's last-used-at date.
//
// 🚨 SECURITY: This returns a user ID if and only if the tokenHexEncoded corresponds to a valid,
// 🚨 SECURITY: This returns an access token if and only if the tokenHexEncoded corresponds to a valid,
// non-deleted access token.
Lookup(ctx context.Context, tokenHexEncoded, requiredScope string) (subjectUserID int32, err error)
Lookup(ctx context.Context, tokenHexEncoded string) (*AccessToken, error)

Transact(context.Context) (AccessTokenStore, error)
With(basestore.ShareableStore) AccessTokenStore
Expand Down Expand Up @@ -187,16 +187,14 @@ INSERT INTO access_tokens(subject_user_id, scopes, value_sha256, note, creator_u
return id, token, nil
}

func (s *accessTokenStore) Lookup(ctx context.Context, tokenHexEncoded, requiredScope string) (subjectUserID int32, err error) {
if requiredScope == "" {
return 0, errors.New("no scope provided in access token lookup")
}

func (s *accessTokenStore) Lookup(ctx context.Context, tokenHexEncoded string) (*AccessToken, error) {
token, err := decodeToken(tokenHexEncoded)
if err != nil {
return 0, errors.Wrap(err, "AccessTokens.Lookup")
return nil, errors.Wrap(err, "AccessTokens.Lookup")
}

t := &AccessToken{}

if err := s.Handle().QueryRowContext(ctx,
// Ensure that subject and creator users still exist.
`
Expand All @@ -205,19 +203,18 @@ WHERE t.id IN (
SELECT t2.id FROM access_tokens t2
JOIN users subject_user ON t2.subject_user_id=subject_user.id AND subject_user.deleted_at IS NULL
JOIN users creator_user ON t2.creator_user_id=creator_user.id AND creator_user.deleted_at IS NULL
WHERE t2.value_sha256=$1 AND t2.deleted_at IS NULL AND
$2 = ANY (t2.scopes)
WHERE t2.value_sha256=$1 AND t2.deleted_at IS NULL
)
RETURNING t.subject_user_id
RETURNING t.id, t.subject_user_id, t.scopes, t.creator_user_id, t.internal, t.created_at
`,
toSHA256Bytes(token), requiredScope,
).Scan(&subjectUserID); err != nil {
toSHA256Bytes(token),
).Scan(&t.ID, &t.SubjectUserID, pq.Array(&t.Scopes), &t.CreatorUserID, &t.Internal, &t.CreatedAt); err != nil {
if err == sql.ErrNoRows {
return 0, ErrAccessTokenNotFound
return nil, ErrAccessTokenNotFound
}
return 0, err
return nil, err
}
return subjectUserID, nil
return t, nil
}

func (s *accessTokenStore) GetByID(ctx context.Context, id int64) (*AccessToken, error) {
Expand Down
35 changes: 16 additions & 19 deletions internal/database/mocks_temp.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit d29b7b2

Please sign in to comment.